Skip to content

Commit

Permalink
Revised support for Jetty 12 (tested against 12.0.0.alpha2)
Browse files Browse the repository at this point in the history 
Avoids HttpFields optimization completely, relying on Servlet header access instead.
ServletServerHttpResponse provides applyHeaders/adaptHeaders split for better reuse.

See gh-29575
  • Loading branch information
@jhoeller
jhoeller committed Dec 1, 2022
1 parent d5732fe commit 955ca4d
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 116 deletions.
4 changes: 2 additions & 2 deletions ...eb/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,10 @@ else if (this.state.compareAndSet(State.COMMIT_ACTION_FAILED, State.COMMITTING))
/**
* Invoked when the response is getting committed allowing subclasses to
* make apply header values to the underlying response.
* <p>Note that most subclasses use an {@link HttpHeaders} instance that
* <p>Note that some subclasses use an {@link HttpHeaders} instance that
* wraps an adapter to the native response headers such that changes are
* propagated to the underlying response on the go. That means this callback
* is typically not used other than for specialized updates such as setting
* might not be used other than for specialized updates such as setting
* the contentType or characterEncoding fields in a Servlet response.
*/
protected abstract void applyHeaders();
Expand Down
4 changes: 2 additions & 2 deletions spring-web/src/main/java/org/springframework/http/server/reactive/JettyHeadersAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class JettyHeadersAdapter implements MultiValueMap<String, String> {
private final HttpFields.Mutable headers;


JettyHeadersAdapter(HttpFields headers) {
this.headers = (headers instanceof HttpFields.Mutable mutable ? mutable : HttpFields.build(headers));
JettyHeadersAdapter(HttpFields.Mutable headers) {
this.headers = headers;
}


Expand Down
129 changes: 55 additions & 74 deletions ...g-web/src/main/java/org/springframework/http/server/reactive/JettyHttpHandlerAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@

import java.io.IOException;
import java.io.OutputStream;
import java.lang.reflect.Method;
import java.net.URISyntaxException;
import java.nio.ByteBuffer;
import java.nio.charset.Charset;

import jakarta.servlet.AsyncContext;
import jakarta.servlet.http.HttpServletRequest;
Expand All @@ -36,19 +34,17 @@
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferFactory;
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.MultiValueMap;
import org.springframework.util.ReflectionUtils;

/**
* {@link ServletHttpHandlerAdapter} extension that uses Jetty APIs for writing
* to the response with {@link ByteBuffer}.
*
* @author Violeta Georgieva
* @author Brian Clozel
* @author Juergen Hoeller
* @since 5.0
* @see org.springframework.web.server.adapter.AbstractReactiveWebInitializer
*/
Expand All @@ -62,14 +58,6 @@ public class JettyHttpHandlerAdapter extends ServletHttpHandlerAdapter {
"org.eclipse.jetty.ee10.servlet.HttpOutput", JettyHttpHandlerAdapter.class.getClassLoader());
*/

@Nullable
private static final Method getRequestHeadersMethod =
ClassUtils.getMethodIfAvailable(Request.class, "getHeaders");

@Nullable
private static final Method getResponseHeadersMethod =
ClassUtils.getMethodIfAvailable(Response.class, "getHeaders");


public JettyHttpHandlerAdapter(HttpHandler httpHandler) {
super(httpHandler);
Expand All @@ -80,23 +68,39 @@ public JettyHttpHandlerAdapter(HttpHandler httpHandler) {
protected ServletServerHttpRequest createRequest(HttpServletRequest request, AsyncContext context)
throws IOException, URISyntaxException {

Assert.state(getServletPath() != null, "Servlet path is not initialized");
return new JettyServerHttpRequest(
request, context, getServletPath(), getDataBufferFactory(), getBufferSize());
if (jetty11Present) {
Assert.state(getServletPath() != null, "Servlet path is not initialized");
return new Jetty11ServerHttpRequest(
request, context, getServletPath(), getDataBufferFactory(), getBufferSize());
}
else {
return super.createRequest(request, context);
}
}

@Override
protected ServletServerHttpResponse createResponse(HttpServletResponse response,
AsyncContext context, ServletServerHttpRequest request) throws IOException {

return new JettyServerHttpResponse(
response, context, getDataBufferFactory(), getBufferSize(), request);
if (jetty11Present) {
return new Jetty11ServerHttpResponse(
response, context, getDataBufferFactory(), getBufferSize(), request);
}
/* Jetty 12: see spring-web.gradle
else if (jetty12Present) {
return new Jetty12ServerHttpResponse(
response, context, getDataBufferFactory(), getBufferSize(), request);
}
*/
else {
return super.createResponse(response, context, request);
}
}


private static final class JettyServerHttpRequest extends ServletServerHttpRequest {
private static final class Jetty11ServerHttpRequest extends ServletServerHttpRequest {

JettyServerHttpRequest(HttpServletRequest request, AsyncContext asyncContext,
Jetty11ServerHttpRequest(HttpServletRequest request, AsyncContext asyncContext,
String servletPath, DataBufferFactory bufferFactory, int bufferSize)
throws IOException, URISyntaxException {

Expand All @@ -105,14 +109,7 @@ private static final class JettyServerHttpRequest extends ServletServerHttpReque

private static MultiValueMap<String, String> createHeaders(HttpServletRequest servletRequest) {
Request request = getRequest(servletRequest);
HttpFields fields;
if (getRequestHeadersMethod != null) {
fields = (HttpFields) ReflectionUtils.invokeMethod(getRequestHeadersMethod, request);
}
else {
fields = request.getHttpFields();
}
return new JettyHeadersAdapter(fields);
return new JettyHeadersAdapter(HttpFields.build(request.getHttpFields()));
}

private static Request getRequest(HttpServletRequest request) {
Expand All @@ -131,9 +128,9 @@ else if (request instanceof HttpServletRequestWrapper wrapper) {
}


private static final class JettyServerHttpResponse extends ServletServerHttpResponse {
private static final class Jetty11ServerHttpResponse extends ServletServerHttpResponse {

JettyServerHttpResponse(HttpServletResponse response, AsyncContext asyncContext,
Jetty11ServerHttpResponse(HttpServletResponse response, AsyncContext asyncContext,
DataBufferFactory bufferFactory, int bufferSize, ServletServerHttpRequest request)
throws IOException {

Expand All @@ -142,14 +139,7 @@ private static final class JettyServerHttpResponse extends ServletServerHttpResp

private static HttpHeaders createHeaders(HttpServletResponse servletResponse) {
Response response = getResponse(servletResponse);
HttpFields fields;
if (getResponseHeadersMethod != null) {
fields = (HttpFields) ReflectionUtils.invokeMethod(getResponseHeadersMethod, response);
}
else {
fields = response.getHttpFields();
}
return new HttpHeaders(new JettyHeadersAdapter(fields));
return new HttpHeaders(new JettyHeadersAdapter(response.getHttpFields()));
}

private static Response getResponse(HttpServletResponse response) {
Expand All @@ -169,53 +159,44 @@ else if (response instanceof HttpServletResponseWrapper wrapper) {
@Override
protected int writeToOutputStream(DataBuffer dataBuffer) throws IOException {
OutputStream output = getOutputStream();
if (jetty11Present) {
if (output instanceof HttpOutput httpOutput) {
ByteBuffer input = dataBuffer.toByteBuffer();
int len = input.remaining();
httpOutput.write(input);
return len;
}
}
/* Jetty 12: see spring-web.gradle
else if (jetty12Present) {
if (output instanceof org.eclipse.jetty.ee10.servlet.HttpOutput httpOutput) {
ByteBuffer input = dataBuffer.toByteBuffer();
int len = input.remaining();
httpOutput.write(input);
return len;
}
if (output instanceof HttpOutput httpOutput) {
ByteBuffer input = dataBuffer.toByteBuffer();
int len = input.remaining();
httpOutput.write(input);
return len;
}
*/
return super.writeToOutputStream(dataBuffer);
}

@Override
protected void applyHeaders() {
HttpServletResponse response = getNativeResponse();
adaptHeaders(false);
}
}

MediaType contentType = null;
try {
contentType = getHeaders().getContentType();
}
catch (Exception ex) {
String rawContentType = getHeaders().getFirst(HttpHeaders.CONTENT_TYPE);
response.setContentType(rawContentType);
}
if (response.getContentType() == null && contentType != null) {
response.setContentType(contentType.toString());
}

Charset charset = (contentType != null ? contentType.getCharset() : null);
if (response.getCharacterEncoding() == null && charset != null) {
response.setCharacterEncoding(charset.name());
}
/* Jetty 12: see spring-web.gradle
private static final class Jetty12ServerHttpResponse extends ServletServerHttpResponse {
Jetty12ServerHttpResponse(HttpServletResponse response, AsyncContext asyncContext,
DataBufferFactory bufferFactory, int bufferSize, ServletServerHttpRequest request)
throws IOException {
long contentLength = getHeaders().getContentLength();
if (contentLength != -1) {
response.setContentLengthLong(contentLength);
super(response, asyncContext, bufferFactory, bufferSize, request);
}
@Override
protected int writeToOutputStream(DataBuffer dataBuffer) throws IOException {
OutputStream output = getOutputStream();
if (output instanceof org.eclipse.jetty.ee10.servlet.HttpOutput httpOutput) {
ByteBuffer input = dataBuffer.toByteBuffer();
int len = input.remaining();
httpOutput.write(input);
return len;
}
return super.writeToOutputStream(dataBuffer);
}
}
*/

}
17 changes: 7 additions & 10 deletions ...web/src/main/java/org/springframework/http/server/reactive/ServletHttpHandlerAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public void service(ServletRequest request, ServletResponse response) throws Ser
AsyncListener requestListener;
String logPrefix;
try {
httpRequest = createRequest(((HttpServletRequest) request), asyncContext);
httpRequest = createRequest((HttpServletRequest) request, asyncContext);
requestListener = httpRequest.getAsyncListener();
logPrefix = httpRequest.getLogPrefix();
}
Expand All @@ -182,8 +182,10 @@ public void service(ServletRequest request, ServletResponse response) throws Ser
return;
}

ServerHttpResponse httpResponse = createResponse(((HttpServletResponse) response), asyncContext, httpRequest);
AsyncListener responseListener = ((ServletServerHttpResponse) httpResponse).getAsyncListener();
ServletServerHttpResponse wrappedResponse =
createResponse((HttpServletResponse) response, asyncContext, httpRequest);
ServerHttpResponse httpResponse = wrappedResponse;
AsyncListener responseListener = wrappedResponse.getAsyncListener();
if (httpRequest.getMethod() == HttpMethod.HEAD) {
httpResponse = new HttpHeadResponseDecorator(httpResponse);
}
Expand Down Expand Up @@ -263,9 +265,7 @@ private static class HttpHandlerAsyncListener implements AsyncListener {

private final String logPrefix;


public HttpHandlerAsyncListener(
AsyncListener requestAsyncListener, AsyncListener responseAsyncListener,
public HttpHandlerAsyncListener(AsyncListener requestAsyncListener, AsyncListener responseAsyncListener,
Runnable handlerDisposeTask, AtomicBoolean completionFlag, String logPrefix) {

this.requestAsyncListener = requestAsyncListener;
Expand All @@ -275,7 +275,6 @@ public HttpHandlerAsyncListener(
this.logPrefix = logPrefix;
}


@Override
public void onTimeout(AsyncEvent event) {
// Should never happen since we call asyncContext.setTimeout(-1)
Expand Down Expand Up @@ -361,9 +360,7 @@ private static class HandlerResultSubscriber implements Subscriber<Void>, Runnab
@Nullable
private volatile Subscription subscription;

public HandlerResultSubscriber(
AsyncContext asyncContext, AtomicBoolean completionFlag, String logPrefix) {

public HandlerResultSubscriber(AsyncContext asyncContext, AtomicBoolean completionFlag, String logPrefix) {
this.asyncContext = asyncContext;
this.completionFlag = completionFlag;
this.logPrefix = logPrefix;
Expand Down
13 changes: 12 additions & 1 deletion ...web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ protected void applyHeaders() {
this.response.addHeader(headerName, headerValue);
}
});

adaptHeaders(false);
}

protected void adaptHeaders(boolean removeAdaptedHeaders) {
MediaType contentType = null;
try {
contentType = getHeaders().getContentType();
Expand All @@ -140,19 +145,25 @@ protected void applyHeaders() {
if (this.response.getContentType() == null && contentType != null) {
this.response.setContentType(contentType.toString());
}

Charset charset = (contentType != null ? contentType.getCharset() : null);
if (this.response.getCharacterEncoding() == null && charset != null) {
this.response.setCharacterEncoding(charset.name());
}

long contentLength = getHeaders().getContentLength();
if (contentLength != -1) {
this.response.setContentLengthLong(contentLength);
}

if (removeAdaptedHeaders) {
getHeaders().remove(HttpHeaders.CONTENT_TYPE);
getHeaders().remove(HttpHeaders.CONTENT_LENGTH);
}
}

@Override
protected void applyCookies() {

// Servlet Cookie doesn't support same site:
// https://github.com/eclipse-ee4j/servlet-api/issues/175

Expand Down
29 changes: 2 additions & 27 deletions ...-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.lang.reflect.Field;
import java.net.URISyntaxException;
import java.nio.ByteBuffer;
import java.nio.charset.Charset;

import jakarta.servlet.AsyncContext;
import jakarta.servlet.http.HttpServletRequest;
Expand All @@ -37,7 +36,6 @@
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferFactory;
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.util.Assert;
import org.springframework.util.MultiValueMap;
import org.springframework.util.ReflectionUtils;
Expand All @@ -49,6 +47,7 @@
* @author Violeta Georgieva
* @author Brian Clozel
* @author Sam Brannen
* @author Juergen Hoeller
* @since 5.0
* @see org.springframework.web.server.adapter.AbstractReactiveWebInitializer
*/
Expand Down Expand Up @@ -193,31 +192,7 @@ else if (response instanceof HttpServletResponseWrapper wrapper) {

@Override
protected void applyHeaders() {
HttpServletResponse response = getNativeResponse();

MediaType contentType = null;
try {
contentType = getHeaders().getContentType();
}
catch (Exception ex) {
String rawContentType = getHeaders().getFirst(HttpHeaders.CONTENT_TYPE);
response.setContentType(rawContentType);
}
if (response.getContentType() == null && contentType != null) {
response.setContentType(contentType.toString());
}
getHeaders().remove(HttpHeaders.CONTENT_TYPE);

Charset charset = (contentType != null ? contentType.getCharset() : null);
if (response.getCharacterEncoding() == null && charset != null) {
response.setCharacterEncoding(charset.name());
}

long contentLength = getHeaders().getContentLength();
if (contentLength != -1) {
response.setContentLengthLong(contentLength);
}
getHeaders().remove(HttpHeaders.CONTENT_LENGTH);
adaptHeaders(true);
}

@Override
Expand Down

0 comments on commit 955ca4d

Please sign in to comment.