Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,13 @@ 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 == null
? StandardCharsets.UTF_8
: contentMediaType.charset(StandardCharsets.UTF_8);
requestBody = RequestBody.create(contentMediaType, body.getBytes(charset));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hey99xx I updated this line to include your suggestion for null safety. Just for posterity too, this matches how other parts of the code are handling this case:

https://github.com/facebook/react-native/blob/cda8171af30815edfa331e07d1bbf605f0926303/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java#L562-L563

}
} else if (data.hasKey(REQUEST_BODY_KEY_BASE64)) {
if (contentType == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -346,6 +347,94 @@ 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<Object>() {
@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<Request> 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 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<Object>() {
@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<Request> 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");
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hey99xx I also added this test case to cover the null pointer exception instance on bad content types. It asserts that the content is still parseable as UTF8 and the Content Type header is preserved.


@Test
public void testMultipartPostRequestSimple() throws Exception {
PowerMockito.mockStatic(RequestBodyUtil.class);
Expand Down