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

Preserve original header casing which becomes exposed by getNames(). #2

Merged
merged 1 commit into from
Sep 30, 2016
Merged

Preserve original header casing which becomes exposed by getNames(). #2

merged 1 commit into from
Sep 30, 2016

Conversation

alexjeffburke
Copy link
Collaborator

Hey,

This probably needs a little more work, but wanted to get started right away.

This commit adds a dictionary that stores the original casing headers were supplied with. getNames() is altered to pull those out, but other places accessing headers are forced to use an explicit lowercase version of the function to avoid behavioural change.

Two things I'm aware may need changing:

  • the name of the extra dictionary (currently namesWithCase)
  • whether getNamesLowerCase() should be exposed

Cheers, Alex.

@coveralls
Copy link

coveralls commented Sep 27, 2016

Coverage Status

Coverage decreased (-0.1%) to 91.753% when pulling b523c72 on alexjeffburke:feature/preserve-header-casing into 38e40dd on papandreou:master.

@papandreou
Copy link
Owner

This looks really promising. I think I would have come up with exactly the same implementation :)

@alexjeffburke
Copy link
Collaborator Author

alexjeffburke commented Sep 29, 2016

This has been tested against unexpected-messy - there were two cases of direct access to the lowercase names which when addressed allowed the entire test suite to pass as is.

After our quick touch base earlier, if I can get an ack here will start getting these things merged. I guess we're talking semver-minor for these bits to cover our bases?

@alexjeffburke
Copy link
Collaborator Author

Oh and this immediately causes unexpected-mitm to write headers specified using response object syntax with their original case :)

@papandreou
Copy link
Owner

toString, toJSON, and toCanonicalObject should probably also preserve the casing now that we're able to do it?

Not a blocker, though, so 👍 from me :)

@alexjeffburke alexjeffburke merged commit b523c72 into papandreou:master Sep 30, 2016
@alexjeffburke alexjeffburke deleted the feature/preserve-header-casing branch September 30, 2016 10:52
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

3 participants