Log the mime types that failed to parse with error log level before p…

…ropagating exception. Fix bugs in encoding symbol table names with special characters RB=1891356 R=rtecco,mnchen,kbalasub A=kbalasub
linkedin · Nov 22, 2019 · fe840a2 · fe840a2
1 parent 0e8e26c
commit fe840a2
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 16 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,5 +1,11 @@
+28.0.8
+------
+
 28.0.7
 ------
+(RB=1891356)
+Log the mime types that failed to parse with error log level before propagating exception. Fix bugs in encoding symbol table names with special characters
+
 [pegasus] Disable SymlinkAwareZooKeeperTest temporarily
 
 (RB=1889124)

diff --git a/gradle.properties b/gradle.properties
@@ -1,4 +1,4 @@
-version=28.0.6
+version=28.0.7
 sonatypeUsername=please_set_in_home_dir_if_uploading_to_maven_central
 sonatypePassword=please_set_in_home_dir_if_uploading_to_maven_central
 org.gradle.configureondemand=true

diff --git a/restli-common/src/main/java/com/linkedin/restli/common/ContentType.java b/restli-common/src/main/java/com/linkedin/restli/common/ContentType.java
@@ -33,6 +33,8 @@
 import java.util.concurrent.ConcurrentHashMap;
 import javax.activation.MimeType;
 import javax.activation.MimeTypeParseException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 
 /**
@@ -43,6 +45,7 @@
  */
 public class ContentType
 {
+  private static final Logger LOG = LoggerFactory.getLogger(ContentType.class);
   private static final JacksonDataCodec JACKSON_DATA_CODEC = new JacksonDataCodec();
   private static final JacksonStreamDataCodec JACKSON_STREAM_DATA_CODEC = new JacksonStreamDataCodec(R2Constants.DEFAULT_DATA_CHUNK_SIZE);
   private static final JacksonLICORDataCodec LICOR_TEXT_DATA_CODEC = new JacksonLICORDataCodec(false);
@@ -176,7 +179,7 @@ public static Optional<ContentType> getContentType(String contentTypeHeaderValue
     {
       return Optional.of(JSON);
     }
-    MimeType parsedMimeType = new MimeType(contentTypeHeaderValue);
+    MimeType parsedMimeType = parseMimeType(contentTypeHeaderValue);
     ContentTypeProvider provider = SUPPORTED_TYPE_PROVIDERS.get(parsedMimeType.getBaseType().toLowerCase());
     if (provider == null)
     {
@@ -201,7 +204,7 @@ public static Optional<ContentType> getRequestContentType(String rawMimeType, UR
     {
       return Optional.of(JSON);
     }
-    MimeType parsedMimeType = new MimeType(rawMimeType);
+    MimeType parsedMimeType = parseMimeType(rawMimeType);
     ContentTypeProvider provider = SUPPORTED_TYPE_PROVIDERS.get(parsedMimeType.getBaseType().toLowerCase());
     if (provider == null)
     {
@@ -228,7 +231,7 @@ public static Optional<ContentType> getResponseContentType(String rawMimeType, U
     {
       return Optional.of(JSON);
     }
-    MimeType parsedMimeType = new MimeType(rawMimeType);
+    MimeType parsedMimeType = parseMimeType(rawMimeType);
     ContentTypeProvider provider = SUPPORTED_TYPE_PROVIDERS.get(parsedMimeType.getBaseType().toLowerCase());
     if (provider == null)
     {
@@ -237,6 +240,19 @@ public static Optional<ContentType> getResponseContentType(String rawMimeType, U
     return Optional.of(provider.getResponseContentType(rawMimeType, parsedMimeType, requestUri, requestHeaders));
   }
 
+  private static MimeType parseMimeType(String rawMimeType) throws MimeTypeParseException
+  {
+    try
+    {
+      return new MimeType(rawMimeType);
+    }
+    catch (MimeTypeParseException e)
+    {
+      LOG.error("Exception parsing mime type: " + rawMimeType, e);
+      throw e;
+    }
+  }
+
   private final String _headerKey;
   private final DataCodec _codec;
   private final StreamDataCodec _streamCodec;

diff --git a/...-common/src/main/java/com/linkedin/restli/common/SymbolTableBasedContentTypeProvider.java b/...-common/src/main/java/com/linkedin/restli/common/SymbolTableBasedContentTypeProvider.java
@@ -57,42 +57,43 @@ public final ContentType getContentType(String rawMimeType, MimeType mimeType)
 
     final SymbolTable symbolTable =
         SymbolTableProviderHolder.INSTANCE.getSymbolTableProvider().getSymbolTable(symbolTableName);
-    return getContentType(rawMimeType, symbolTable, false);
+    if (symbolTable == null)
+    {
+      return _baseContentType;
+    }
+
+    return _symbolTableMapper.apply(rawMimeType, symbolTable);
   }
 
   @Override
   public ContentType getRequestContentType(String rawMimeType, MimeType mimeType, URI requestUri)
   {
     final SymbolTable requestSymbolTable =
         SymbolTableProviderHolder.INSTANCE.getSymbolTableProvider().getRequestSymbolTable(requestUri);
-    return getContentType(rawMimeType, requestSymbolTable, true);
+    return getContentType(mimeType, requestSymbolTable);
   }
 
   @Override
   public ContentType getResponseContentType(String rawMimeType, MimeType mimeType, URI requestUri,
       Map<String, String> requestHeaders) {
     final SymbolTable responseSymbolTable =
         SymbolTableProviderHolder.INSTANCE.getSymbolTableProvider().getResponseSymbolTable(requestUri, requestHeaders);
-    return getContentType(rawMimeType, responseSymbolTable, true);
+    return getContentType(mimeType, responseSymbolTable);
   }
 
   public ContentType getBaseContentType()
   {
     return _baseContentType;
   }
 
-  private ContentType getContentType(String rawMimeType, SymbolTable symbolTable, boolean appendSymbolTable)
+  private ContentType getContentType(MimeType mimeType, SymbolTable symbolTable)
   {
     if (symbolTable == null)
     {
       return _baseContentType;
     }
 
-    if (appendSymbolTable)
-    {
-      rawMimeType = rawMimeType + "; " + RestConstants.CONTENT_TYPE_PARAM_SYMBOL_TABLE + "=" + symbolTable.getName();
-    }
-
-    return _symbolTableMapper.apply(rawMimeType, symbolTable);
+    mimeType.setParameter(RestConstants.CONTENT_TYPE_PARAM_SYMBOL_TABLE, symbolTable.getName());
+    return _symbolTableMapper.apply(mimeType.toString(), symbolTable);
   }
 }
diff --git a/restli-common/src/test/java/com/linkedin/restli/internal/common/TestContentType.java b/restli-common/src/test/java/com/linkedin/restli/internal/common/TestContentType.java
@@ -36,7 +36,7 @@
 public class TestContentType
 {
   private static final String TEST_REQUEST_SYMBOL_TABLE_NAME = "HahaRequest";
-  private static final String TEST_RESPONSE_SYMBOL_TABLE_NAME = "HahaResponse";
+  private static final String TEST_RESPONSE_SYMBOL_TABLE_NAME = "abc.linkedin.com:123|HahaResponse";
   private static final URI TEST_URI = URI.create("https://www.linkedin.com");
 
   @BeforeClass
@@ -164,6 +164,6 @@ public void testGetResponseProtobufContentType() throws MimeTypeParseException
   {
     ContentType contentType =
         ContentType.getResponseContentType(RestConstants.HEADER_VALUE_APPLICATION_PROTOBUF, TEST_URI, Collections.emptyMap()).get();
-    Assert.assertEquals("application/x-protobuf; symbol-table=HahaResponse", contentType.getHeaderKey());
+    Assert.assertEquals("application/x-protobuf; symbol-table=\"abc.linkedin.com:123|HahaResponse\"", contentType.getHeaderKey());
   }
 }