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

[🐛] [iOS 17] Get requests failing with error: "invalid json data" #2235

Closed
Jonatthu opened this issue Sep 21, 2023 · 20 comments
Closed

[🐛] [iOS 17] Get requests failing with error: "invalid json data" #2235

Jonatthu opened this issue Sep 21, 2023 · 20 comments
Labels
Bug Something isn't working in the SDK iOS Only Issue that only concern iOS platform

Comments

@Jonatthu
Copy link

Latest stream packages.
iOS Simulator with xCode 15
QueryMembers failed with error: "invalid json data"

It does not fail on real device...
image

@vishalnarkhede
Copy link
Contributor

Are you able to reproduce this consistently on even api call to queryMembers?

@vishalnarkhede
Copy link
Contributor

Also please try to follow the issue template and provide as much information as possible from the template

@Jonatthu
Copy link
Author

Jonatthu commented Sep 22, 2023

@vishalnarkhede many members of my team started to experience this on simulators after upgrading to xcode 15, or iOS17, the funny part is that the same code works when on a device targeting the same environment.

image

@vishalnarkhede
Copy link
Contributor

hmm interesting ... looking into it

@vishalnarkhede
Copy link
Contributor

I am able to reproduce the issue

@jimmyphan
Copy link

This is also happening to me too

image

@joeporpeglia
Copy link

joeporpeglia commented Sep 26, 2023

I don't use the react native package but I'm encountering the same issue using stream-chat in a react native app. These also started for me after upgrading to xcode 15 and ios 17. In the network inspector I can see that some of our requests are successful and one fails. For the failing request, the payload query param appears to be url encoded. Both of these are calling the search method on a channel instance.

Successful:

channel.search({ pinned: true })
// -> payload: {"filter_conditions":{"cid":REMOVED},"message_filter_conditions":{"pinned":true}}

Failing:

channel.search({ 'attachments.type': { $eq: 'poll' } }, { sort: { created_at: -1 } });
// -> payload: %7B%22filter_conditions%22:%7B%22cid%22:%22REMOVED%22%7D,%22sort%22:[%7B%22field%22:%22created_at%22,%22direction%22:-1%7D],%22message_filter_conditions%22:%7B%22attachments.type%22:%7B%22$eq%22:%22poll%22%7D%7D%7D

@vishalnarkhede
Copy link
Contributor

vishalnarkhede commented Sep 30, 2023

After some investigation, it turns out for some reason on iOS 17 simulator, request payload for all the get requests are getting double encoded.

Will let you know more, as the investigation continues.

@vishalnarkhede
Copy link
Contributor

the funny part is that the same code works when on a device targeting the same environment.

I just tried testing the same thing on real device with iOS 17, and the requests are failing there as well.

@vishalnarkhede
Copy link
Contributor

vishalnarkhede commented Oct 2, 2023

This change in js client seems to fix the issue. But I can't say I understand the issue completely yet. So will keep digging

    this.axiosInstance.defaults.paramsSerializer = (params) => {
      const newParams: any = {};
      for (let k in params) {
        if (typeof params[k] === 'object') {
          newParams[k] = JSON.stringify(params[k]);
        } else {
          newParams[k] = params[k];
        }
      }

      const str = qs.stringify(newParams);

      return str;
    }

@vishalnarkhede
Copy link
Contributor

This seems relevant : https://developer.apple.com/documentation/foundation/nsurl/1572047-urlwithstring

For apps linked on or after iOS 17 and aligned OS versions, NSURL parsing has updated from the obsolete RFC 1738/1808 parsing to the same RFC 3986 parsing as 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.

@vishalnarkhede
Copy link
Contributor

@joeporpeglia @Jonatthu whats the version of react-native that you are using?

@vishalnarkhede
Copy link
Contributor

I think this has nothing to do with iOS17 or xcode 15. It could be react-native 0.72 related - facebook/react-native@2be409f

however, this will regress the original issue that this fixed (https://fb.workplace.com/groups/rn.support/posts/25129344753354136), where encoding square brackets in the URI is causing percents to be re-encoded, resulting in a mixed uri with mixed encoding, i.e. %2522%5B%5D. we will be fixing this by not serving decoding brackets in our php serializer which was originally be the follow-up fix, since that does not respect the RFC.

@vishalnarkhede
Copy link
Contributor

I have requested them for an update - facebook/react-native@2be409f#commitcomment-128920983

@joeporpeglia
Copy link

Nice find @vishalnarkhede! I'm running react-native@0.72.5 (expo@49.0.13)

@vishalnarkhede
Copy link
Contributor

vishalnarkhede commented Oct 3, 2023

Summary

Ok so I think the solution is simply to not rely on axios's default encoding since it doesn't respect RFC 3986. This has been brought on axios repository multiple times, but not sure why its not prioritized -

axios/axios#4432

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 parsing has updated from the obsolete RFC 1738/1808 parsing to the same RFC 3986 parsing as 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.

As a result of this change, the parsing algorithm of NSURL has been modified. When it encounters a reserved character, such as [, the parser will percent encode all possible characters in the URL, including %.

This adjustment can pose challenges for URLs that already have some form of encoding applied. For instance, if you have the string %22[], the updated parsing algorithm will yield the following outcomes:

Under RFC 1738/1808: %22%5B%5D
Under RFC 3986: %2522%5B%5D (which is considered an invalid encoding)

React Native tried fixing this issue here - but later they reverted the fix for some other reason:

Solution

  • Override axios's default encoder using paramsSerializer
  • Make sure all the brackets are encoded. One way would be to stringify the object type params
    this.axiosInstance.defaults.paramsSerializer = (params) => {
      const newParams = [];
      for (let k in params) {
        if (Array.isArray(params[k]) || typeof params[k] === 'object') {
          newParams.push(`${k}=${JSON.stringify(params[k])}`);
        } else {
          newParams.push(`${k}=${encodeURIComponent(params[k])}`);
        }
      }
      return newParams.join('&');
    }

I will raise the PR for this and get internal review as soon as possible.

@vishalnarkhede vishalnarkhede added Bug Something isn't working in the SDK iOS Only Issue that only concern iOS platform and removed Needs Triaging labels 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
@vishalnarkhede
Copy link
Contributor

Hey @joeporpeglia @Jonatthu could you please upgrade to 5.18.1-beta.3 and see if the issue is fixed?

@vishalnarkhede
Copy link
Contributor

vishalnarkhede commented Oct 3, 2023

After you install, please make sure version of stream-chat in your .lock file is 8.12.2.

@vishalnarkhede vishalnarkhede changed the title QueryMembers failed with error: "invalid json data" [🐛] QueryMembers failed with error: "invalid json data" Oct 3, 2023
@joeporpeglia
Copy link

Just tried with 8.12.3 and everything looks good. Thanks for the quick fix @vishalnarkhede!

@vishalnarkhede
Copy link
Contributor

Stable release stream-chat-react-native@5.18.1 is ready now.

@vishalnarkhede vishalnarkhede changed the title [🐛] QueryMembers failed with error: "invalid json data" [🐛] [iOS 17] Get requests failing with error: "invalid json data" Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working in the SDK iOS Only Issue that only concern iOS platform
Projects
None yet
Development

No branches or pull requests

4 participants