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(router): url encode file names #9867

Merged
merged 2 commits into from
Jan 19, 2023

Conversation

jenseng
Copy link
Contributor

@jenseng jenseng commented Jan 10, 2023

This is the RR port of remix-run/remix#4473

Bring FormData serialization in line with the spec with respect to File entries in url-encoded payloads: send the name as the value, as is the case with a vanilla <form> submission.

Rework various 400-error tests not to rely on the old behavior, as it's no longer a bad request :)

References: remix-run/remix#4342
Spec: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#convert-to-a-list-of-name-value-pairs

@changeset-bot
Copy link

changeset-bot bot commented Jan 10, 2023

🦋 Changeset detected

Latest commit: 6f885d3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@remix-run/router Patch
react-router Patch
react-router-dom Patch
react-router-dom-v5-compat Patch
react-router-native Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jenseng jenseng marked this pull request as draft January 10, 2023 02:04
@jenseng jenseng marked this pull request as ready for review January 10, 2023 02:47
Copy link
Contributor

@brophdawg11 brophdawg11 left a comment

Choose a reason for hiding this comment

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

Thanks for this! Just minor comments but I think this is otherwise in good shape

This is the RR port of remix-run/remix#4473

Bring FormData serialization in line with the spec with respect to File entries
in url-encoded payloads: send the `name` as the value, as is the case with a
vanilla `<form>` submission.

Rework various 400-error tests not to rely on the old behavior, as it's no longer
a bad request :)

References: remix-run/remix#4342
Spec: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#convert-to-a-list-of-name-value-pairs
Copy link
Contributor Author

@jenseng jenseng left a comment

Choose a reason for hiding this comment

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

Addressed feedback and rebased (errorMessage stuff in router.ts had a conflict with a recent deferred change)

.changeset/sweet-swans-cry.md Outdated Show resolved Hide resolved
packages/router/__tests__/router-test.ts Outdated Show resolved Hide resolved
packages/router/__tests__/router-test.ts Outdated Show resolved Hide resolved
packages/router/__tests__/router-test.ts Outdated Show resolved Hide resolved
packages/router/__tests__/router-test.ts Outdated Show resolved Hide resolved
@brophdawg11 brophdawg11 self-assigned this Jan 17, 2023
@brophdawg11 brophdawg11 merged commit 72f8ef1 into remix-run:dev Jan 19, 2023
@brophdawg11
Copy link
Contributor

Thanks @jenseng!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 6.7.1-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 6.8.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

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.

3 participants