Automattic / Gravatar-SDK-iOS

Gravatar SDK is a Swift library that allows you to integrate Gravatar features into your own iOS applications.
https://gravatar.com
Mozilla Public License 2.0
36 stars 1 forks source link

Properly encode literal `+` characters in `URLQueryItem`s #409

Closed andrewdmontgomery closed 1 day ago

andrewdmontgomery commented 3 days ago

Closes #404

This PR is meant to replace #406. See that PR for a detailed discussion of why this PR exists.

Description

This introduces a method for adding URLQueryItem's to a URLComponents object, and encapsulates the encoding of + literal characters.

This is a simplified approach. See #406 for a more comprehensive, and more fragile, solution.

Testing Steps

See #404 for details

wpmobilebot commented 3 days ago
Gravatar UIKit Prototype Build📲 You can test the changes from this Pull Request in Gravatar UIKit Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar UIKit Prototype Build Gravatar UIKit Prototype Build
Build Number1268
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-uikit.prototype-build
Commit2ea22c711dc5fafddc4bd06103f76e5925c17032
App Center BuildGravatar SDK Demo - UIKit #139
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.
wpmobilebot commented 3 days ago
Gravatar SwiftUI Prototype Build📲 You can test the changes from this Pull Request in Gravatar SwiftUI Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar SwiftUI Prototype Build Gravatar SwiftUI Prototype Build
Build Number1268
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-swiftui.prototype-build
Commit2ea22c711dc5fafddc4bd06103f76e5925c17032
App Center BuildGravatar SDK Demo - SwiftUI #138
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.
andrewdmontgomery commented 3 days ago

If we agree that this is a safer approach, I will open a PR to adjust our fix in PocketCasts: https://github.com/Automattic/pocket-casts-ios/pull/1770

As it is now, that fix should not crash. But that approach encodes too much, and will make it possible for a future developer to introduce a change that will result in runtime fatalErrors.

andrewdmontgomery commented 2 days ago

https://github.com/Automattic/Gravatar-SDK-iOS/pull/406#issuecomment-2356457161

Of all the characters that are allowed in a Query String, there is only one character whose meaning is defined by the implementation (us) – the + character's meaning is up to us. It is valid to use it as an encoded space. It is also valid to use it as a literal + character.

It would therefore be a bug for Apple to automatically encode the + character.

It is up to us to decide whether the + character is literally, and if so, to encode it.

Since this is the only character that we need to handle manually (based on our own needs), it is simpler and safer to update the percentEncodedQuery property, to encode + characters, than to bypass the built-in encoding and implement our own.