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

fix(ajax): Fix case-insensitive headers in HTTP request #4453

Merged

Conversation

DostalTomas
Copy link
Contributor

@DostalTomas DostalTomas commented Jan 7, 2019

Description:
In RFC 7230 and RFC 7540 is written, that HTTP headers is case-insensitive, so this is fix for correct serializing request body based on HTTP headers.

@coveralls
Copy link

coveralls commented Jan 7, 2019

Pull Request Test Coverage Report for Build 7913

  • 9 of 9 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.003%) to 96.969%

Totals Coverage Status
Change from base Build 7909: 0.003%
Covered Lines: 5791
Relevant Lines: 5972

💛 - Coveralls

Copy link
Member

@benlesh benlesh left a comment

Choose a reason for hiding this comment

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

  1. Do we have an example of where the current implementation fails?
  2. Can we please add a test to confirm that the changes got the desired effect?

In RFC 7230 and RFC 7540 is written, that HTTP headers is case-insensitive, so this is fix for correct serializing request body base on HTTP headers.
@DostalTomas DostalTomas force-pushed the fix-http-headers-case-sensitivity branch from e00d126 to 9355ceb Compare January 15, 2019 10:43
@DostalTomas
Copy link
Contributor Author

Thank You for your response.
Test has been added and branch squashed.
I get this issue, when I set content-type: application/json; charset=UTF-8 header and on server I got URL-encoded data. I know, that this is edge case, but it happens.

const reqSettings = {
   url: 'some/url',
   headers: {
      'content-type': 'application/json; charset=UTF-8'
   },
   body: {
      hello: 'world'
   }
}

ajax(reqSettings).subscribe();

@benlesh benlesh merged commit 673bf47 into ReactiveX:master Jan 22, 2019
@benlesh
Copy link
Member

benlesh commented Jan 22, 2019

Thanks, @DostalTomas!

@DostalTomas DostalTomas deleted the fix-http-headers-case-sensitivity branch January 24, 2019 09:44
@DostalTomas
Copy link
Contributor Author

Thank You too ;)

@lock lock bot locked as resolved and limited conversation to collaborators Feb 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants