-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(http): Properly URL-encode key and values during x-www-form-urlencoded
POSTs
#8037
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
base: main
Are you sure you want to change the base?
Conversation
@@ -40,8 +40,11 @@ open class CapacitorUrlRequest: NSObject, URLSessionTaskDelegate { | |||
throw CapacitorUrlRequestError.serializationError("[ data ] argument for request with content-type [ multipart/form-data ] may only be a plain javascript object") | |||
} | |||
|
|||
let allowed = CharacterSet(charactersIn: "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let allowed = CharacterSet(charactersIn: "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._") | |
let allowed = CharacterSet(charactersIn: "-._*").union(.alphanumerics) |
This can be simplified with an union to .alphanumerics
instead of typing them all
Also added *
to match Android behavior, which uses URLEncoder.encode
and according to the docs the *
should remain the same:
The special characters ".", "-", "*", and "_" remain the same.
https://docs.oracle.com/javase/8/docs/api/java/net/URLEncoder.html#encode-java.lang.String-java.lang.String-
I've noticed that this is also encoding the keys, tested and web also encodes the keys, but Android doesn't encode the keys.
Not sure which one should be the correct behavior, but I think all the 3 platforms should behave the same way, so all of them should encode the keys or none should.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing, on web and Android whitespace (
) is converted to +
, while on iOS it's getting converted into %20
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure which one should be the correct behavior, but I think all the 3 platforms should behave the same way, so all of them should encode the keys or none should.
According to MDN, keys being encoded is the correct behavior. However, changing it on Android could be seen as breaking, unless we categorize non-encoded keys as a bug (which I do).
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, it should be considered a bug and should be fixed
x-www-form-urlencoded
POSTsx-www-form-urlencoded
POSTs
x-www-form-urlencoded
POSTsx-www-form-urlencoded
POSTs
This PR fixes keys and values that were not url-encoded in CapacitorHttp on iOS during
application/x-www-form-urlencoded
HTTP POST requests.closes: #7840