From a601a7048463ed0768222daded1bc4ca173c99ed Mon Sep 17 00:00:00 2001 From: Sean Yang Date: Wed, 23 Oct 2024 00:10:08 +0800 Subject: [PATCH] Keep the behavior consistent with SpringMVC when access through browser --- .../dubbo/remoting/http12/HttpUtils.java | 2 +- .../remoting/http12/message/MediaType.java | 20 +++++----- .../http12/message/codec/HtmlCodec.java | 22 +++++++++-- .../message/codec/HtmlCodecFactory.java | 4 +- .../http12/message/codec/PlainTextCodec.java | 17 +++++++- .../message/codec/PlainTextCodecFactory.java | 6 +-- .../http12/message/codec/CodecTest.java | 2 +- .../tri/rest/RestHttpMessageCodec.java | 7 ---- .../tri/rest/mapping/ContentNegotiator.java | 39 +++++++++++++++---- .../tri/rest/mapping/RequestMapping.java | 2 +- .../mapping/RestRequestHandlerMapping.java | 9 +---- .../support/basic/RestProtocolTest.groovy | 13 +++++++ 12 files changed, 97 insertions(+), 46 deletions(-) diff --git a/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/HttpUtils.java b/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/HttpUtils.java index d20ccec53cb..311d5bcb71d 100644 --- a/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/HttpUtils.java +++ b/dubbo-remoting/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 parseAccept(String header) { - List> mediaTypes = new ArrayList<>(); if (header == null) { return Collections.emptyList(); } + List> 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/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/MediaType.java b/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/MediaType.java index fd7af730042..84f71ce6815 100644 --- a/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/MediaType.java +++ b/dubbo-remoting/dubbo-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/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/HtmlCodec.java b/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/HtmlCodec.java index 5afc69b028a..378df57d54f 100644 --- a/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/HtmlCodec.java +++ b/dubbo-remoting/dubbo-remoting-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/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/HtmlCodecFactory.java b/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/HtmlCodecFactory.java index ee82e05bd03..4d09457121a 100644 --- a/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/HtmlCodecFactory.java +++ b/dubbo-remoting/dubbo-remoting-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/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodec.java b/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodec.java index 3ec4a99032d..3f3ec22a75c 100644 --- a/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodec.java +++ b/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodec.java @@ -17,12 +17,13 @@ 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; @@ -30,6 +31,18 @@ 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/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodecFactory.java b/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodecFactory.java index 69dbc7cfb3f..bed8a7fcd5b 100644 --- a/dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/PlainTextCodecFactory.java +++ b/dubbo-remoting/dubbo-remoting-http12/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/dubbo-remoting/dubbo-remoting-http12/src/test/java/org/apache/dubbo/remoting/http12/message/codec/CodecTest.java b/dubbo-remoting/dubbo-remoting-http12/src/test/java/org/apache/dubbo/remoting/http12/message/codec/CodecTest.java index 9d6d573eef1..43f76abb4a5 100644 --- a/dubbo-remoting/dubbo-remoting-http12/src/test/java/org/apache/dubbo/remoting/http12/message/codec/CodecTest.java +++ b/dubbo-remoting/dubbo-remoting-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/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/RestHttpMessageCodec.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/RestHttpMessageCodec.java index cd6df6f4d69..13cc0bd6159 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/RestHttpMessageCodec.java +++ b/dubbo-rpc/dubbo-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/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/ContentNegotiator.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/ContentNegotiator.java index acc27fafee5..713d695e5e6 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/ContentNegotiator.java +++ b/dubbo-rpc/dubbo-rpc-triple/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 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/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RequestMapping.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RequestMapping.java index 4264594e012..a8a829d02db 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RequestMapping.java +++ b/dubbo-rpc/dubbo-rpc-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/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RestRequestHandlerMapping.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RestRequestHandlerMapping.java index 73aaffc2f97..faf9ed61139 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RestRequestHandlerMapping.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/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)) { diff --git a/dubbo-rpc/dubbo-rpc-triple/src/test/groovy/org/apache/dubbo/rpc/protocol/tri/rest/support/basic/RestProtocolTest.groovy b/dubbo-rpc/dubbo-rpc-triple/src/test/groovy/org/apache/dubbo/rpc/protocol/tri/rest/support/basic/RestProtocolTest.groovy index 57e836d44ac..dc2ffb9332f 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/test/groovy/org/apache/dubbo/rpc/protocol/tri/rest/support/basic/RestProtocolTest.groovy +++ b/dubbo-rpc/dubbo-rpc-triple/src/test/groovy/org/apache/dubbo/rpc/protocol/tri/rest/support/basic/RestProtocolTest.groovy @@ -291,4 +291,17 @@ class RestProtocolTest extends BaseServiceTest { 'POST' | '/mismatchTest?name=earth' | 'text/plain' | 'text/plain' | '{"message":"Unsatisfied query parameter conditions","status":"400"}' } + def "consistent with SpringMVC"() { + given: + def request = new TestRequest( + path: path, + accept: 'text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8' + ) + expect: + runner.run(request).contentType == contentType + where: + path | contentType + '/beanArgTest' | 'application/json' + } + }