This is an automated email from the ASF dual-hosted git repository. bonampak pushed a commit to branch feature/jakarta-jetty-upgrade in repository https://gitbox.apache.org/repos/asf/knox.git
commit fdae88c821902c85e9fb835d4d846b20fe1e4f54 Author: bonampak <[email protected]> AuthorDate: Wed Mar 11 22:13:04 2026 +0100 KNOX-3238: correcting TraceHandler, HSTSHandler and KnoxErrorHandler, along with jetty 12 form keys length and size. --- .../org/apache/knox/gateway/GatewayServer.java | 5 +- .../gateway/config/impl/GatewayConfigImpl.java | 6 +- .../apache/knox/gateway/filter/HSTSHandler.java | 21 +++---- .../knox/gateway/trace/KnoxErrorHandler.java | 13 ++--- .../apache/knox/gateway/trace/TraceHandler.java | 19 +++--- .../org/apache/knox/gateway/trace/TraceInput.java | 48 ++++++---------- .../org/apache/knox/gateway/trace/TraceOutput.java | 54 ++++++----------- .../apache/knox/gateway/trace/TraceRequest.java | 67 ++++++++++++---------- .../apache/knox/gateway/trace/TraceResponse.java | 47 +++++++++------ .../knox/gateway/filter/HSTSHandlerTest.java | 20 +++---- 10 files changed, 139 insertions(+), 161 deletions(-) diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java index 0252950aa..b5b2de132 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java @@ -60,6 +60,7 @@ import org.apache.knox.gateway.util.XmlUtils; import org.apache.knox.gateway.websockets.GatewayWebsocketHandler; import org.eclipse.jetty.server.ConnectionFactory; import org.eclipse.jetty.server.Connector; +import org.eclipse.jetty.server.FormFields; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; @@ -784,9 +785,9 @@ public class GatewayServer { void createJetty() throws IOException, CertificateException, NoSuchAlgorithmException, KeyStoreException, AliasServiceException { jetty = new Server( new QueuedThreadPool( config.getThreadPoolMax() ) ); - jetty.setAttribute(ContextHandler.MAX_FORM_CONTENT_SIZE_KEY, config.getJettyMaxFormContentSize()); + jetty.setAttribute(FormFields.MAX_LENGTH_ATTRIBUTE, config.getJettyMaxFormContentSize()); log.setMaxFormContentSize(config.getJettyMaxFormContentSize()); - jetty.setAttribute(ContextHandler.MAX_FORM_KEYS_KEY, config.getJettyMaxFormKeys()); + jetty.setAttribute(FormFields.MAX_FIELDS_ATTRIBUTE, config.getJettyMaxFormKeys()); log.setMaxFormKeys(config.getJettyMaxFormKeys()); // Add a handler for the 404 responses when a topology is being redeployed (i.e., is inactive) diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java b/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java index d0e34d0fc..e3b21a0bc 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java @@ -52,7 +52,7 @@ import org.apache.knox.gateway.dto.HomePageProfile; import org.apache.knox.gateway.fips.FipsUtils; import org.apache.knox.gateway.i18n.messages.MessagesFactory; import org.apache.knox.gateway.services.security.impl.ZookeeperRemoteAliasService; -import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.ee10.servlet.ServletContextHandler; import org.joda.time.Period; import org.joda.time.format.PeriodFormatter; import org.joda.time.format.PeriodFormatterBuilder; @@ -1591,12 +1591,12 @@ public class GatewayConfigImpl extends Configuration implements GatewayConfig { @Override public int getJettyMaxFormContentSize() { - return getInt(JETTY_MAX_FORM_CONTENT_SIZE, ContextHandler.DEFAULT_MAX_FORM_CONTENT_SIZE); + return getInt(JETTY_MAX_FORM_CONTENT_SIZE, ServletContextHandler.DEFAULT_MAX_FORM_CONTENT_SIZE); } @Override public int getJettyMaxFormKeys() { - return getInt(JETTY_MAX_FORM_KEYS, ContextHandler.DEFAULT_MAX_FORM_KEYS); + return getInt(JETTY_MAX_FORM_KEYS, ServletContextHandler.DEFAULT_MAX_FORM_KEYS); } @Override diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/filter/HSTSHandler.java b/gateway-server/src/main/java/org/apache/knox/gateway/filter/HSTSHandler.java index 79b9f7939..f0bd4c6fe 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/filter/HSTSHandler.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/filter/HSTSHandler.java @@ -16,16 +16,13 @@ */ package org.apache.knox.gateway.filter; -import com.google.common.net.HttpHeaders; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.handler.HandlerWrapper; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.Callback; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import java.io.IOException; - -public class HSTSHandler extends HandlerWrapper { +public class HSTSHandler extends Handler.Wrapper { private final String option; @@ -34,8 +31,8 @@ public class HSTSHandler extends HandlerWrapper { } @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { - response.setHeader(HttpHeaders.STRICT_TRANSPORT_SECURITY, option); - super.handle(target, baseRequest, request, response); + public boolean handle(Request request, Response response, Callback callback) throws Exception { + response.getHeaders().put(HttpHeader.STRICT_TRANSPORT_SECURITY, option); + return super.handle(request, response, callback); } -} +} \ No newline at end of file diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/trace/KnoxErrorHandler.java b/gateway-server/src/main/java/org/apache/knox/gateway/trace/KnoxErrorHandler.java index b3faeb45b..3ec532db5 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/trace/KnoxErrorHandler.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/trace/KnoxErrorHandler.java @@ -18,12 +18,10 @@ package org.apache.knox.gateway.trace; import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.handler.ErrorHandler; +import org.eclipse.jetty.util.Callback; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import java.io.IOException; import java.util.Set; public class KnoxErrorHandler extends ErrorHandler { @@ -35,10 +33,9 @@ public class KnoxErrorHandler extends ErrorHandler { } @Override - public void handle( String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response ) - throws IOException, ServletException { - HttpServletResponse traceResponse = new TraceResponse( response, bodyFilter ); - super.handle( target, baseRequest, request, traceResponse ); + public boolean handle(Request request, Response response, Callback callback) throws Exception { + Response newResponse = new TraceResponse(request, response, bodyFilter); + return super.handle(request, newResponse, callback); } } diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceHandler.java b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceHandler.java index e8a992f47..5c47ac966 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceHandler.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceHandler.java @@ -17,16 +17,14 @@ */ package org.apache.knox.gateway.trace; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.handler.HandlerWrapper; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.Callback; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import java.io.IOException; import java.util.Set; -public class TraceHandler extends HandlerWrapper { +public class TraceHandler extends Handler.Wrapper { static final String HTTP_LOGGER = "org.apache.knox.gateway.http"; static final String HTTP_REQUEST_LOGGER = HTTP_LOGGER + ".request"; @@ -44,11 +42,10 @@ public class TraceHandler extends HandlerWrapper { } @Override - public void handle( String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response ) - throws IOException, ServletException { - HttpServletRequest newRequest = new TraceRequest( request ); - HttpServletResponse newResponse = new TraceResponse( response, bodyFilter ); - super.handle( target, baseRequest, newRequest, newResponse ); + public boolean handle(Request request, Response response, Callback callback) throws Exception { + Request newRequest = new TraceRequest(request); + Response newResponse = new TraceResponse(request, response, bodyFilter); + return super.handle(newRequest, newResponse, callback); } } diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceInput.java b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceInput.java index 6f62a49d8..ccd7fb37e 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceInput.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceInput.java @@ -17,54 +17,42 @@ */ package org.apache.knox.gateway.trace; -import org.apache.knox.gateway.servlet.SynchronousServletInputStreamAdapter; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import jakarta.servlet.ServletInputStream; -import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.util.Locale; -class TraceInput extends SynchronousServletInputStreamAdapter { +class TraceInput { private static final Logger log = LogManager.getLogger( TraceHandler.HTTP_REQUEST_LOGGER ); private static final Logger bodyLog = LogManager.getLogger( TraceHandler.HTTP_REQUEST_BODY_LOGGER ); - private ServletInputStream delegate; - private static final int BUFFER_LIMIT = 1024; - private StringBuilder buffer = new StringBuilder( BUFFER_LIMIT ); + private final StringBuilder buffer = new StringBuilder( BUFFER_LIMIT ); - TraceInput( ServletInputStream delegate ) { - this.delegate = delegate; - } - @Override - public int read() throws IOException { - int b = delegate.read(); - if( b >= 0 ) { - buffer.append( (char)b ); - if( buffer.length() == BUFFER_LIMIT || delegate.available() == 0 ) { - traceBody(); + public synchronized void extractContent(ByteBuffer view, boolean last) { + if (view != null && view.hasRemaining()) { + while (view.hasRemaining() && buffer.length() < BUFFER_LIMIT) { + String s = StandardCharsets.UTF_8.decode(view).toString(); + buffer.append(s); } } - return b; - } - - @Override - public void close() throws IOException { - traceBody(); - delegate.close(); + if (buffer.length() >= BUFFER_LIMIT || last) { + traceBody(); + } } private synchronized void traceBody() { - if( buffer.length() > 0 ) { + if (!buffer.isEmpty()) { String body = buffer.toString(); - buffer.setLength( 0 ); + buffer.setLength(0); StringBuilder sb = new StringBuilder(); - TraceUtil.appendCorrelationContext( sb ); - sb.append( String.format(Locale.ROOT, "|RequestBody[%d]%n\t%s", body.length(), body ) ); - if( bodyLog.isTraceEnabled() ) { - log.trace( sb.toString() ); + TraceUtil.appendCorrelationContext(sb); + sb.append(String.format(Locale.ROOT, "|RequestBody[%d]%n\t%s", body.length(), body)); + if (bodyLog.isTraceEnabled()) { + log.trace(sb.toString()); } } } diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceOutput.java b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceOutput.java index c3de1c755..5b3720f5a 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceOutput.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceOutput.java @@ -17,59 +17,41 @@ */ package org.apache.knox.gateway.trace; -import org.apache.knox.gateway.servlet.SynchronousServletOutputStreamAdapter; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import jakarta.servlet.ServletOutputStream; -import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.util.Locale; -class TraceOutput extends SynchronousServletOutputStreamAdapter { +class TraceOutput { private static final Logger log = LogManager.getLogger( TraceHandler.HTTP_RESPONSE_LOGGER ); private static final Logger bodyLog = LogManager.getLogger( TraceHandler.HTTP_RESPONSE_BODY_LOGGER ); - private ServletOutputStream delegate; - private static final int BUFFER_LIMIT = 1024; - private StringBuilder buffer = new StringBuilder( BUFFER_LIMIT ); - - TraceOutput( ServletOutputStream delegate ) { - this.delegate = delegate; - } + private final StringBuilder buffer = new StringBuilder( BUFFER_LIMIT ); - @Override - public synchronized void write( int b ) throws IOException { - if( b >= 0 ) { - buffer.append( (char)b ); - if( buffer.length() == BUFFER_LIMIT ) { - traceBody(); + public synchronized void extractContent(ByteBuffer view, boolean last) { + if (view != null && view.hasRemaining()) { + while (view.hasRemaining() && buffer.length() < BUFFER_LIMIT) { + String s = StandardCharsets.UTF_8.decode(view).toString(); + buffer.append(s); } } - delegate.write( b ); - } - - @Override - public void flush() throws IOException { - traceBody(); - delegate.flush(); - } - - @Override - public void close() throws IOException { - traceBody(); - delegate.close(); + if (buffer.length() >= BUFFER_LIMIT || last) { + traceBody(); + } } - private synchronized void traceBody() { - if( buffer.length() > 0 ) { + private void traceBody() { + if (!buffer.isEmpty()) { String body = buffer.toString(); - buffer.setLength( 0 ); + buffer.setLength(0); StringBuilder sb = new StringBuilder(); - TraceUtil.appendCorrelationContext( sb ); - sb.append( String.format(Locale.ROOT, "|ResponseBody[%d]%n\t%s", body.length(), body ) ); + TraceUtil.appendCorrelationContext(sb); + sb.append(String.format(Locale.ROOT, "|ResponseBody[%d]%n\t%s", body.length(), body)); if( bodyLog.isTraceEnabled() ) { - log.trace( sb.toString() ); + log.trace(sb.toString()); } } } diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceRequest.java b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceRequest.java index 39f564114..8dbfe2743 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceRequest.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceRequest.java @@ -20,46 +20,56 @@ package org.apache.knox.gateway.trace; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import jakarta.servlet.ServletInputStream; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletRequestWrapper; -import java.io.IOException; -import java.util.Enumeration; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Request; + +import java.nio.ByteBuffer; import java.util.Locale; -class TraceRequest extends HttpServletRequestWrapper { +class TraceRequest extends Request.Wrapper { private static final Logger log = LogManager.getLogger( TraceHandler.HTTP_REQUEST_LOGGER ); private static final Logger headLog = LogManager.getLogger( TraceHandler.HTTP_REQUEST_HEADER_LOGGER ); - private ServletInputStream input; + private TraceInput delegate; - TraceRequest( HttpServletRequest request ) { - super( request ); - if( log.isTraceEnabled() ) { + TraceRequest(Request request) { + super(request); + if (log.isTraceEnabled()) { + delegate = new TraceInput(); traceRequestDetails(); } } @Override - public synchronized ServletInputStream getInputStream() throws IOException { - if( log.isTraceEnabled() ) { - if( input == null ) { - input = new TraceInput( super.getInputStream() ); + public void demand(Runnable demandCallback) { + super.demand(demandCallback); + } + + @Override + public Content.Chunk read() { + Content.Chunk chunk = super.read(); + if (chunk != null && log.isTraceEnabled()) { + if (Content.Chunk.isFailure(chunk)) { + // Log that the request failed if you want + return chunk; } - return input; - } else { - return super.getInputStream(); + ByteBuffer data = chunk.getByteBuffer(); + // Use slice() so the tracer doesn't interfere with the data + delegate.extractContent(data != null ? data.slice() : null, chunk.isLast()); } + return chunk; } private void traceRequestDetails() { StringBuilder sb = new StringBuilder(); TraceUtil.appendCorrelationContext( sb ); sb.append("|Request=") - .append(getMethod()) - .append(' ') - .append(getRequestURI()); - String qs = getQueryString(); + .append(getMethod()) + .append(' ') + .append(getHttpURI().getPath()); + String qs = getHttpURI().getQuery(); if( qs != null ) { sb.append('?').append(qs); } @@ -67,15 +77,12 @@ class TraceRequest extends HttpServletRequestWrapper { log.trace(sb.toString()); } - private void appendHeaders( StringBuilder sb ) { - if( headLog.isTraceEnabled() ) { - Enumeration<String> names = getHeaderNames(); - while( names.hasMoreElements() ) { - String name = names.nextElement(); - Enumeration<String> values = getHeaders( name ); - while( values.hasMoreElements() ) { - String value = values.nextElement(); - sb.append( String.format(Locale.ROOT, "%n\tHeader[%s]=%s", name, value ) ); + private void appendHeaders(StringBuilder sb) { + if (headLog.isTraceEnabled()) { + HttpFields requestHeaders = getHeaders(); + if (requestHeaders != null) { + for (HttpField header: requestHeaders) { + sb.append( String.format(Locale.ROOT, "%n\tHeader[%s]=%s", header.getName(), header.getValue())); } } } diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceResponse.java b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceResponse.java index b52052793..2cf63ae6e 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceResponse.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceResponse.java @@ -21,57 +21,66 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import jakarta.servlet.ServletOutputStream; -import jakarta.servlet.http.HttpServletResponse; -import jakarta.servlet.http.HttpServletResponseWrapper; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.Callback; + + import java.io.IOException; +import java.nio.ByteBuffer; import java.util.Collection; import java.util.Locale; import java.util.Set; -class TraceResponse extends HttpServletResponseWrapper { +class TraceResponse extends Response.Wrapper { private static final Logger log = LogManager.getLogger( TraceHandler.HTTP_RESPONSE_LOGGER ); private static final Logger headLog = LogManager.getLogger( TraceHandler.HTTP_RESPONSE_HEADER_LOGGER ); - private ServletOutputStream output; - private Set<Integer> filter; + private final TraceOutput output; + private final Set<Integer> filter; - TraceResponse( HttpServletResponse response, Set<Integer> filter ) { - super( response ); + TraceResponse(Request request, Response wrapped, Set<Integer> filter ) { + super(request, wrapped); this.filter = filter; + this.output = new TraceOutput(); + if (log.isTraceEnabled()) { + traceResponseDetails(); + } } @Override - public synchronized ServletOutputStream getOutputStream() throws IOException { + public void write(boolean last, ByteBuffer content, Callback callback) { if( log.isTraceEnabled() ) { - traceResponseDetails(); - if( output == null && ( filter == null || filter.isEmpty() || filter.contains( getStatus() ) ) ) { - output = new TraceOutput( super.getOutputStream() ); + if (filter == null || filter.isEmpty() || filter.contains(getStatus())) { + ByteBuffer view = (content != null) ? content.slice() : null; + output.extractContent(view, last); } - return output; - } else { - return super.getOutputStream(); } + super.write(last, content, callback); } private void traceResponseDetails() { StringBuilder sb = new StringBuilder(); TraceUtil.appendCorrelationContext( sb ); sb.append( "|Response=" ) - .append( getStatus() ); + .append( getStatus() ); appendHeaders( sb ); log.trace( sb.toString() ); } private void appendHeaders( StringBuilder sb ) { if( headLog.isTraceEnabled() ) { - Collection<String> names = getHeaderNames(); - for( String name : names ) { - for( String value : getHeaders( name ) ) { - sb.append( String.format(Locale.ROOT, "%n\tHeader[%s]=%s", name, value ) ); + HttpFields responseHeaders = getHeaders(); + if (responseHeaders != null) { + for (HttpField header: responseHeaders) { + sb.append( String.format(Locale.ROOT, "%n\tHeader[%s]=%s", header.getName(), header.getValue())); } } } } + } diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/filter/HSTSHandlerTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/filter/HSTSHandlerTest.java index efc55d04c..777883aef 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/filter/HSTSHandlerTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/filter/HSTSHandlerTest.java @@ -16,25 +16,25 @@ */ package org.apache.knox.gateway.filter; -import junit.framework.TestCase; import org.easymock.EasyMock; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.server.Response; import org.junit.Test; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletResponse; -import java.io.IOException; - -public class HSTSHandlerTest extends TestCase { +public class HSTSHandlerTest { @Test - public void testHandle() throws ServletException, IOException { - HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class); - response.setHeader("Strict-Transport-Security", "max-age=1000"); + public void testHandle() throws Exception { + Response response = EasyMock.createNiceMock(Response.class); + HttpFields.Mutable headers = HttpFields.build(); + EasyMock.expect(response.getHeaders()).andReturn(headers).anyTimes(); + + response.getHeaders().put("Strict-Transport-Security", "max-age=1000"); EasyMock.expectLastCall().once(); EasyMock.replay(response); HSTSHandler hstsHandler = new HSTSHandler("max-age=1000"); - hstsHandler.handle("", null, null, response); + hstsHandler.handle(null, response, null); EasyMock.verify(response); }
