Skip to content

Commit

Permalink
Fix 2 bugs in MediaType parameter handling:
Browse files Browse the repository at this point in the history 
1. Make empty parameter values serialize to a quoted string.
2. Require parameter values to be ASCII.

Fixes #3626

Relnotes:
  Made `MediaType` serialize empty parameter values to quoted strings.
  Made `MediaType` reject non-ASCII parameter values.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=272190935
  • Loading branch information
@cpovirk
cpovirk committed Oct 1, 2019
1 parent afd127c commit 2278123
Show file tree
Hide file tree
Showing 5 changed files with 206 additions and 8 deletions.
77 changes: 76 additions & 1 deletion android/guava-tests/test/com/google/common/net/MediaTypeTest.java
View file
Expand Up @@ -148,6 +148,22 @@ public void testCreate_wildcardTypeDeclaredSubtype() {
}
}

public void testCreate_nonAsciiParameter() {
try {
MediaType.create("…", "a");
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testCreate_nonAsciiParameterValue() {
try {
MediaType.create("a", "…");
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testCreateApplicationType() {
MediaType newType = MediaType.createApplicationType("yams");
assertEquals("application", newType.type());
Expand Down Expand Up @@ -225,6 +241,26 @@ public void testWithParameters_invalidAttribute() {
}
}

public void testWithParameters_nonAsciiParameter() {
MediaType mediaType = MediaType.parse("text/plain");
ImmutableListMultimap<String, String> parameters = ImmutableListMultimap.of("…", "a");
try {
mediaType.withParameters(parameters);
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParameters_nonAsciiParameterValue() {
MediaType mediaType = MediaType.parse("text/plain");
ImmutableListMultimap<String, String> parameters = ImmutableListMultimap.of("a", "…");
try {
mediaType.withParameters(parameters);
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParameter() {
assertEquals(
MediaType.parse("text/plain; a=1"), MediaType.parse("text/plain").withParameter("a", "1"));
Expand All @@ -248,6 +284,24 @@ public void testWithParameter_invalidAttribute() {
}
}

public void testWithParameter_nonAsciiParameter() {
MediaType mediaType = MediaType.parse("text/plain");
try {
mediaType.withParameter("…", "a");
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParameter_nonAsciiParameterValue() {
MediaType mediaType = MediaType.parse("text/plain");
try {
mediaType.withParameter("a", "…");
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParametersIterable() {
assertEquals(
MediaType.parse("text/plain"),
Expand Down Expand Up @@ -275,6 +329,24 @@ public void testWithParametersIterable_invalidAttribute() {
}
}

public void testWithParametersIterable_nonAsciiParameter() {
MediaType mediaType = MediaType.parse("text/plain");
try {
mediaType.withParameters("…", ImmutableSet.of("a"));
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParametersIterable_nonAsciiParameterValue() {
MediaType mediaType = MediaType.parse("text/plain");
try {
mediaType.withParameters("a", ImmutableSet.of("…"));
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParametersIterable_nullValue() {
MediaType mediaType = MediaType.parse("text/plain");
try {
Expand Down Expand Up @@ -507,10 +579,13 @@ public void testNullPointer() {
public void testToString() {
assertEquals("text/plain", MediaType.create("text", "plain").toString());
assertEquals(
"text/plain; something=\"cr@zy\"; something-else=\"crazy with spaces\"",
"text/plain; something=\"cr@zy\"; something-else=\"crazy with spaces\";"
+ " and-another-thing=\"\"; normal-thing=foo",
MediaType.create("text", "plain")
.withParameter("something", "cr@zy")
.withParameter("something-else", "crazy with spaces")
.withParameter("and-another-thing", "")
.withParameter("normal-thing", "foo")
.toString());
}
}
10 changes: 7 additions & 3 deletions android/guava/src/com/google/common/net/MediaType.java
View file
Expand Up @@ -55,8 +55,8 @@
* type or subtype value. A media type may not have wildcard type with a declared subtype. The
* {@code *} character has no special meaning as part of a parameter. All values for type, subtype,
* parameter attributes or parameter values must be valid according to RFCs <a
* href="http://www.ietf.org/rfc/rfc2045.txt">2045</a> and <a
* href="http://www.ietf.org/rfc/rfc2046.txt">2046</a>.
* href="https://tools.ietf.org/html/rfc2045">2045</a> and <a
* href="https://tools.ietf.org/html/rfc2046">2046</a>.
*
* <p>All portions of the media type that are case-insensitive (type, subtype, parameter attributes)
* are normalized to lowercase. The value of the {@code charset} parameter is normalized to
Expand Down Expand Up @@ -941,6 +941,8 @@ private static String normalizeToken(String token) {
}

private static String normalizeParameterValue(String attribute, String value) {
checkNotNull(value); // for GWT
checkArgument(ascii().matchesAllOf(value), "parameter values must be ASCII: %s", value);
return CHARSET_ATTRIBUTE.equals(attribute) ? Ascii.toLowerCase(value) : value;
}

Expand Down Expand Up @@ -1088,7 +1090,9 @@ private String computeToString() {
new Function<String, String>() {
@Override
public String apply(String value) {
return TOKEN_MATCHER.matchesAllOf(value) ? value : escapeAndQuote(value);
return (TOKEN_MATCHER.matchesAllOf(value) && !value.isEmpty())
? value
: escapeAndQuote(value);
}
});
PARAMETER_JOINER.appendTo(builder, quotedParameters.entries());
Expand Down
40 changes: 40 additions & 0 deletions guava-gwt/test/com/google/common/net/MediaTypeTest_gwt.java
View file
Expand Up @@ -53,6 +53,16 @@ public void testCreate_invalidType() throws Exception {
testCase.testCreate_invalidType();
}

public void testCreate_nonAsciiParameter() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testCreate_nonAsciiParameter();
}

public void testCreate_nonAsciiParameterValue() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testCreate_nonAsciiParameterValue();
}

public void testCreate_wildcardTypeDeclaredSubtype() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testCreate_wildcardTypeDeclaredSubtype();
Expand Down Expand Up @@ -138,6 +148,16 @@ public void testWithParameter_invalidAttribute() throws Exception {
testCase.testWithParameter_invalidAttribute();
}

public void testWithParameter_nonAsciiParameter() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testWithParameter_nonAsciiParameter();
}

public void testWithParameter_nonAsciiParameterValue() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testWithParameter_nonAsciiParameterValue();
}

public void testWithParameters() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testWithParameters();
Expand All @@ -153,6 +173,16 @@ public void testWithParametersIterable_invalidAttribute() throws Exception {
testCase.testWithParametersIterable_invalidAttribute();
}

public void testWithParametersIterable_nonAsciiParameter() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testWithParametersIterable_nonAsciiParameter();
}

public void testWithParametersIterable_nonAsciiParameterValue() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testWithParametersIterable_nonAsciiParameterValue();
}

public void testWithParametersIterable_nullValue() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testWithParametersIterable_nullValue();
Expand All @@ -163,6 +193,16 @@ public void testWithParameters_invalidAttribute() throws Exception {
testCase.testWithParameters_invalidAttribute();
}

public void testWithParameters_nonAsciiParameter() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testWithParameters_nonAsciiParameter();
}

public void testWithParameters_nonAsciiParameterValue() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testWithParameters_nonAsciiParameterValue();
}

public void testWithoutParameters() throws Exception {
com.google.common.net.MediaTypeTest testCase = new com.google.common.net.MediaTypeTest();
testCase.testWithoutParameters();
Expand Down
77 changes: 76 additions & 1 deletion guava-tests/test/com/google/common/net/MediaTypeTest.java
View file
Expand Up @@ -148,6 +148,22 @@ public void testCreate_wildcardTypeDeclaredSubtype() {
}
}

public void testCreate_nonAsciiParameter() {
try {
MediaType.create("…", "a");
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testCreate_nonAsciiParameterValue() {
try {
MediaType.create("a", "…");
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testCreateApplicationType() {
MediaType newType = MediaType.createApplicationType("yams");
assertEquals("application", newType.type());
Expand Down Expand Up @@ -225,6 +241,26 @@ public void testWithParameters_invalidAttribute() {
}
}

public void testWithParameters_nonAsciiParameter() {
MediaType mediaType = MediaType.parse("text/plain");
ImmutableListMultimap<String, String> parameters = ImmutableListMultimap.of("…", "a");
try {
mediaType.withParameters(parameters);
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParameters_nonAsciiParameterValue() {
MediaType mediaType = MediaType.parse("text/plain");
ImmutableListMultimap<String, String> parameters = ImmutableListMultimap.of("a", "…");
try {
mediaType.withParameters(parameters);
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParameter() {
assertEquals(
MediaType.parse("text/plain; a=1"), MediaType.parse("text/plain").withParameter("a", "1"));
Expand All @@ -248,6 +284,24 @@ public void testWithParameter_invalidAttribute() {
}
}

public void testWithParameter_nonAsciiParameter() {
MediaType mediaType = MediaType.parse("text/plain");
try {
mediaType.withParameter("…", "a");
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParameter_nonAsciiParameterValue() {
MediaType mediaType = MediaType.parse("text/plain");
try {
mediaType.withParameter("a", "…");
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParametersIterable() {
assertEquals(
MediaType.parse("text/plain"),
Expand Down Expand Up @@ -275,6 +329,24 @@ public void testWithParametersIterable_invalidAttribute() {
}
}

public void testWithParametersIterable_nonAsciiParameter() {
MediaType mediaType = MediaType.parse("text/plain");
try {
mediaType.withParameters("…", ImmutableSet.of("a"));
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParametersIterable_nonAsciiParameterValue() {
MediaType mediaType = MediaType.parse("text/plain");
try {
mediaType.withParameters("a", ImmutableSet.of("…"));
fail();
} catch (IllegalArgumentException expected) {
}
}

public void testWithParametersIterable_nullValue() {
MediaType mediaType = MediaType.parse("text/plain");
try {
Expand Down Expand Up @@ -507,10 +579,13 @@ public void testNullPointer() {
public void testToString() {
assertEquals("text/plain", MediaType.create("text", "plain").toString());
assertEquals(
"text/plain; something=\"cr@zy\"; something-else=\"crazy with spaces\"",
"text/plain; something=\"cr@zy\"; something-else=\"crazy with spaces\";"
+ " and-another-thing=\"\"; normal-thing=foo",
MediaType.create("text", "plain")
.withParameter("something", "cr@zy")
.withParameter("something-else", "crazy with spaces")
.withParameter("and-another-thing", "")
.withParameter("normal-thing", "foo")
.toString());
}
}
10 changes: 7 additions & 3 deletions guava/src/com/google/common/net/MediaType.java
View file
Expand Up @@ -55,8 +55,8 @@
* type or subtype value. A media type may not have wildcard type with a declared subtype. The
* {@code *} character has no special meaning as part of a parameter. All values for type, subtype,
* parameter attributes or parameter values must be valid according to RFCs <a
* href="http://www.ietf.org/rfc/rfc2045.txt">2045</a> and <a
* href="http://www.ietf.org/rfc/rfc2046.txt">2046</a>.
* href="https://tools.ietf.org/html/rfc2045">2045</a> and <a
* href="https://tools.ietf.org/html/rfc2046">2046</a>.
*
* <p>All portions of the media type that are case-insensitive (type, subtype, parameter attributes)
* are normalized to lowercase. The value of the {@code charset} parameter is normalized to
Expand Down Expand Up @@ -941,6 +941,8 @@ private static String normalizeToken(String token) {
}

private static String normalizeParameterValue(String attribute, String value) {
checkNotNull(value); // for GWT
checkArgument(ascii().matchesAllOf(value), "parameter values must be ASCII: %s", value);
return CHARSET_ATTRIBUTE.equals(attribute) ? Ascii.toLowerCase(value) : value;
}

Expand Down Expand Up @@ -1088,7 +1090,9 @@ private String computeToString() {
new Function<String, String>() {
@Override
public String apply(String value) {
return TOKEN_MATCHER.matchesAllOf(value) ? value : escapeAndQuote(value);
return (TOKEN_MATCHER.matchesAllOf(value) && !value.isEmpty())
? value
: escapeAndQuote(value);
}
});
PARAMETER_JOINER.appendTo(builder, quotedParameters.entries());
Expand Down

0 comments on commit 2278123

Please sign in to comment.