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

Provide access to request headers via the headers helper #317

Closed
cobbr2 opened this issue Jan 17, 2013 · 4 comments
Closed

Provide access to request headers via the headers helper #317

cobbr2 opened this issue Jan 17, 2013 · 4 comments

Comments

@cobbr2
Copy link

cobbr2 commented Jan 17, 2013

The top-level README.markdown implies at https://github.com/intridea/grape#headers that the headers helper might be used to get the values of headers sent in the HTTP request. It presents the use of the helper and the use of the env as roughly equivalent.

In fact, the helper (Grape::Endpoint#headers) only provides access to headers that have been set by the API; it is also the helper you use to set them.

To access headers in the incoming request, you must use the env method.

I'll rewrite the markdown to clarify that the two examples are addressing two different use cases and submit a pull request -- or is there some intent to unify this access (I don't see a good way to do so, but having unified naming of the keys would certainly make code clearer)

@dblock
Copy link
Member

dblock commented Jan 17, 2013

I think we should definitely fallback in headers to get the ones from ENV. Isn't this just a matter of storing both separately and returning a merged hash from headers?

@cobbr2
Copy link
Author

cobbr2 commented Jan 17, 2013

From a convenience standpoint, I agree with you. On the other hand, it sure seems sensible to be able to tell the difference between "what you sent me" and "what I'm sending you". Haven't thought of an API-level case where that's important, but for example, session middleware handling csrf and session hijacking definitely has to tell the difference.

Here's a (probably too ambitious) proposal for the APIs:

  • Two new Hashie mixins, which are combined to construct a Hashie::HeaderHash class:
    • Hashie::Titleized: represents keys using Titleized-And-Dashed strings roughly like you'd see in http://www.iana.org/assignments/message-headers/perm-headers.html, but is indifferent to access by symbol or string and to case differences.
    • Hashie::Multivalue: Supports multi-valued hash keys (e.g., more than one WWW-Authenticate header), perhaps using this approach. I haven't spelunked the methods Rack uses for handling this recently.
  • headers (returns the merged unified HeaderHash of both request_headers and response_headers)
  • headers[]= delegates to response_headers[]=
  • request_headers initializes its HeaderHash from env.keys.grep /^HTTP_/, removing the HTTP_ and replacing _ with -.
  • response_headers works just like current headers does, but using a HeaderHash
  • the actual rack response to call would use response_headers, not headers

One point I just got beat on the head with: HTTP headers (on the wire) treat - and _ differently. In order to keep programmers from thinking poorly, some intermediaries, e.g., nginx treat underscores as illegal characters in header names by default.

When using custom headers, I think it's better to encourage programmers to think with the '-' (so they don't, e.g., document "_" to their early API consumers and then have to fight with their network team to get them allowed). So I'm not proposing to normalize _ to a '-' at the stage where we normalize the header names.

@cobbr2 cobbr2 closed this as completed Jan 17, 2013
@cobbr2 cobbr2 reopened this Jan 17, 2013
@dblock
Copy link
Member

dblock commented Jan 18, 2013

I like it, headers has everything and request_headers and response_headers separate the two.

If you're going to PR this, make sure to read everything coming up with rack request headers in Google (like this), seems that there're a few gotchas.

@dblock dblock closed this as completed in f6f585e Feb 7, 2013
@dblock
Copy link
Member

dblock commented Feb 7, 2013

I added headers that will return a parsed hash of headers. Check it out.

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

No branches or pull requests

2 participants