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

Update URLEncodedFormEncoder encoding rules #3192

Merged
merged 6 commits into from May 20, 2024
Merged

Update URLEncodedFormEncoder encoding rules #3192

merged 6 commits into from May 20, 2024

Conversation

ptoffy
Copy link
Member

@ptoffy ptoffy commented May 13, 2024

These changes are now available in 4.100.1

Solves #3173
References https://url.spec.whatwg.org/#application-x-www-form-urlencoded-percent-encode-set to set the encoding rules

The application/x-www-form-urlencoded percent-encode set contains all code points, except the ASCII alphanumeric, U+002A (*), U+002D (-), U+002E (.), and U+005F (_).

@ptoffy ptoffy requested review from 0xTim and gwynne as code owners May 13, 2024 12:52
@ptoffy ptoffy linked an issue May 13, 2024 that may be closed by this pull request
@ptoffy ptoffy requested a review from 0xTim May 13, 2024 17:14
Comment on lines +77 to +78
var allowed = CharacterSet.alphanumerics
allowed.insert(charactersIn: "*-._")
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm honestly not sure why this is allowing [ and ]

Copy link
Member

Choose a reason for hiding this comment

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

Originally because of Vapor's support for "array" URL parameters, but that doesn't matter for this usage.

@ptoffy ptoffy changed the title Percent-encode $ in URLEncodedFormEncoder Update URLEncodedFormEncoder encoding rules May 14, 2024
@0xTim 0xTim added the semver-patch Internal changes only label May 20, 2024
Comment on lines -165 to +169
Date(timeIntervalSince1970: 10_000),
Date(timeIntervalSince1970: 20_000),
Date(timeIntervalSince1970: 30_000),
Date(timeIntervalSince1970: 40_000),
Date(timeIntervalSince1970: 50_000),
Date(timeIntervalSince1970: 10000),
Date(timeIntervalSince1970: 20000),
Date(timeIntervalSince1970: 30000),
Date(timeIntervalSince1970: 40000),
Date(timeIntervalSince1970: 50000),
Copy link
Member

Choose a reason for hiding this comment

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

Honestly, turn down the settings on your code formatter, this is a change for the worse 🤣

@0xTim 0xTim enabled auto-merge (squash) May 20, 2024 16:02
@0xTim 0xTim merged commit 50c6196 into main May 20, 2024
14 checks passed
@0xTim 0xTim deleted the urlencoder-issue branch May 20, 2024 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch Internal changes only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URLFormEncoder does not escape all reserved characters
3 participants