Skip to content

Commit

Permalink
Improve the comment regarding CURLOPT_ENCODING handling (#2904)
Browse files Browse the repository at this point in the history
While researching how the `decode_content` option is meant to be used and
looking into the Guzzle source code I stumbled over this part of the code,
because it was non-obvious to me that providing an empty string to
`CURLOPT_ENCODING` does not *disable* the decoding, but instead enables *all*
decoders.
  • Loading branch information
TimWolla committed Jul 17, 2021
1 parent 702cca4 commit 1d46cec
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/Handler/CurlFactory.php
Expand Up @@ -385,8 +385,11 @@ private function applyHandlerOptions(EasyHandle $easy, array &$conf): void
if ($accept) {
$conf[\CURLOPT_ENCODING] = $accept;
} else {
// The empty string enables all available decoders and implicitly
// sets a matching 'Accept-Encoding' header.
$conf[\CURLOPT_ENCODING] = '';
// Don't let curl send the header over the wire
// But as the user did not specify any acceptable encodings we need
// to overwrite this implicit header with an empty one.
$conf[\CURLOPT_HTTPHEADER][] = 'Accept-Encoding:';

This comment has been minimized.

Copy link
@divinity76

divinity76 Oct 24, 2022

Contributor

this header is incorrect. according to badly designed specs (i have no idea what the designers were thinking, the default is horrible!), when the user-agent does not specify any encodings, the server is free to choose any encoding it wishes. the correct way to specify no encoding whatsoever is

Accept-Encoding: identity

one famous example of a webserver choosing an encoding because the client didn't request any is Amazon.com

This comment has been minimized.

Copy link
@jtojnar

jtojnar Apr 13, 2024

I think this is working as intended, even if I disagree with the feature. I have opened a PR that clarifies the comment #3215

}
}
Expand Down

0 comments on commit 1d46cec

Please sign in to comment.