Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Allow the user to specify or omit the default user-agent header #206

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

gdb commented Oct 10, 2012

No description provided.

Owner

igrigorik commented Oct 10, 2012

I don't think you need this, as we already have this functionality.. Simply pass in an empty UA header when you make the request and you'll get the same behavior.

gdb commented Oct 10, 2012

Unless I'm mistaken, won't you end up with a present-but-empty UA header in that case? This lets you omit the header entirely.

Owner

igrigorik commented Oct 10, 2012

Hmm, you're right - we do the empty check for params, but not for headers. If we want the functionality, then that's the way to do it.. No need to expose extra params on the object.

gdb commented Oct 10, 2012

But then you'd lose the ability to send empty headers. It's probably uncommon that people want that, but my use-case ATM is a reverse proxy where I'd like to pass through the user's header hash as faithfully as possible.

gdb commented Oct 28, 2012

Any thoughts? I'd love to stop having to maintain a fork with such a trivial patch. Thanks!

Owner

igrigorik commented Oct 28, 2012

The current patch is not the behavior we want, need to switch to empty header override instead.. just haven't gotten around to it.

gdb commented Oct 28, 2012

Gotcha. It'd be cool if there were a way to still support people who want empty headers, though understandable if you don't want to support that case. I'm not sure if there's a good API for that... I guess maybe something like:

  • When emitting headers, if headers.include?('foo') but headers['foo'] is nil, then don't encode a Foo header. I.e. consider a nil header value to be a placeholder telling the library not to add this header.
  • Change the code here to be something like: "headers['user-agent'] = 'Default User Agent' unless headers.include?('user-agent')"

would work, though it feels a bit janky.

Owner

igrigorik commented Oct 28, 2012

Empty headers is an edge case, inside of edge case for the HTTP spec.. If you're relying on empty headers, you're doing it wrong. :-)

Ex, open bug on Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=474845, and good overview on SO: http://stackoverflow.com/a/12131993/510112

Re, implementation: .nil? check seems like the right way to do it. If that's all we do, and don't check for empty strings, then in theory you can have your behavior, but I'd personally probably also argue for a .empty? check.

gdb commented Oct 28, 2012

Agreed that empty headers are kind of edgy. But if you're writing a reverse proxy, you want to be able to send along whatever data you have so the upstream can decide what to do with it.

Hi. I didn't see this one before, but yesterday I submitted a patch to do this. If the header is set to nil or "" then the header would be removed. Default UA would be used if no UA is present (i.e. headers.key?('user-agent') == false

#228

Owner

igrigorik commented Jun 17, 2013

Already handled by #228.

@igrigorik igrigorik closed this Jun 17, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment