Skip to content

Add percentEncodedQueryItems #3096

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

Merged
merged 2 commits into from
Oct 26, 2021

Conversation

lha
Copy link

@lha lha commented Oct 20, 2021

Some code with queryItems, use existing code in CFNetwork

Fixes SR-12340

This seems to have been attempted a couple of times before (#2929, #2940) by @rauhul

Some code with queryItems, use existing code in CFNetwork

Fixes SR-12340
Copy link
Contributor

@millenomi millenomi left a comment

Choose a reason for hiding this comment

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

Patch is fine with a single issue, but code looks good if it passes tests.

/// Returns an array of query items for this `URLComponents`, in the order in which they appear in the original query string. Any percent-encoding in a query item name or value is retained
///
/// The setter combines an array containing any number of `URLQueryItem`s, each of which represents a single key-value pair, into a query string and sets the `URLComponents` query property. This property assumes the query item names and values are already correctly percent-encoded, and that the query item names do not contain the query item delimiter characters '&' and '='. Attempting to set an incorrectly percent-encoded query item or a query item name with the query item delimiter characters '&' and '=' will cause a `fatalError`.
@available(macOS 10.13, iOS 11.0, tvOS 11.0, watchOS 4.0, *)
Copy link
Contributor

Choose a reason for hiding this comment

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

No availability markers in swift-corelibs-foundation, please.

@millenomi
Copy link
Contributor

@swift-ci please test

@millenomi
Copy link
Contributor

@lha Please ping me when this is fixed and I'll force merge since tests all pass.

@lha
Copy link
Author

lha commented Oct 25, 2021

@lha Please ping me when this is fixed and I'll force merge since tests all pass.

branch updated with @available removed @millenomi

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.

2 participants