facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
119.28k stars 24.35k forks source link

Fetch adds charset=utf-8 on Android but not on iOS #8237

Closed dstaver closed 5 years ago

dstaver commented 8 years ago

This fetch request sends "application/json" as ContentType on IOS but "application/json; charset=utf-8" on Android:

fetch(url, {
    method: 'POST',
    headers: {
        'Accept': 'application/json',
        'Content-Type': 'application/json',
    }
})

The server I'm working against requires application/json and fails when charset is added. I've filed a bug report with them too, but shouldn't fetch behave identically on ios and android?

dstaver commented 8 years ago

http://stackoverflow.com/questions/25560601/how-to-suppress-charset-being-automatically-added-to-content-type-in-okhttp

etodanik commented 8 years ago

I'm also experiencing this. Having different headers between platforms doesn't make sense and introduces bugs

lacker commented 8 years ago

What's the right thing to do here - should the charset be appended or not?

himelnagrana commented 7 years ago

Having the same issue while working with Urban Airship APIs. Frustrating as UA supposed to be not changing at their end. What to do?

Negan1911 commented 7 years ago

Im also seeing this

ide commented 7 years ago

What's the right thing to do here - should the charset be appended or not?

Looking at Chrome, it doesn't modify the content-type header at all so we shouldn't be adding a charset.

alterx commented 7 years ago

I'm also experiencing this issue, somehow charset=utf-8 is added as a parameter to the Content-Type header. It only happens on Android. Also, not using fetch but the XMLHttpRequest polyfill that comes with ReactNative by default.

In my specific case, I'm interacting with a JSON API (http://jsonapi.org/format/) and adding media type parameters causes the server to respond with a 415 error every time. This pretty much breaks the app :(

Edit:

I've debugged deep down into react-native/Libraries/BatchedBridge/NativeModules.js:


function genMethod(moduleID: number, methodID: number, type: MethodType) {
  let fn = null;
  if (type === 'promise') {
    fn = function(...args: Array<any>) {
      return new Promise((resolve, reject) => {
        BatchedBridge.enqueueNativeCall(moduleID, methodID, args,
          (data) => resolve(data),
          (errorData) => reject(createErrorFromErrorData(errorData)));
      });
    };
  } else if (type === 'sync') {
    fn = function(...args: Array<any>) {
      return global.nativeCallSyncHook(moduleID, methodID, args);
    };
  } else {
    fn = function(...args: Array<any>) {
      const lastArg = args.length > 0 ? args[args.length - 1] : null;
      const secondLastArg = args.length > 1 ? args[args.length - 2] : null;
      const hasSuccessCallback = typeof lastArg === 'function';
      const hasErrorCallback = typeof secondLastArg === 'function';
      hasErrorCallback && invariant(
        hasSuccessCallback,
        'Cannot have a non-function arg after a function arg.'
      );
      const onSuccess = hasSuccessCallback ? lastArg : null;
      const onFail = hasErrorCallback ? secondLastArg : null;
      const callbackCount = hasSuccessCallback + hasErrorCallback;
      args = args.slice(0, args.length - callbackCount);
      BatchedBridge.enqueueNativeCall(moduleID, methodID, args, onFail, onSuccess);
    };
  }
  fn.type = type;
  return fn;
}

and, even here the Content-Typeheader remains untouched what leads me to think that is the native code in Android what's adding the charset=utf-8

laurent22 commented 7 years ago

I'm also having this problem and, while doing some testing, I've tried setting an invalid charset "dropbox-cors-hack" (as I'm testing the Dropbox API) so the request was like this:

apiRequest = request.post(getBaseURL(host) + path)
  //.type('application/octet-stream;')
  .type('text/plain; charset=dropbox-cors-hack')
  .set('Authorization', 'Bearer ' + accessToken)
  .set('Dropbox-API-Arg', httpHeaderSafeJson(args));

When doing so I got the following exception:

And that lead me to this file from the okhttp library:

https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/RequestBody.java#L53

So it seems it appends "charset=utf-8" if the charset is not set. I'm not sure if this is React-Native that is calling this lib or if it's Android, but is there anything that can be done to prevent this behaviour?

laurent22 commented 7 years ago

Based on this comment it doesn't look like it will be fixed on the okhttp side:

https://github.com/square/okhttp/issues/3081

They suggest using the create overload that takes a byte[] parameter. Can it be done at the React-Native level?

Knight704 commented 7 years ago

@laurent22 you could customize okhttp client on android that is used under the hood to implement fetch. You can add interceptor to this client and modify any header before it is being processed. Unfortunately, current version of react-native doesn't give you an easy way to provide custom okhttp client, but here I posted a way to workaround this.

hey99xx commented 7 years ago

@Knight704 our discussion in other thread was about keeping a single OkHttp instance. I don't understand how that helps to fix this behavior in OkHttp. Are you overriding entire NetworkingModule to call RequestBody.create with (MediaType, byte[]) arguments? Otherwise did you figure out a way to override the OkHttp default utf-8 charset append behavior, because it's a static method that adds the charset type?

Knight704 commented 7 years ago

@hey99xx the idea boils down to put your custom interceptor to okhttpclient. Yes, okhttp will continue appending charset into header, but inside interceptor you can modify any headers before it really goes to the network, i.e cutting down charset part. This is of course a workaround, but it'll work. Another point that okhttp isn't easy to access right now, so I provided link to solution to that problem as well

hey99xx commented 7 years ago

@Knight704 How would you know if the "Content-type" is automatically set to have the charset part by OkHttp, and not by manually adding the header during fetch call? I mean it's an acceptable solution, but it comes with few questions. Ideally it should be fixed in one of react-native or OkHttp so we happily keep developing our apps.

On my end, I've managed to fix the issue by contacting the backend people to be tolerant to the charset header, but I don't think that's doable when you contact external APIs such as AWS and Dropbox.

Knight704 commented 7 years ago

@hey99xx yeah, I've already mentioned that this is just a hack, and preferably it should be fixed in SDK. Nevertheless, if you are developing your app you should know endpoints where charset should (or shoudn't) be added and may have black/white list that is checked inside interceptor, there are many ways for this, the first that came to my mind was to put your custom header like 'no-charset-append' on js side and cut down this appending if this header exists on native side (of course removing this 'hack' header as well). Ugly, but flexible and would work

jeremy8883 commented 7 years ago

For anyone wanting a copy/paste example, follow the steps outlined here (thanks heaps for that by the way @Knight704), then you can copy my version of NetworkingModuleUtils:

package com.facebook.react.modules.network;

import com.facebook.react.bridge.ReactApplicationContext;

import java.io.IOException;

import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;

public class NetworkingModuleUtils {
    public static NetworkingModule createNetworkingModuleWithCustomClient(ReactApplicationContext context) {
        OkHttpClient client = OkHttpClientProvider.createClient();

        OkHttpClient customClient = client.newBuilder()
            .addNetworkInterceptor(new Interceptor() {
                @Override
                public Response intercept(Chain chain) throws IOException {
                    Request request = chain.request();
                    String contentType = request.header("Content-Type");
                    Request customRequest = "application/json; charset=utf-8".equals(contentType) ?
                        request.newBuilder()
                            .header("content-type", "application/json")
                            .build() : request;

                    Response response = chain.proceed(customRequest);
                    return response;
                }
            }).build();

        return new NetworkingModule(context, null, customClient);
    }
}
Kottidev commented 7 years ago

@jeremy8883, in which directory i copy and past your code ? in src/java/com/ ?

your class resolve the problem for appending "charset=utf-8" to content-type ?

jeremy8883 commented 7 years ago

in which directory i copy and past your code ? in src/java/com/ ?

Yeah, plus the package location, so src/main/java/com/facebook/react/modules/network/NetworkModuleUtils.java. Make sure you also follow the steps in this link.

your class resolve the problem for appending "charset=utf-8" to content-type ?

That's correct

ceefour commented 6 years ago

I use Content-Type: image/jpeg and it becomes Content-Type: image/jpeg; charset=utf-8 which obviously doesn't make sense. :(

I agree with @laurent22 , can we change Fetch implementation to use okhttp's byte[] overload? Fetch API should be consistent on all platforms, not to mention that automatically adding charset breaks various stuff.

Note: @Knight704 's hack won't work with CRNA/Expo due to unable to access Android code

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

laurent22 commented 6 years ago

Please don't close, this is still an issue in latest RN

nateReiners commented 6 years ago

+1 still an issue with rn 0.50.3, but if I am debugging js remotely AND enable network inspect, it works.

etodanik commented 6 years ago

This still breaks some requests, please don't close it!

alien3d commented 6 years ago

react-native version :0.53

03-03 10:14:35.084  8880  9189 E UncaughtException: 
03-03 10:14:35.084  8880  9189 E UncaughtException: java.nio.charset.UnsupportedCharsetException: utf8mb4

It disappear after removing from

Content-Type: application/json; charset=utf8mb4

to

Content-Type: application/json; 

** this solution a bit unpracticle. since sometimes we don;t control server output.

stale[bot] commented 6 years ago

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 "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

etodanik commented 6 years ago

This is still an issue in lates RN , please don’t close On Sun, 17 Jun 2018 at 5:37 stale[bot] notifications@github.com wrote:

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 "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/facebook/react-native/issues/8237#issuecomment-397850473, or mute the thread https://github.com/notifications/unsubscribe-auth/AB4Q-EzEsJzM5KqNZBsVTbJAIO6sqFAeks5t9cDkgaJpZM4I5aJ7 .

archer56 commented 6 years ago

+1

limbuster commented 6 years ago

This is still an issue

programadornatal commented 6 years ago

To resolve this problem, use lib axios https://github.com/qiangmao/axios

jeremy8883 commented 6 years ago

Axios is a javascript library. The problem is in the RN/okhttp java implementation.

programadornatal commented 6 years ago

Axios work for me in IOS

ceefour commented 6 years ago

@programadornatal iOS doesn't have this bug even using pure fetch, without using axios. This is Android specific.

glenndebacker commented 5 years ago

Axios has the same problem on Android. There is a topic describing the same problem but the solution of changing the default header does not seem to work. Android (or RN) seems to be modifying it on a lower level.

nhunzaker commented 5 years ago

This seemed like a fun one to dig into, so I proposed a fix here: https://github.com/facebook/react-native/pull/23580

hey99xx commented 5 years ago

@nhunzaker I know I approved your PR but just looking again, I wanted to clarify this. I cannot ask on the PR itself since it's locked already. You had mentioned

I wondered about that, or passing in the character set as defined by MediaType.parse.

Do you still think it'd be better? One use case that came to my mind is setting a custom content-type header like: Content-Type: text/plain; charset=utf-16.

In that case OkHttp's RequestBody.create would have likely used utf-16 charset to get the bytes. Now after this change we'd be sending bytes encoded with utf-8, but the charset in the header does not match, potentially confusing the servers.

I really don't know what the correct implementation is supposed to be. Maybe only default to utf-8 is MediaType is null or charset-less? That's what OkHttp internally does.

hey99xx commented 5 years ago

Example of how this failure could be> I've never written a raw http server though so cant tell if this is how it works:

String s = "Friðjónsson";
Charset UTF_8 = StandardCharsets.UTF_8;
Charset UTF_16 = StandardCharsets.UTF_16;
System.out.println("original: " + s);
System.out.println("utf8: " + new String(s.getBytes(UTF_8), UTF_8));
System.out.println("utf16: " + new String(s.getBytes(UTF_16), UTF_16));
System.out.println("utf8/16: " + new String(s.getBytes(UTF_8), UTF_16));

output is:

original: Friðjónsson
utf8: Friðjónsson
utf16: Friðjónsson
utf8/16: 䙲槃끪쎳湳獯�
nhunzaker commented 5 years ago

@hey99xx you are correct. my implementation needs to respect the existing character set. I have a test case that reproduces the issue, I'll send out another PR. The code I changed needs to really be something like:

Charset charset = contentMediaType.charset();

if (charset == null) {
  charset = StandardCharsets.UTF_8;
}

requestBody = RequestBody.create(contentMediaType, body.getBytes(charset));
hey99xx commented 5 years ago

The fixes are in 0.59.0-rc.3 which is just released. Can someone verify the problem is gone and we can then resolve this issue?

bartolkaruza commented 5 years ago

Confirmed that this works on RN 0.59.1!