Check if NSURLProtocolClient has private callback before calling it #2765
Conversation
| @@ -288,7 +288,8 @@ - (void)startLoading { | |||
| // TODO #2721: If this properly supports streaming bodies instead of flattening the data up front, | |||
| // more accurate callbacks can be done | |||
| NSInteger contentSize = self.request.HTTPBody ? self.request.HTTPBody.length : _flattenedBodyStream.size(); | |||
| if (contentSize > 0) { | |||
| if (([self.client respondsToSelector:@selector(URLProtocol:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:)]) && | |||
There was a problem hiding this comment.
In looking for this issue, I realized that no header declares the protocol method URLProtocol:didWriteData:...; could we add one in NSURLProtocolInternal? Ideally, NSURLConnection would conform to this private protocol, and the compiler would warn us if it didn't have the right methods on it. NSURLSessionTask should eventually do this, but it doesn't.
There was a problem hiding this comment.
This is literally declared in NSURLProtocolInternal.h...
There was a problem hiding this comment.
there are other "private" invocations in this file, are these expected to succeed?
There was a problem hiding this comment.
What do you mean by other "private" invocations?
|
@ms-jihua this is the entirety of NSURLProtocolInternal.h for me: //******************************************************************************
//
// Copyright (c) Microsoft. All rights reserved.
//
// This code is licensed under the MIT License (MIT).
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
//
//******************************************************************************
#pragma once
#import "Starboard/SmartTypes.h"
@interface NSURLProtocol ()
+ (id)_URLProtocolClassForRequest:(id)request;
@end |
|
Yup, we lack POST/UploadTask tests for NSURLSession - should I add some? |
|
@DHowett-MSFT ...I somehow missed the existence of another NSURLProtocolInternal.h in Frameworks/include. I'll coalesce it with the one in Frameworks\Foundation. |
|
@ms-jihua yes, It would be helpful to have at least one test for POST using NSURLSession. |
|
@DHowett-MSFT @rajsesh-msft wait a second, [NSURLSession uploadTaskWith...] functions are still stubs. How did we hit this bug? |
|
@ms-jihua two things: 1) users can implement NSURLProtocolClient on their own, and they don't know about this. 2) A data task can still be a post with a body stream if it's created from an NSURLRequest with a body/bodystream. Also, GETs can have bodies just as well as POSTs can. 😄 |
- Add test for a POST NSURLSessionTask
Fixes #2764
This change is