Skip to content

Sanitize UTF8 characters from headers#1674

Merged
jmhooper merged 1 commit intomasterfrom
jmhooper-header-encoding
Sep 15, 2017
Merged

Sanitize UTF8 characters from headers#1674
jmhooper merged 1 commit intomasterfrom
jmhooper-header-encoding

Conversation

@jmhooper
Copy link
Contributor

Why: Rack encodes headers into 8 bit ASCII which results in encoding
compatibility errors further down the stack when the app tries to
manipulate them. This commit encodes the headers and replaces
incompatible characters with ? characters so the headers do not cause
the app to respond with 500s.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jmhooper jmhooper force-pushed the jmhooper-header-encoding branch from 7c00e17 to f58943f Compare September 14, 2017 23:04
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might be the wrong header to test because we delete the entire header?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed up this spec so that it doesn't use the Host header. Mind taking another look?

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for fixing

@jmhooper jmhooper force-pushed the jmhooper-header-encoding branch from 289d3de to 759f522 Compare September 15, 2017 19:11
**Why**: Rack encodes headers into 8 bit ASCII which results in encoding
compadibility errors futher down the stack when the app tries to
manipulate them. This commit encodes the headers and replaces
incompatible characters with `?` characters so the headers do not cause
the app to respond with 500s.
@jmhooper jmhooper merged commit 1fcbf66 into master Sep 15, 2017
@jmhooper jmhooper deleted the jmhooper-header-encoding branch December 12, 2017 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants