[foundation] Fix POST/PUT issues with NSUrlSessionHandler. Fixes #52682 (#1772) (#1778) - #1790
Merged
Merged
Conversation
…82 (dotnet#1772) (dotnet#1778) The latest NSUrlSessionHandler implementation switched from loading the request into memory (NSData) to loading it from a stream (NSInputStream). The later is more efficient, at least for large POST'ed requests which could, in theory, not even fit in memory. Now using a stream means a different API is used. NSMutableUrlRequest Body and BodyStream properties are exclusive but also a bit different as the later won't set Content-Length [1] and switch to chunked encoding [2] Even if the current code is compliant with HTTP/1.1 this breaks some common POST/PUT usage that worked "as expected" in the previous (C8) release. To fix this we ask for the stream's length. We assume that if the length is known then it's (likely) already (or fitting) in memory and use the (old, in memory) Body property, which will set Content-Length. If the length is unknown then we use the BodyStream approach so large POST will continue to work (well over past/C8 limits). Finally there might be case where developers will prefer to avoid the extra memory (of `Body`) so a new property `MaxInputInMemory` is added to set a maximum. It defaults to `Int64.MaxValue` so `Body` will be used whenever possible. It can be set to `0` to get back the original C9 (always use a stream) behavior. references: [1] https://bugzilla.xamarin.com/show_bug.cgi?id=52682 [2] https://bugzilla.xamarin.com/show_bug.cgi?id=52711 [3] https://devforums.apple.com/message/919330#919330
spouliot
added
requires-approval-before-merge
Contributor
|
Build failure |
Contributor
Author
|
known random issue
|
rolfbjarne
approved these changes
Mar 1, 2017
Contributor
Author
spouliot
added a commit
to spouliot/xamarin-macios
that referenced
this pull request
Mar 27, 2017
…82 (dotnet#1772) (dotnet#1778) (dotnet#1790) The latest NSUrlSessionHandler implementation switched from loading the request into memory (NSData) to loading it from a stream (NSInputStream). The later is more efficient, at least for large POST'ed requests which could, in theory, not even fit in memory. Now using a stream means a different API is used. NSMutableUrlRequest Body and BodyStream properties are exclusive but also a bit different as the later won't set Content-Length [1] and switch to chunked encoding [2] Even if the current code is compliant with HTTP/1.1 this breaks some common POST/PUT usage that worked "as expected" in the previous (C8) release. To fix this we ask for the stream's length. We assume that if the length is known then it's (likely) already (or fitting) in memory and use the (old, in memory) Body property, which will set Content-Length. If the length is unknown then we use the BodyStream approach so large POST will continue to work (well over past/C8 limits). Finally there might be case where developers will prefer to avoid the extra memory (of `Body`) so a new property `MaxInputInMemory` is added to set a maximum. It defaults to `Int64.MaxValue` so `Body` will be used whenever possible. It can be set to `0` to get back the original C9 (always use a stream) behavior. references: [1] https://bugzilla.xamarin.com/show_bug.cgi?id=52682 [2] https://bugzilla.xamarin.com/show_bug.cgi?id=52711 [3] https://devforums.apple.com/message/919330#919330
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The latest NSUrlSessionHandler implementation switched from loading the
request into memory (NSData) to loading it from a stream (NSInputStream).
The later is more efficient, at least for large POST'ed requests which
could, in theory, not even fit in memory.
Now using a stream means a different API is used. NSMutableUrlRequest
Body and BodyStream properties are exclusive but also a bit different as
the later won't set Content-Length [1] and switch to chunked encoding [2]
Even if the current code is compliant with HTTP/1.1 this breaks some
common POST/PUT usage that worked "as expected" in the previous (C8)
release.
To fix this we ask for the stream's length. We assume that if the length
is known then it's (likely) already (or fitting) in memory and use the
(old, in memory) Body property, which will set Content-Length.
If the length is unknown then we use the BodyStream approach so large
POST will continue to work (well over past/C8 limits).
Finally there might be case where developers will prefer to avoid the
extra memory (of
Body) so a new propertyMaxInputInMemoryis addedto set a maximum. It defaults to
Int64.MaxValuesoBodywill beused whenever possible. It can be set to
0to get back the original C9(always use a stream) behavior.
references:
[1] https://bugzilla.xamarin.com/show_bug.cgi?id=52682
[2] https://bugzilla.xamarin.com/show_bug.cgi?id=52711
[3] https://devforums.apple.com/message/919330#919330