Skip to content
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

Add support for sending empty header values #132

Merged
merged 1 commit into from
Nov 28, 2016

Conversation

GUI
Copy link
Contributor

@GUI GUI commented Nov 25, 2016

Currently, there's not a super-easy way to pass a header with an empty value (e.g., X-Foo: ) in Typhoeus or Ethon. If an empty string is passed in as the header value, then currently the header is omitted from the request altogether. This is due to how curl historically treated empty values (see https://curl.haxx.se/mail/lib-2010-08/0174.html). Curl 7.23.0 (released November 2011) added a more standard way to pass empty values, but it requires special syntax of using a semicolon terminator.

This changes Ethon's behavior, so that when an empty string is passed as the value of a header, it gets sent by curl as an empty HTTP header, rather than omitting the header. A nil value will still omit the header. This seems like a more intuitive approach to me, but this does make this a backwards incompatible change if you were relying on the previous empty string behavior. So if that seems problematic, let me know if you have any other ideas on how this should be handled, or if you don't think Ethon should handle this at all.

A few implementation notes:

  • This requires curl 7.23.0 or higher, where the ability to send empty header values (via a semicolon terminator) was added. If using an older version of curl without this support, then curl's previous behavior of omitting empty headers when an empty string is used remains the same. While this doesn't necessarily seem ideal (since Ethon's behavior will differ depending on the version of curl you have), I'm not sure of the best alternative.
  • Based on some micro-benchmarks, detecting empty strings by using the EMPTY_STRING_VALUE constant seems like the fastest approach (while .empty? is marginally faster it becomes slower once you account for checking whether or not the value is a String).
  • The behavior hasn't changed if the value is any other object type, like numbers, but I added a test when passing a number just as a sanity check.

If an empty string is passed as the value of a header, it now gets sent
as an empty HTTP header, rather than omitting the header. A nil value
will still omit the header.

This requires curl 7.23.0 or higher, where the ability to send empty
header values (via a semicolon terminator) was added. If using an older
version of curl without this support, then curl's previous behavior of
omitting empty headers remains the same.
@hanshasselberg
Copy link
Member

Hi @GUI, thank you for your work and your detailed description.

While this doesn't necessarily seem ideal (since Ethon's behavior will differ depending on the version of curl you have), I'm not sure of the best alternative.

Technically, the behavior of Ethon doesn't change, but the one of curl. Right? If thats what you meant, I don't think it is a big deal. Curls behaviour changes every once in a while. Ethon doesn't depend on a specific version.

@GUI
Copy link
Contributor Author

GUI commented Nov 27, 2016

Yes, you're correct. Ethon's behavior is the same with this PR: an empty string value will result in the header being sent to libcurl as semicolon terminated. How curl interprets that semicolon differs depending on the version of curl: less than version 7.23.0 will result in the header not being set (similar to the previous behavior), while 7.23.0+ will omit the header. This test demonstrates the difference: https://github.com/typhoeus/ethon/pull/132/files#diff-aa2b066a5d2286fb8a792b169fe666c1R50 Let me know if you have any other questions or suggestions.

Thanks for taking a look!

@hanshasselberg hanshasselberg merged commit 3e58db3 into typhoeus:master Nov 28, 2016
gonubana pushed a commit to gonubana/ethon that referenced this pull request Dec 1, 2022
while also allowing the user to remove default headers set by libcurl.
See https://curl.se/libcurl/c/CURLOPT_HTTPHEADER.html for more details.

passing " headers: {'h1' => 'v1', 'h2' => '', 'h3;' => ''} "
corresponds to these curl options: " -H 'h1: v1' -H 'h2:'  -H 'h3;' "

Enhances typhoeus#132
Fixes typhoeus/typhoeus#706
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants