1
0
mirror of https://github.com/square/okhttp.git synced 2025-11-26 06:43:09 +03:00

Change MediaType's failure mode to not crash on charset problems.

As-is it throws unchecked exceptions on unexpected charsets. This is a problem
because it can cause a misbehaving webserver to crash the client.

I don't expect this to break existing clients; returning 'null' has always
been a possibility; it's just returned in more cases.
This commit is contained in:
jwilson
2017-01-29 13:02:08 -05:00
parent f54130499a
commit 6651a9c15e
4 changed files with 21 additions and 49 deletions

View File

@@ -18,7 +18,6 @@ package okhttp3.logging;
import java.io.EOFException; import java.io.EOFException;
import java.io.IOException; import java.io.IOException;
import java.nio.charset.Charset; import java.nio.charset.Charset;
import java.nio.charset.UnsupportedCharsetException;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import okhttp3.Connection; import okhttp3.Connection;
import okhttp3.Headers; import okhttp3.Headers;
@@ -241,15 +240,7 @@ public final class HttpLoggingInterceptor implements Interceptor {
Charset charset = UTF8; Charset charset = UTF8;
MediaType contentType = responseBody.contentType(); MediaType contentType = responseBody.contentType();
if (contentType != null) { if (contentType != null) {
try {
charset = contentType.charset(UTF8); charset = contentType.charset(UTF8);
} catch (UnsupportedCharsetException e) {
logger.log("");
logger.log("Couldn't decode the response body; charset is likely malformed.");
logger.log("<-- END HTTP");
return response;
}
} }
if (!isPlaintext(buffer)) { if (!isPlaintext(buffer)) {

View File

@@ -563,7 +563,7 @@ public final class HttpLoggingInterceptorTest {
server.enqueue(new MockResponse() server.enqueue(new MockResponse()
.setHeader("Content-Type", "text/html; charset=0") .setHeader("Content-Type", "text/html; charset=0")
.setBody("Ignore This")); .setBody("Body with unknown charset"));
Response response = client.newCall(request().build()).execute(); Response response = client.newCall(request().build()).execute();
response.body().close(); response.body().close();
@@ -578,8 +578,8 @@ public final class HttpLoggingInterceptorTest {
.assertLogEqual("Content-Type: text/html; charset=0") .assertLogEqual("Content-Type: text/html; charset=0")
.assertLogMatch("Content-Length: \\d+") .assertLogMatch("Content-Length: \\d+")
.assertLogMatch("") .assertLogMatch("")
.assertLogEqual("Couldn't decode the response body; charset is likely malformed.") .assertLogEqual("Body with unknown charset")
.assertLogEqual("<-- END HTTP") .assertLogEqual("<-- END HTTP (25-byte body)")
.assertNoMoreLogs(); .assertNoMoreLogs();
applicationLogs applicationLogs
@@ -589,8 +589,8 @@ public final class HttpLoggingInterceptorTest {
.assertLogEqual("Content-Type: text/html; charset=0") .assertLogEqual("Content-Type: text/html; charset=0")
.assertLogMatch("Content-Length: \\d+") .assertLogMatch("Content-Length: \\d+")
.assertLogEqual("") .assertLogEqual("")
.assertLogEqual("Couldn't decode the response body; charset is likely malformed.") .assertLogEqual("Body with unknown charset")
.assertLogEqual("<-- END HTTP") .assertLogEqual("<-- END HTTP (25-byte body)")
.assertNoMoreLogs(); .assertNoMoreLogs();
} }

View File

@@ -17,15 +17,12 @@
package okhttp3; package okhttp3;
import java.nio.charset.Charset; import java.nio.charset.Charset;
import java.nio.charset.IllegalCharsetNameException;
import java.nio.charset.UnsupportedCharsetException;
import okhttp3.internal.Util; import okhttp3.internal.Util;
import org.junit.Test; import org.junit.Test;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
/** /**
* Test MediaType API and parsing. * Test MediaType API and parsing.
@@ -123,29 +120,17 @@ public class MediaTypeTest {
} }
@Test public void testMultipleCharsets() { @Test public void testMultipleCharsets() {
try { assertNull(MediaType.parse("text/plain; charset=utf-8; charset=utf-16"));
MediaType.parse("text/plain; charset=utf-8; charset=utf-16");
fail();
} catch (IllegalArgumentException expected) {
}
} }
@Test public void testIllegalCharsetName() { @Test public void testIllegalCharsetName() {
MediaType mediaType = MediaType.parse("text/plain; charset=\"!@#$%^&*()\""); MediaType mediaType = MediaType.parse("text/plain; charset=\"!@#$%^&*()\"");
try { assertNull(mediaType.charset());
mediaType.charset();
fail();
} catch (IllegalCharsetNameException expected) {
}
} }
@Test public void testUnsupportedCharset() { @Test public void testUnsupportedCharset() {
MediaType mediaType = MediaType.parse("text/plain; charset=utf-wtf"); MediaType mediaType = MediaType.parse("text/plain; charset=utf-wtf");
try { assertNull(mediaType.charset());
mediaType.charset();
fail();
} catch (UnsupportedCharsetException expected) {
}
} }
/** /**
@@ -159,20 +144,12 @@ public class MediaTypeTest {
@Test public void testCharsetNameIsDoubleQuotedAndSingleQuoted() throws Exception { @Test public void testCharsetNameIsDoubleQuotedAndSingleQuoted() throws Exception {
MediaType mediaType = MediaType.parse("text/plain;charset=\"'utf-8'\""); MediaType mediaType = MediaType.parse("text/plain;charset=\"'utf-8'\"");
try { assertNull(mediaType.charset());
mediaType.charset();
fail();
} catch (IllegalCharsetNameException expected) {
}
} }
@Test public void testCharsetNameIsDoubleQuotedSingleQuote() throws Exception { @Test public void testCharsetNameIsDoubleQuotedSingleQuote() throws Exception {
MediaType mediaType = MediaType.parse("text/plain;charset=\"'\""); MediaType mediaType = MediaType.parse("text/plain;charset=\"'\"");
try { assertNull(mediaType.charset());
mediaType.charset();
fail();
} catch (IllegalCharsetNameException expected) {
}
} }
@Test public void testDefaultCharset() throws Exception { @Test public void testDefaultCharset() throws Exception {
@@ -189,7 +166,7 @@ public class MediaTypeTest {
MediaType mediaType = MediaType.parse("text/plain;"); MediaType mediaType = MediaType.parse("text/plain;");
assertEquals("text", mediaType.type()); assertEquals("text", mediaType.type());
assertEquals("plain", mediaType.subtype()); assertEquals("plain", mediaType.subtype());
assertEquals(null, mediaType.charset()); assertNull(mediaType.charset());
assertEquals("text/plain;", mediaType.toString()); assertEquals("text/plain;", mediaType.toString());
} }

View File

@@ -73,7 +73,7 @@ public final class MediaType {
charsetParameter = parameter.group(3); charsetParameter = parameter.group(3);
} }
if (charset != null && !charsetParameter.equalsIgnoreCase(charset)) { if (charset != null && !charsetParameter.equalsIgnoreCase(charset)) {
throw new IllegalArgumentException("Multiple different charsets: " + string); return null; // Multiple different charsets!
} }
charset = charsetParameter; charset = charsetParameter;
} }
@@ -100,15 +100,19 @@ public final class MediaType {
* Returns the charset of this media type, or null if this media type doesn't specify a charset. * Returns the charset of this media type, or null if this media type doesn't specify a charset.
*/ */
public Charset charset() { public Charset charset() {
return charset != null ? Charset.forName(charset) : null; return charset(null);
} }
/** /**
* Returns the charset of this media type, or {@code defaultValue} if this media type doesn't * Returns the charset of this media type, or {@code defaultValue} if either this media type
* specify a charset. * doesn't specify a charset, of it its charset is unsupported by the current runtime.
*/ */
public Charset charset(Charset defaultValue) { public Charset charset(Charset defaultValue) {
try {
return charset != null ? Charset.forName(charset) : defaultValue; return charset != null ? Charset.forName(charset) : defaultValue;
} catch (IllegalArgumentException e) {
return defaultValue; // This charset is invalid or unsupported. Give up.
}
} }
/** /**