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: ignore 'content-type' header capitalization (#1463) #1594

Merged
merged 1 commit into from
May 30, 2021

Conversation

thislooksfun
Copy link
Contributor

Fixes #1463

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts

@benmccann benmccann added the bug Something isn't working label May 30, 2021
@thislooksfun thislooksfun force-pushed the fix/header-capitalization branch from 2ee331d to cbe029e Compare May 30, 2021 16:21
@thislooksfun
Copy link
Contributor Author

thislooksfun commented May 30, 2021

Um... no idea why the tests all failed now. I'm pretty sure that's not my fault.

EDIT: seeing as #1601 also failed in the same way I’m 99% sure it’s a runner failure, and thus not due to this pr.

@Rich-Harris Rich-Harris merged commit c6aa75e into sveltejs:master May 30, 2021
@Rich-Harris
Copy link
Member

Thank you. I made a small change — the headers are already getting lowercased, there's no need to do it again. We can just pass the lowercased headers through to parse_body babe515

1 similar comment
@Rich-Harris
Copy link
Member

Thank you. I made a small change — the headers are already getting lowercased, there's no need to do it again. We can just pass the lowercased headers through to parse_body babe515

@thislooksfun thislooksfun deleted the fix/header-capitalization branch May 30, 2021 21:39
@thislooksfun
Copy link
Contributor Author

Yeah I thought of that but it's way harder to add a test that way. Ah well, balancing these things is hard. Either way I'm glad it's fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SSR is case sensitive for headers (content-type)
3 participants