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

Keep underscores when normalizing headers #964

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mvz
Copy link

@mvz mvz commented Jan 5, 2022

Some servers distinguish underscores from dashes in request headers. To allow users to test that the correct character is used, checking the sent headers should make the same distinction.

With this change, webmock no longer normalizes underscores to dashes for headers defined using strings.

Since symbols are commonly used with underscores to specify headers that use dashes, headers specified with symbol keys are still normalized to dashes.

Fixes #474.

Some servers distinguish underscores from dashes in request headers. To
allow users to test that the correct character is used, checking the
sent headers should make the same distinction.

With this change, webmock no longer normalizes underscores to dashes for
headers defined using strings.

Since symbols are commonly used with underscores to specify headers that
use dashes, headers specified with symbol keys are still normalized to
dashes.
@bblimke
Copy link
Owner

bblimke commented Jan 6, 2022

@mvz thank you for the PR. I'm not sure about the new logic though. It has a different behaviour for symbols than strings based on assumption "commonly used with underscores" yet strings are not.

I think the behaviour should be unified and deterministic for every case.

Change like that will require a major version bump anyway.

I'm keen to stop normalizing headers completely, do a major version bump, yet have a way of headers matching,
that has a mapping of acceptable common symbols used for some headers. E.g. :content_type => 'Content-Type'
That mapping could be extended by developers.

@mvz
Copy link
Author

mvz commented Jan 7, 2022

It has a different behaviour for symbols than strings based on assumption "commonly used with underscores" yet strings are not.

Yes, that is on purpose since symbols with dashes are harder to write so many developers will probably have symbols used like that. However, I'm all for being more strict here and just unifying the behavior.

In fact, given #863 (comment), normalization could then become just a simple .downcase on the stringified key.

Change like that will require a major version bump anyway.

To make this smoother, this could also be introduced as an optional change controlled by a setting. The old behavior can than be removed with a major version bump later.

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.

WebMock::Util::Headers.normalize_headers replaces underscores with dashes
2 participants