Refactor Parsing Content Encoding Http Headers #15245
Refactor Parsing Content Encoding Http Headers #15245Hrushi20 wants to merge 3 commits intoNixOS:masterfrom
Conversation
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>
src/libstore/filetransfer.cc
Outdated
There was a problem hiding this comment.
Will refactor the function to return CompressionAlgo instead of string based on #15238
|
Also fixes #14324 |
|
Could you also run maintainers/format.sh to make clang-format happy? |
src/libstore/filetransfer.cc
Outdated
There was a problem hiding this comment.
| throw nix::Error("Stacked contentEncoding is not supported: %s", contentEncoding); | |
| throw nix::Error("Stacked Content-Encoding is not supported: %s", contentEncoding); |
|
|
||
| else if (name == "content-encoding") | ||
| encoding = trim(line.substr(i + 1)); | ||
| encoding = parseContentEncoding(toLower(trim(line.substr(i + 1)))); |
There was a problem hiding this comment.
Should we add a comment referring to the spec paragraph about case insensitivity?
xokdvium
left a comment
xokdvium
left a comment
There was a problem hiding this comment.
Should we also modify Accept-Encoding to not send xz as it's really not supported by anything out there?
src/libstore/filetransfer.cc
Outdated
There was a problem hiding this comment.
I wonder if we should have a placeholder for deflate and/or check if it's really broken? I have a suspicion that it is, but would be nice to check. There's already some infra in content-encoding.nix test for testing various compression methods. Could be hard to check it with nginx's support for deflate.
There was a problem hiding this comment.
Or I can add a placeholder for deflate for now.
We can remove xz from accept-encoding as we are removing support from content-encoding. |
|
Closed accidentally? |
…, run clang-formatter Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>
|
Yeah. Laptop slipped from my hand and accidentally closed the PR 🙃 |
Removed xz from Accept-Encoding. |
| result.urls.push_back(effectiveUriCStr); | ||
| } | ||
|
|
||
| std::string parseContentEncoding(std::string_view contentEncoding) |
3 participants
Support parsing following encodings in case-insensitive manner -
gzip, x-gzip, compress, x-compress, br, zstd, bzip2, and identity
Throw Exception for Stacked/Unsupported encodings.
Based on conversation #15238