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

RequestSerializer: include Content-Type and Content-Length headers. #79

Merged
merged 2 commits into from
Nov 14, 2019

Conversation

cristiangreco
Copy link
Contributor

RequestSerializer returns, among other things, the filtered http headers
of the handled request, that is: all the HTTP_* headers normally
provided by Rack with a filtering rule applied (this is useful if you
want to e.g. mask or transform the content of some headers).

Among the request headers injected by Rack into the request environment
there are two notable exceptions to the 'HTTP_' rule: both CONTENT_TYPE
and CONTENT_LENGTH are not prefixed with 'HTTP_'. (more on this on the
Rack SPEC doc [0] and the lint file [1]).

This pull request changes the headers filtering logic to take both
CONTENT_TYPE and CONTENT_LENGTH into account. RequestSerializer will now
return 'http_content_type' and 'http_content_length'.

As far as I can tell this is a backward-compatible change as we're just
returning a new pair of headers. No filtering rule should be broken by
this change as such rules are only applied to the filtered headers.

[0] https://rubydoc.info/github/rack/rack/master/file/SPEC
[1] https://github.com/rack/rack/blob/master/lib/rack/lint.rb

RequestSerializer returns, among other things, the filtered http headers
of the handled request, that is: all the HTTP_* headers normally
provided by Rack with a filtering rule applied (this is useful if you
want to e.g. mask or transform the content of some headers).

Among the request headers injected by Rack into the request environment
there are two notable exceptions to the 'HTTP_' rule: both CONTENT_TYPE
and CONTENT_LENGTH are not prefixed with 'HTTP_'. (more on this on the
Rack SPEC doc [0] and the lint file [1]).

This pull request changes the headers filtering logic to take both
CONTENT_TYPE and CONTENT_LENGTH into account. RequestSerializer will now
return 'http_content_type' and 'http_content_length'.

As far as I can tell this is a backward-compatible change as we're just
returning a new pair of headers. No filtering rule should be broken by
this change as such rules are only applied to the filtered headers.

[0] https://rubydoc.info/github/rack/rack/master/file/SPEC
[1] https://github.com/rack/rack/blob/master/lib/rack/lint.rb
@cristiangreco
Copy link
Contributor Author

@lawrencejones I think you've already hit this in one of our services. I realised this thing when discovering that we're not logging the content-type of requests we get in.

@cristiangreco
Copy link
Contributor Author

@nickcampbell18 do you think we could cut a new release with this in? In that case, should I update the changelog in this PR?

@nickcampbell18
Copy link
Contributor

Yes that's probably best. It's probably a minor bump because this is a feature rather than bug?

@cristiangreco
Copy link
Contributor Author

I've pushed the updates to CHANGELOG to prepare for 2.1.0 (it also includes a PR from @jacobpgn 🎉).

@nickcampbell18 feel free to merge and publish if you want to.

@nickcampbell18 nickcampbell18 merged commit 371adf2 into master Nov 14, 2019
@nickcampbell18 nickcampbell18 deleted the cristian-rack-unprefixed-headers branch November 14, 2019 10:53
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.

None yet

2 participants