Android 4.4.4 axios JSON.parse(response) error

[wxcchdStar]

Environment

System:
  OS: macOS High Sierra 10.13.3
  CPU: x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
  Memory: 94.82 MB / 16.00 GB
  Shell: 3.2.57 - /bin/bash
Binaries:
  Node: 8.11.4 - ~/.nvm/versions/node/v8.11.4/bin/node
  npm: 5.6.0 - ~/.nvm/versions/node/v8.11.4/bin/npm
  Watchman: 4.9.0 - /usr/local/bin/watchman
SDKs:
  iOS SDK:
    Platforms: iOS 11.4, macOS 10.13, tvOS 11.4, watchOS 4.3
  Android SDK:
    Build Tools: 19.1.0, 20.0.0, 21.1.1, 21.1.2, 22.0.0, 22.0.1, 23.0.0, 23.0.1, 23.0.2, 23.0.3, 24.0.0, 24.0.1, 24.0.2, 24.0.3, 25.0.0, 25.0.1, 25.0.2, 25.0.3, 26.0.0, 26.0.1, 26.0.2, 26.0.3, 27.0.1, 27.0.2, 27.0.3, 28.0.2
    API Levels: 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27
IDEs:
  Android Studio: 3.1 AI-173.4907809
  Xcode: 9.4.1/9F2000 - /usr/bin/xcodebuild
npmPackages:
  react: 16.4.1 => 16.4.1
  react-native: 0.56.0 => 0.56.0
npmGlobalPackages:
  react-native-git-upgrade: 0.2.7

Description

On Android 4.4.4，when response body is too big(~20kb), response body will cut the content and JSON.parse(response) will occur the SyntaxError: Unexpected token i in JSON at position. I print the request body, and this content like this: {"id":1,"name":""isRequired":1}, the content after name lose the value",. The complete content should be {"id":1,"name":"value","isRequired":1}.

I also set maxContentLength: Number.MAX_VALUE, but it's not work.

I using react native v0.56.0 and axios v0.18.0.

I also tried Fetch and it's work.

[react-native-bot]

Can you run react-native info and edit your issue to include these results under the Environment section?

If you believe this information is irrelevant to the reported issue, you may write [skip envinfo] under Environment to let us know.

[MarkDaleman]

Hallo! I've tried recreating this problem in React-Native version 0.56. I'm also using Axios version 0.18.0.
I've made a request with a body of: 37.62 KB and header size headers: 450 B.

I'm using an Android Simulator running 4.4.2 and the request works fine, and the application doesn't crash

Does this problem also happen to you on 4.4.2? If so, maybe try updating React-Native to version 0.56?

[gengjiawen]

Valid json key is string, so id should be "id", maybe it's the problem. Can you check this ?

[react-native-bot]

It looks like you are using an older version of React Native. Please update to the latest release, v0.56 and verify if the issue still exists.

The ":rewind:Old Version" label will be removed automatically once you edit your original post with the results of running react-native info on a project using the latest release.

[wxcchdStar]

@gengjiawen，my mistake，but here is an example. So it's not the real reason.

[wxcchdStar]

@MarkDaleman，i tried updating React-Native to version 0.56, but the problem still remains. Maybe you can make the response body bigger, like 100kb?

[MarkDaleman]

Hello! I've made a simple demo here. I'm using JSON data with a response body size of 148.97KB as you can see in the image.

I've also uploaded my demo to github, maybe you can clone it and see if it still happens?
https://github.com/MarkDaleman/AxiosTest

[gengjiawen]

@wxcchdStar You need to provide an repro to find the issue.

[wxcchdStar]

@gengjiawen @MarkDaleman , I provide a repo to reappear the issue: https://github.com/wxcchdStar/AxiosTest。 It's work in Postman, but not work in this repo.

[gengjiawen]

The final request url is https://testapi.estate.ttonservice.com/v2/test/task/list?page=1&size=20, I reproduce it on android 4.4 using 0.57.rc4 .Android 7.0 is fine.
I also tracked the network using charles, the actual response is complete.

[gengjiawen]

Also tried upgrade to okhttp 3.11.0, the problem still exists.

[gengjiawen]

I tried fetch, and it's not working too, the json returned is as expected. What do you mean it works @wxcchdStar , do you see correct json returned ?

[gengjiawen]

An expo snack repro: https://snack.expo.io/@gengjiawen/fetch-bug .
Run android version you can see the wrong output.

[wxcchdStar]

@gengjiawen, yes, it's work above Android 5.0 and not work below Android 4.4.4.

I tried your expo snack repro and the resp.json() on Android 4.4.4 return this, so the Fetch is work for me:

[gengjiawen]

What's _40 and _55 in json ?

[wxcchdStar]

What's _40 and _55 in json ?

@gengjiawen, it's a part of Promise construct. resp.json() return Promise not JSON. So you can resp.json().then(data=>console.log(data)) for print data

[jesenko]

I have also noticed sporadic malformed JSON responses on Android <4.4. The issue appears to be in ProgressiveStringDecoder which is used to decode full JSON response from partial byte arrays. Issue is due to apparent bug in Java versions used in Android <4.4, namely misbehaviour of CharsetDecoder, which does not raise on invalid input. This code should determine how many bytes should be trimmed and passed to subsequent byte array, however in Android < 4.4 CharacterCodingException is not thrown, and thus decoding of next byte array fails.

I have found no related bug report for CharsetDecoder or mention of changed behaviour, however, following code demonstrates difference in Android <4.4 and 5+:

byte[] data = new byte[] { 79, -59 };
ByteBuffer decodeBuffer = ByteBuffer.wrap(data, 0, 2);
CharsetDecoder decoder = Charset.forName("UTF-8").newDecoder();
decoder.decode(decodeBuffer);
 

On Android <4.4, this code decodes to "O", with no exception being thrown (NOT OK!)
On Android 5+, this code raises MalformedInputException (OK).

This issue affects only older devices, however it leads to sporadic failures that are very hard to debug. Maybe disabling progressive requests on older devices would be enough; alternatively, some alternative method of decoding in ProgressiveStringDecoder might be better - current logic of trying to decode whole buffer multiple times to determine whether it's last 4 bytes are ok seems a bit redundant...

[jesenko]

cc @dryganets

[jesenko]

Also note that this issue is manifested only when incremental networking is enabled, e.g. when certain listeners are set on XMLHttpRequest. In our case this was due to some instrumentation library extending default XMLHttpRequest, thus incremental networking was triggered implicitly on all requests. Avoiding incremental networking provides a quick workaround...

[codebymikey]

Hi @dryganets @lexs

Ran into the same issue myself, and had to do some deep dive to find out what was wrong.
This breaks some of my network calls without giving back any indicator that the original network response has been ignored.

The regression bug can be recreated using this sample code:

var request = new XMLHttpRequest();
request.onreadystatechange = (e) => {
  if (request.readyState !== 4) {
    return;
  }
  if (request.status === 200) {
    // This returns an empty string rather than the actual response.
    console.log('success', request.responseText);
  } else {
    console.warn('error');
  }
};
request.open('GET', 'https://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt');
request.send();

Following on from the #15295 fix, it assumes the invalid byte is at the end of the input rather than somewhere in the middle.

The ProgressiveStringDecoder::decodeNext needs to catch the index of the invalid bytes and probably skip them (or apply whatever the preferred behaviour should be).

Further discussion might be needed on this.

Does anyone know the most relevant active contributor to pick this up with?

[tabfed]

me too

[adnkh]

I have the same problem, It is happening also in some Huawei devices with Android 8.
Please have you found any workaround for this bug.

I got this in the Logcat

W/unknown:ReactNative: failed to decode string from byte array
from ProgressiveStringDecoder

[dryganets]

The initial version of my PR supported UTF-8 encoding only, but I was asked to make a solution similar to iOS.
The progressive download is not something that is enabled by default. If I remember correctly you could avoid it by not subscribing to the onprogress and readystate change events of XmlHttpRequest:

if (type === 'readystatechange' || type === 'progress') {
      this._incrementalEvents = true;
    }

The code which might help to fix the problem on 4.4 UTF-8 is listed below.
The downside of this solution that support of UTF-16 and UTF-32 needs to be added separately also for UTF-16 and 32 BOM mark makes difference and the solution I contributed at the end takes care of that.
But it might work as a workaround of 4.4 issues.

/**
* Copyright (c) 2017-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/
package com.facebook.react.modules.network;

import com.facebook.react.common.StandardCharsets;

/**
* Class to decode UTF-8 strings from byte array chunks.
* UTF-8 could have symbol size from 1 to 4 bytes.
* In case of progressive decoding we could accidentally break the original string.
*
* Use this class to make sure that we extract Strings from byte stream correctly.
*/
public class ProgressiveUTF8StreamDecoder {

  private byte[] mRemainder = null;

  /**
   * Bit mask implementation performed 1.5x worse than this one
   *
   * @param firstByte - first byte of the symbol
   * @return count of bytes in the symbol
   */
  private int symbolSize(byte firstByte) {
    int code = firstByte & 0XFF;
    if (code >= 240) {
        return 4;
    } else if (code >= 224 ) {
        return 3;
    } else if (code >= 192 ) {
        return 2;
    }

    return 1;
  }

  /**
   * Parses data to UTF-8 String
   * If last symbol is partial we save it to mRemainder and concatenate it to the next chunk
   * @param data
   * @param length length of data to decode
   * @return
   */
  public String decodeNext(byte[] data, int length) {
    int i = 0;
    int lastSymbolSize = 0;
    if (mRemainder != null) {
      i = symbolSize(mRemainder[0]) - mRemainder.length;
    }
    while (i < length) {
      lastSymbolSize = symbolSize(data[i]);
      i += lastSymbolSize;

    }

    byte[] result;
    int symbolsToCopy = length;
    boolean hasNewReminder = false;
    if (i > length) {
      hasNewReminder = true;
      symbolsToCopy = i - lastSymbolSize;
    }

    if (mRemainder == null) {
      result = data;
    } else {
      result = new byte[symbolsToCopy + mRemainder.length];
      System.arraycopy(mRemainder, 0, result, 0, mRemainder.length);
      System.arraycopy(data, 0, result, mRemainder.length, symbolsToCopy);
      mRemainder = null;
      symbolsToCopy = result.length;
    }

    if (hasNewReminder) {
      int reminderSize =  lastSymbolSize - i + length;
      mRemainder = new byte[reminderSize];
      System.arraycopy(data, length - reminderSize, mRemainder, 0, reminderSize );
    }

    return new String(result, 0, symbolsToCopy, StandardCharsets.UTF_8);
  }
}

[TareqElMasriDev]

@dryganets I have this issue for months, when is this gonna be merged? Or how can I implement this fix in my project?

[dryganets]

The issue is hard to reproduce and test. Our app also not run on 4.4, so I unlikely will be able to help with testing/fixing it. Without my original fix all Android versions crashing because of the way how imporoperly encoded strings are handled inside of the JSC and that issue is way harder to debug/workaround than this one.

I would suggest you run the solution I posted on one of the affected Android devices, you could use the unit test from my original PR to run it.
Also, it would be great to find the string 4.4 devices choke on and add it to the tests.

As I said the previous version of the code is not comprehensive as it has UTF-8 support only.
ie if you are using UTF-16 for your servers by some reason you need to implement proper UTF-16 support, though UTF-16 is easier than UTF-8 as you might have only 1 extra character.

In case of UTF-16LE you will need to swap the last two bytes from the byte array (default java byte order in BigEndian), convert them to char and check with Character.isLowerSurrogate(ch) if it is lower you can't split byte array here, instead you need to add two last bytes to the remainder and join with the next bytearray.

I don't think anyone uses UTF-32 on serverside in practice, so it might be good enough.

[TareqElMasriDev]

@dryganets can you please link me your PR so I can test it?

[dryganets]

@TareqElMasri, I had no PR that fixes your problem and my original PR doesn't have the old code anymore.
I took a moment today to implement the fix you guys asking for in this thread.
If it helps I could make a PR.
The fix is in this branch:
https://github.com/dryganets/react-native/tree/sergeyd/android-progressive-decoding-kitkat-compat

I also fixed the IDE support for UnitTests for your convenience.

ProgressiveStringDecoderTest will be a good place to reproduce the original KitKat issue.
ProgressiveUTF8StringDecoderTest testing the class used for KitKat.

So, you basically need to have a data sample which fails the ProgressiveStringDecoderTest and works with ProgressiveUTF8StringDecoderTest on KitKat to verify that fix is working.

[biomancer]

@dryganets thank you, this issue makes our app useless on Android 4.4, and cherry-picking your commits on top of 0.58 stable branch fixed it (compared to unmodified 0.58.6).
Will have to use a fork until this issue is fixed by merging your changes or somehow else.
Could you please consider creating PR with these fixes to initiate review and further discussion?

[elicwhite]

@dryganets it would be helpful if you could send a PR for this, yeah.

[dryganets]

@TheSavior, here you go:
#24038

[megabayt]

@dryganets
I've created a patch from your PR, applied it to my node_modules/react-native dir and nothing changed. I'm still having troubles with truncated response. How can I check that I'm applied your PR correctly?

[dryganets]

@megabayt,

Do you build react-native from source?
ie does your app/build.gradle has
implementation project(':ReactAndroid')
instead of
implementation 'react-native:*'

[stale]

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

[stale]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.