From eb61442245bd2db64d47352a137486d0cb83de45 Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Fri, 22 Feb 2019 07:57:49 -0800 Subject: [PATCH 1/2] Use existing character set in POST body when possible This commit fixes a bug introduced in a previous attempt to address an issue where okhttp appended `charset=utf-8` to the Content-Type header when otherwise not specified. In that commit, I converted all characters to UTF-8, however it should instead use an existing encoding when possible. Related issues: https://github.com/facebook/react-native/issues/8237#issuecomment-466304854 --- .../modules/network/NetworkingModule.java | 6 ++- .../modules/network/NetworkingModuleTest.java | 44 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java index 0bfeb3f1ef44..dd8328d786d8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java @@ -372,7 +372,11 @@ public void onProgress(long bytesWritten, long contentLength, boolean done) { return; } } else { - requestBody = RequestBody.create(contentMediaType, body.getBytes(StandardCharsets.UTF_8)); + // Use getBytes() to convert the body into a byte[], preventing okhttp from + // appending the character set to the Content-Type header when otherwise unspecified + // https://github.com/facebook/react-native/issues/8237 + Charset charset = contentMediaType.charset(StandardCharsets.UTF_8); + requestBody = RequestBody.create(contentMediaType, body.getBytes(charset)); } } else if (data.hasKey(REQUEST_BODY_KEY_BASE64)) { if (contentType == null) { diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java index cb6136e7f074..5b0ec9386184 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java @@ -18,6 +18,7 @@ import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.WritableArray; import com.facebook.react.bridge.WritableMap; +import com.facebook.react.common.StandardCharsets; import com.facebook.react.common.network.OkHttpCallUtil; import com.facebook.react.modules.core.DeviceEventManagerModule.RCTDeviceEventEmitter; @@ -346,6 +347,49 @@ public Object answer(InvocationOnMock invocation) throws Throwable { assertThat(argumentCaptor.getValue().body().contentType().toString()).isEqualTo("application/json"); } + @Test + public void testRespectsExistingCharacterSet() throws Exception { + RCTDeviceEventEmitter emitter = mock(RCTDeviceEventEmitter.class); + ReactApplicationContext context = mock(ReactApplicationContext.class); + when(context.getJSModule(any(Class.class))).thenReturn(emitter); + + OkHttpClient httpClient = mock(OkHttpClient.class); + when(httpClient.newCall(any(Request.class))).thenAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + Call callMock = mock(Call.class); + return callMock; + } + }); + OkHttpClient.Builder clientBuilder = mock(OkHttpClient.Builder.class); + when(clientBuilder.build()).thenReturn(httpClient); + when(httpClient.newBuilder()).thenReturn(clientBuilder); + NetworkingModule networkingModule = new NetworkingModule(context, "", httpClient); + + JavaOnlyMap body = new JavaOnlyMap(); + body.putString("string", "Friðjónsson"); + + mockEvents(); + + networkingModule.sendRequest( + "POST", + "http://somedomain/bar", + 0, + JavaOnlyArray.of(JavaOnlyArray.of("Content-Type", "text/plain; charset=utf-16")), + body, + /* responseType */ "text", + /* useIncrementalUpdates*/ true, + /* timeout */ 0, + /* withCredentials */ false); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); + verify(httpClient).newCall(argumentCaptor.capture()); + + Buffer contentBuffer = new Buffer(); + argumentCaptor.getValue().body().writeTo(contentBuffer); + assertThat(contentBuffer.readString(StandardCharsets.UTF_16)).isEqualTo("Friðjónsson"); + } + @Test public void testMultipartPostRequestSimple() throws Exception { PowerMockito.mockStatic(RequestBodyUtil.class); From a09eb8e24d323abdce96a775798edf7b6b41f098 Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Fri, 22 Feb 2019 13:32:24 -0800 Subject: [PATCH 2/2] Handle invalid content types When okhttp's MediaType class fails to parse an invalid content type, it returns null. This commit adds a guard for this case and adds a test to ensure that the content is still parsable as UTF-8 and the conten type header is preserved. --- .../modules/network/NetworkingModule.java | 4 +- .../modules/network/NetworkingModuleTest.java | 45 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java index dd8328d786d8..209ead5a3968 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java @@ -375,7 +375,9 @@ public void onProgress(long bytesWritten, long contentLength, boolean done) { // Use getBytes() to convert the body into a byte[], preventing okhttp from // appending the character set to the Content-Type header when otherwise unspecified // https://github.com/facebook/react-native/issues/8237 - Charset charset = contentMediaType.charset(StandardCharsets.UTF_8); + Charset charset = contentMediaType == null + ? StandardCharsets.UTF_8 + : contentMediaType.charset(StandardCharsets.UTF_8); requestBody = RequestBody.create(contentMediaType, body.getBytes(charset)); } } else if (data.hasKey(REQUEST_BODY_KEY_BASE64)) { diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java index 5b0ec9386184..cdbcb8908c05 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java @@ -390,6 +390,51 @@ public Object answer(InvocationOnMock invocation) throws Throwable { assertThat(contentBuffer.readString(StandardCharsets.UTF_16)).isEqualTo("Friðjónsson"); } + @Test + public void testGracefullyRecoversFromInvalidContentType() throws Exception { + RCTDeviceEventEmitter emitter = mock(RCTDeviceEventEmitter.class); + ReactApplicationContext context = mock(ReactApplicationContext.class); + when(context.getJSModule(any(Class.class))).thenReturn(emitter); + + OkHttpClient httpClient = mock(OkHttpClient.class); + when(httpClient.newCall(any(Request.class))).thenAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + Call callMock = mock(Call.class); + return callMock; + } + }); + OkHttpClient.Builder clientBuilder = mock(OkHttpClient.Builder.class); + when(clientBuilder.build()).thenReturn(httpClient); + when(httpClient.newBuilder()).thenReturn(clientBuilder); + NetworkingModule networkingModule = new NetworkingModule(context, "", httpClient); + + JavaOnlyMap body = new JavaOnlyMap(); + body.putString("string", "test"); + + mockEvents(); + + networkingModule.sendRequest( + "POST", + "http://somedomain/bar", + 0, + JavaOnlyArray.of(JavaOnlyArray.of("Content-Type", "invalid")), + body, + /* responseType */ "text", + /* useIncrementalUpdates*/ true, + /* timeout */ 0, + /* withCredentials */ false); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); + verify(httpClient).newCall(argumentCaptor.capture()); + + Buffer contentBuffer = new Buffer(); + argumentCaptor.getValue().body().writeTo(contentBuffer); + + assertThat(contentBuffer.readString(StandardCharsets.UTF_8)).isEqualTo("test"); + assertThat(argumentCaptor.getValue().header("Content-Type")).isEqualTo("invalid"); + } + @Test public void testMultipartPostRequestSimple() throws Exception { PowerMockito.mockStatic(RequestBodyUtil.class);