Keep the behavior consistent with SpringMVC when access through browser

apache · Oct 22, 2024 · dc7d71a · dc7d71a
1 parent 74218a4
commit dc7d71a
Show file tree

Hide file tree

Showing 11 changed files with 84 additions and 46 deletions.
diff --git a/...oting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/HttpUtils.java b/...oting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/HttpUtils.java
@@ -93,10 +93,10 @@ public static String encodeCookie(HttpCookie cookie) {
     }
 
     public static List<String> parseAccept(String header) {
-        List<Item<String>> mediaTypes = new ArrayList<>();
         if (header == null) {
             return Collections.emptyList();
         }
+        List<Item<String>> mediaTypes = new ArrayList<>();
         for (String item : StringUtils.tokenize(header, ',')) {
             int index = item.indexOf(';');
             mediaTypes.add(new Item<>(StringUtils.substring(item, 0, index), parseQuality(item, index)));

diff --git a/...bbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/MediaType.java b/...bbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/MediaType.java
@@ -20,6 +20,8 @@ public final class MediaType {
 
     public static final String WILDCARD = "*";
 
+    public static final String TEXT = "text";
+
     public static final MediaType ALL = new MediaType(WILDCARD, WILDCARD);
 
     public static final MediaType APPLICATION_JSON = new MediaType("application", "json");
@@ -40,21 +42,21 @@ public final class MediaType {
 
     public static final MediaType MULTIPART_FORM_DATA = new MediaType("multipart", "form-data");
 
-    public static final MediaType TEXT_JSON = new MediaType("text", "json");
+    public static final MediaType TEXT_JSON = new MediaType(TEXT, "json");
 
-    public static final MediaType TEXT_XML = new MediaType("text", "xml");
+    public static final MediaType TEXT_XML = new MediaType(TEXT, "xml");
 
-    public static final MediaType TEXT_YAML = new MediaType("text", "yaml");
+    public static final MediaType TEXT_YAML = new MediaType(TEXT, "yaml");
 
-    public static final MediaType TEXT_CSS = new MediaType("text", "css");
+    public static final MediaType TEXT_CSS = new MediaType(TEXT, "css");
 
-    public static final MediaType TEXT_JAVASCRIPT = new MediaType("text", "javascript");
+    public static final MediaType TEXT_JAVASCRIPT = new MediaType(TEXT, "javascript");
 
-    public static final MediaType TEXT_HTML = new MediaType("text", "html");
+    public static final MediaType TEXT_HTML = new MediaType(TEXT, "html");
 
-    public static final MediaType TEXT_PLAIN = new MediaType("text", "plain");
+    public static final MediaType TEXT_PLAIN = new MediaType(TEXT, "plain");
 
-    public static final MediaType TEXT_EVENT_STREAM = new MediaType("text", "event-stream");
+    public static final MediaType TEXT_EVENT_STREAM = new MediaType(TEXT, "event-stream");
 
     private final String name;
 
@@ -81,6 +83,6 @@ public String getSubType() {
     }
 
     public boolean isPureText() {
-        return this == TEXT_HTML || this == TEXT_PLAIN;
+        return TEXT.equals(type);
     }
 }
diff --git a/...moting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/HtmlCodec.java b/...moting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/HtmlCodec.java
@@ -17,27 +17,43 @@
 package org.apache.dubbo.remoting.http12.message.codec;
 
 import org.apache.dubbo.common.io.StreamUtils;
-import org.apache.dubbo.common.utils.JsonUtils;
+import org.apache.dubbo.remoting.http12.HttpJsonUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.exception.HttpStatusException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
 
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
 
-public class HtmlCodec implements HttpMessageCodec {
+public final class HtmlCodec implements HttpMessageCodec {
+
+    public static final HtmlCodec INSTANCE = new HtmlCodec();
+
+    private final HttpJsonUtils httpJsonUtils;
+
+    private HtmlCodec() {
+        this(FrameworkModel.defaultModel());
+    }
+
+    public HtmlCodec(FrameworkModel frameworkModel) {
+        httpJsonUtils = frameworkModel.getBeanFactory().getOrRegisterBean(HttpJsonUtils.class);
+    }
 
     @Override
     public void encode(OutputStream os, Object data, Charset charset) throws EncodeException {
+        if (data == null) {
+            return;
+        }
         try {
             if (data instanceof CharSequence) {
                 os.write((data.toString()).getBytes(charset));
                 return;
             }
-            os.write(JsonUtils.toJson(data).getBytes(charset));
+            os.write(httpJsonUtils.toJson(data).getBytes(charset));
         } catch (HttpStatusException e) {
             throw e;
         } catch (Throwable t) {

diff --git a/...http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/HtmlCodecFactory.java b/...http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/HtmlCodecFactory.java
@@ -27,11 +27,9 @@
 @Activate
 public final class HtmlCodecFactory implements HttpMessageEncoderFactory, HttpMessageDecoderFactory {
 
-    private static final HtmlCodec INSTANCE = new HtmlCodec();
-
     @Override
     public HttpMessageCodec createCodec(URL url, FrameworkModel frameworkModel, String mediaType) {
-        return INSTANCE;
+        return frameworkModel == FrameworkModel.defaultModel() ? HtmlCodec.INSTANCE : new HtmlCodec(frameworkModel);
     }
 
     @Override

diff --git a/...g-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodec.java b/...g-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodec.java
@@ -17,19 +17,32 @@
 package org.apache.dubbo.remoting.http12.message.codec;
 
 import org.apache.dubbo.common.io.StreamUtils;
-import org.apache.dubbo.common.utils.JsonUtils;
+import org.apache.dubbo.remoting.http12.HttpJsonUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.exception.HttpStatusException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
 
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
 
 public final class PlainTextCodec implements HttpMessageCodec {
 
+    public static final PlainTextCodec INSTANCE = new PlainTextCodec();
+
+    private final HttpJsonUtils httpJsonUtils;
+
+    private PlainTextCodec() {
+        this(FrameworkModel.defaultModel());
+    }
+
+    public PlainTextCodec(FrameworkModel frameworkModel) {
+        httpJsonUtils = frameworkModel.getBeanFactory().getOrRegisterBean(HttpJsonUtils.class);
+    }
+
     @Override
     public void encode(OutputStream os, Object data, Charset charset) throws EncodeException {
         if (data == null) {
@@ -40,7 +53,7 @@ public void encode(OutputStream os, Object data, Charset charset) throws EncodeE
                 os.write((data.toString()).getBytes(charset));
                 return;
             }
-            os.write(JsonUtils.toJson(data).getBytes(charset));
+            os.write(httpJsonUtils.toJson(data).getBytes(charset));
         } catch (HttpStatusException e) {
             throw e;
         } catch (Throwable t) {

diff --git a/...2/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodecFactory.java b/...2/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodecFactory.java
@@ -27,11 +27,11 @@
 @Activate(order = 10000)
 public final class PlainTextCodecFactory implements HttpMessageEncoderFactory, HttpMessageDecoderFactory {
 
-    private static final PlainTextCodec INSTANCE = new PlainTextCodec();
-
     @Override
     public HttpMessageCodec createCodec(URL url, FrameworkModel frameworkModel, String mediaType) {
-        return INSTANCE;
+        return frameworkModel == FrameworkModel.defaultModel()
+                ? PlainTextCodec.INSTANCE
+                : new PlainTextCodec(frameworkModel);
     }
 
     @Override

diff --git a/...moting-http12/src/test/java/org/apache/dubbo/remoting/http12/message/codec/CodecTest.java b/...moting-http12/src/test/java/org/apache/dubbo/remoting/http12/message/codec/CodecTest.java
@@ -75,7 +75,7 @@ void testPlainText() {
         Assertions.assertEquals("Hello, world", res);
 
         in = new ByteArrayInputStream(utf8Bytes);
-        codec = new PlainTextCodec();
+        codec = PlainTextCodec.INSTANCE;
         res = (String) codec.decode(in, String.class, Charsets.UTF_8);
         Assertions.assertEquals("你好，世界", res);
 

diff --git a/...rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/RestHttpMessageCodec.java b/...rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/RestHttpMessageCodec.java
@@ -26,7 +26,6 @@
 import org.apache.dubbo.remoting.http12.message.HttpMessageEncoder;
 import org.apache.dubbo.remoting.http12.message.MediaType;
 import org.apache.dubbo.rpc.protocol.tri.rest.argument.ArgumentResolver;
-import org.apache.dubbo.rpc.protocol.tri.rest.argument.TypeConverter;
 import org.apache.dubbo.rpc.protocol.tri.rest.mapping.meta.ParameterMeta;
 
 import java.io.ByteArrayInputStream;
@@ -44,7 +43,6 @@ public final class RestHttpMessageCodec implements HttpMessageDecoder, HttpMessa
     private final HttpResponse response;
     private final ParameterMeta[] parameters;
     private final ArgumentResolver argumentResolver;
-    private final TypeConverter typeConverter;
     private final HttpMessageEncoder messageEncoder;
     private final Charset charset;
 
@@ -53,13 +51,11 @@ public RestHttpMessageCodec(
             HttpResponse response,
             ParameterMeta[] parameters,
             ArgumentResolver argumentResolver,
-            TypeConverter typeConverter,
             HttpMessageEncoder messageEncoder) {
         this.request = request;
         this.response = response;
         this.parameters = parameters;
         this.argumentResolver = argumentResolver;
-        this.typeConverter = typeConverter;
         this.messageEncoder = messageEncoder;
         charset = request.charsetOrDefault();
     }
@@ -125,9 +121,6 @@ public void encode(OutputStream os, Object data) throws EncodeException {
                     }
                     return;
                 }
-                if (messageEncoder.mediaType().isPureText() && type != String.class) {
-                    data = typeConverter.convert(data, String.class);
-                }
             } catch (HttpStatusException e) {
                 throw e;
             } catch (Exception e) {

diff --git a/...riple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/ContentNegotiator.java b/...riple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/ContentNegotiator.java
@@ -25,6 +25,7 @@
 import org.apache.dubbo.remoting.http12.message.codec.CodecUtils;
 import org.apache.dubbo.rpc.model.FrameworkModel;
 import org.apache.dubbo.rpc.protocol.tri.rest.RestConstants;
+import org.apache.dubbo.rpc.protocol.tri.rest.mapping.meta.HandlerMeta;
 
 import java.util.HashMap;
 import java.util.List;
@@ -44,7 +45,7 @@ public ContentNegotiator(FrameworkModel frameworkModel) {
         codecUtils = frameworkModel.getBeanFactory().getOrRegisterBean(CodecUtils.class);
     }
 
-    public String negotiate(HttpRequest request) {
+    public String negotiate(HttpRequest request, HandlerMeta meta) {
         String mediaType;
 
         // 1. find mediaType by producible
@@ -60,11 +61,19 @@ public String negotiate(HttpRequest request) {
 
         // 2. find mediaType by accept header
         List<String> accepts = HttpUtils.parseAccept(request.accept());
+        String preferMediaType = null;
+        boolean hasAll = false;
         if (accepts != null) {
             for (int i = 0, size = accepts.size(); i < size; i++) {
-                mediaType = getSuitableMediaType(accepts.get(i));
-                if (mediaType != null) {
-                    return mediaType;
+                String accept = accepts.get(i);
+                if (!hasAll && MediaType.ALL.getName().equals(accept)) {
+                    hasAll = true;
+                }
+                if (preferMediaType == null) {
+                    mediaType = getSuitableMediaType(accept);
+                    if (mediaType != null) {
+                        preferMediaType = mediaType;
+                    }
                 }
             }
         }
@@ -78,15 +87,29 @@ public String negotiate(HttpRequest request) {
             }
         }
 
-        // 5. find mediaType by extension
+        // 4. find mediaType by extension
         String path = request.rawPath();
         int index = path.lastIndexOf('.');
         if (index != -1) {
-            String extension = path.substring(index + 1);
-            return getMediaTypeByExtension(extension);
+            mediaType = getMediaTypeByExtension(path.substring(index + 1));
+            if (mediaType != null) {
+                return mediaType;
+            }
+        }
+
+        if (preferMediaType == null) {
+            return null;
+        }
+
+        // Keep consistent with Spring MVC behavior
+        if (hasAll && preferMediaType.startsWith("text/")) {
+            Class<?> responseType = meta.getMethodMetadata().getActualResponseType();
+            if (responseType != null && !CharSequence.class.isAssignableFrom(responseType)) {
+                return MediaType.APPLICATION_JSON.getName();
+            }
         }
 
-        return null;
+        return preferMediaType;
     }
 
     public boolean supportExtension(String extension) {

diff --git a/...c-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RequestMapping.java b/...c-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RequestMapping.java
@@ -465,7 +465,7 @@ public RequestMapping build() {
                     isEmpty(consumes) ? null : new ConsumesCondition(consumes),
                     isEmpty(produces) ? null : new ProducesCondition(produces),
                     customCondition == null ? null : ConditionWrapper.wrap(customCondition),
-                    cors == null ? null : cors,
+                    cors == null || cors.isEmpty() ? null : cors,
                     responseStatus == null ? null : new ResponseMeta(responseStatus, responseReason));
         }
     }

diff --git a/...c/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RestRequestHandlerMapping.java b/...c/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RestRequestHandlerMapping.java
@@ -36,8 +36,6 @@
 import org.apache.dubbo.rpc.protocol.tri.rest.RestHttpMessageCodec;
 import org.apache.dubbo.rpc.protocol.tri.rest.argument.ArgumentResolver;
 import org.apache.dubbo.rpc.protocol.tri.rest.argument.CompositeArgumentResolver;
-import org.apache.dubbo.rpc.protocol.tri.rest.argument.GeneralTypeConverter;
-import org.apache.dubbo.rpc.protocol.tri.rest.argument.TypeConverter;
 import org.apache.dubbo.rpc.protocol.tri.rest.mapping.condition.MethodsCondition;
 import org.apache.dubbo.rpc.protocol.tri.rest.mapping.meta.HandlerMeta;
 import org.apache.dubbo.rpc.protocol.tri.rest.util.RequestUtils;
@@ -51,19 +49,15 @@ public final class RestRequestHandlerMapping implements RequestHandlerMapping {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(RestRequestHandlerMapping.class);
 
-    private final FrameworkModel frameworkModel;
     private final RequestMappingRegistry requestMappingRegistry;
     private final ArgumentResolver argumentResolver;
-    private final TypeConverter typeConverter;
     private final ContentNegotiator contentNegotiator;
     private final CodecUtils codecUtils;
 
     public RestRequestHandlerMapping(FrameworkModel frameworkModel) {
-        this.frameworkModel = frameworkModel;
         ScopeBeanFactory beanFactory = frameworkModel.getBeanFactory();
         requestMappingRegistry = beanFactory.getOrRegisterBean(DefaultRequestMappingRegistry.class);
         argumentResolver = beanFactory.getOrRegisterBean(CompositeArgumentResolver.class);
-        typeConverter = beanFactory.getOrRegisterBean(GeneralTypeConverter.class);
         contentNegotiator = beanFactory.getOrRegisterBean(ContentNegotiator.class);
         codecUtils = beanFactory.getOrRegisterBean(CodecUtils.class);
     }
@@ -89,7 +83,7 @@ public RequestHandler getRequestHandler(URL url, HttpRequest request, HttpRespon
         }
 
         String requestMediaType = request.mediaType();
-        String responseMediaType = contentNegotiator.negotiate(request);
+        String responseMediaType = contentNegotiator.negotiate(request, meta);
         if (responseMediaType != null) {
             response.setContentType(responseMediaType);
         } else {
@@ -104,7 +98,6 @@ public RequestHandler getRequestHandler(URL url, HttpRequest request, HttpRespon
                 response,
                 meta.getParameters(),
                 argumentResolver,
-                typeConverter,
                 codecUtils.determineHttpMessageEncoder(url, responseMediaType));
 
         if (HttpMethods.supportBody(method) && !RequestUtils.isFormOrMultiPart(request)) {