Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep the behavior consistent with SpringMVC when access through browser #14816

Merged
merged 1 commit into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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;

Expand All @@ -81,6 +83,6 @@ public String getSubType() {
}

public boolean isPureText() {
return this == TEXT_HTML || this == TEXT_PLAIN;
return TEXT.equals(type);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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();
}
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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;
}
}
}
}
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand All @@ -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 {
Expand All @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}

}
Loading