-
-
Notifications
You must be signed in to change notification settings - Fork 11k
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
[Feature Request] Support RFC 7230 and RFC 3986 by Axios configuration #4432
Comments
This was referenced Oct 3, 2023
vishalnarkhede
added a commit
to GetStream/stream-chat-js
that referenced
this issue
Oct 3, 2023
## Issue **Fixes**: GetStream/stream-chat-react-native#2235 On iOS 17, all the `get` requests which involve object or array in url params (e.g. `queryMembers`) are failing. In iOS 17, NSURLs are now encoded according to RFC 3986 standards (as specified in https://www.ietf.org/rfc/rfc3986.txt), whereas they used to adhere to RFC 1738/1808 standards in earlier versions. reference: https://developer.apple.com/documentation/foundation/nsurl/1572047-urlwithstring > For apps linked on or after iOS 17 and aligned OS versions, [NSURL](https://developer.apple.com/documentation/foundation/nsurl) parsing has updated from the obsolete RFC 1738/1808 parsing to the same [RFC 3986](https://www.ietf.org/rfc/rfc3986.txt) parsing as [NSURLComponents](https://developer.apple.com/documentation/foundation/nsurlcomponents). This unifies the parsing behaviors of the NSURL and NSURLComponents APIs. Now, NSURL automatically percent- and IDNA-encodes invalid characters to help create a valid URL. And axios on the other hand doesn't adhere to RFC 3986 - it doesn't encode brackets such as `[`, `{` etc ([source](https://github.com/axios/axios/blob/v1.x/lib/helpers/buildURL.js#L20)). As a result of this, whenever `NSUrl` encounters a reserved character, such as `[`, the parser will percent encode all possible characters in the URL, including %. And this results into double encoded url, which doesn't pass the validation on Stream backend. E.g., ``` payload=%257B%2522type%2522:%2522messaging%2522,%2522id%2522:%2522campaign-test-channel-0%2522,%2522sort%2522:%5B%5D,%2522filter_conditions%2522:%257B%2522name%2522:%2522Robert%2522%257D%257D ``` And this is a known issue with axios - axios/axios#4432 React Native tried handling this issue here - but later they reverted the fix for some other reason: - facebook/react-native@9841bd8 - reverted facebook/react-native@2be409f ## Solution So we need to override default param serialization of axios, and make sure that the url param string is RFC 3986 compliant - if param is object or array, simply stringify it and then encode it. - for the rest, do a normal uri encoding
please open the PR as mentioned if you think this is still relevant |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
Tomcat 7.0.73 add the RFC 7230 and RFC 3986 to resolve the HTTP path.
But RFC 3986 add the reserved characters in server side, just like ":", "[" and "]". If axios use the path with the query like
The http request will be broken.
I search the axios source code, and then found /lib/helpers/buildURL.js( function encode(val) ) do this work.
Describe the solution you'd like
I think
config.paramsSerializer
is a good idea to resolve the special words.But in this way, I should rewrite the
paramsSerializer
function in business code. Copy and paste the axios buildURL.js code, and set theconfig.paramsSerializer
in global configuration.In my opinion, axios should support RFCs and function
paramsSerializer
doesn't seem very graceful.So can we add a switcher to enable or disable encoding all reserved characters? And let any server, which using RFC-7230 and RFC-3986, work well? Just like these code below.
https://github.com/axios/axios/blob/master/lib/helpers/buildURL.js#L41
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
If you think the solution is fine, I will commit this feature for axios. 😄
The text was updated successfully, but these errors were encountered: