Skip to content

Commit

Permalink
Use existing character set in POST body when possible (#23603)
Browse files Browse the repository at this point in the history
Summary:
This commit fixes a bug introduced in a previous attempt (#23580) 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:
#8237 (comment)

[Android][fixed] - Respect existing character set when specified in fetch() POST request
Pull Request resolved: #23603

Differential Revision: D14191750

Pulled By: hramos

fbshipit-source-id: 11c1bfd98ccd33cd8e54ea426285b7d2ce9c2d7c
  • Loading branch information
nhunzaker authored and facebook-github-bot committed Feb 23, 2019
1 parent c1349b5 commit 0d5aebb
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 1 deletion.
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));
}
} 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");
}

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

0 comments on commit 0d5aebb

Please sign in to comment.