-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[URLSession] Fix handling of compressed responses. #2108
[URLSession] Fix handling of compressed responses. #2108
Conversation
@swift-ci test |
7f945b0
to
979ce02
Compare
Changed an incorrect test assert message. |
@swift-ci test |
1 similar comment
@swift-ci test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This diff has two-space indentation. Could you please change it to four-space, something which we've been following here? The patch looks great otherwise.
When the server answers with a compressed response (with Content-Encoding header), cURL is configured to automatically decompress the response. However, there was some inconsistencies with the Foundation implementation in Darwin platforms, and some bugs in the handling of compressed responses. - In Darwin Foundation, the expected number of bytes is -1 for compressed responses. This is because Content-Lenght will report the length of the compressed content, but the total bytes written reports uncompressed lengths. The code is changed to set the expected number of bytes to -1 if the response includes a Content-Encoding header different to "identity". - Since the expected number of bytes is unknown, the data received callback cannot check for the total bytes received to be equal to close the file handler and provide the temporary file URL to the upper level. That responsability has been moved into the complete task callback, where it was already happening for tasks with completion blocks. - Added two tests (one for data tasks, one for download tasks) with gzipped data. - Since gzipped data cannot be represented by UTF-8 strings, the HTTPServer code has to be modified to allow providing raw data as part of the HTTP response. There's a lot of changes so the body is raw data, and the previously provided strings are transformed into data using UTF-8 instead. - There was a small bug in the HTTPServer code where the setUp will wait for a flag to be different of -2 to indicate the server is ready. However, the flag should be checked against -1, which is the initial state, while -2 is the final state. I found this when the server port that my test wanted to use was uninitialized, because the server is started asynchronously in another queue, and the value wasn't valid yet.
979ce02
to
770a8f5
Compare
Fixed the indentation. I hope I fixed every case, but don’t hesitate to point out if you find any more. |
@swift-ci test |
1 similar comment
@swift-ci test |
@drodriguez Thanks! Looks good to me now. |
@swift-ci test and merge |
1 similar comment
@swift-ci test and merge |
When the server answers with a compressed response (with
Content-Encoding header), cURL is configured to automatically decompress
the response. However, there was some inconsistencies with the
Foundation implementation in Darwin platforms, and some bugs in the
handling of compressed responses.
compressed responses. This is because Content-Lenght will report the
length of the compressed content, but the total bytes written reports
uncompressed lengths. The code is changed to set the expected number
of bytes to -1 if the response includes a Content-Encoding header
different to "identity".
callback cannot check for the total bytes received to be equal to
close the file handler and provide the temporary file URL to the upper
level. That responsability has been moved into the complete task
callback, where it was already happening for tasks with completion
blocks.
gzipped data.
HTTPServer code has to be modified to allow providing raw data as part
of the HTTP response. There's a lot of changes so the body is raw
data, and the previously provided strings are transformed into data
using UTF-8 instead.
for a flag to be different of -2 to indicate the server is ready.
However, the flag should be checked against -1, which is the initial
state, while -2 is the final state. I found this when the server port
that my test wanted to use was uninitialized, because the server is
started asynchronously in another queue, and the value wasn't valid
yet.