From d39f3ee5c62906312e34bd4296aff5e8a49a64df Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Fri, 14 Jun 2024 07:09:14 -0700 Subject: [PATCH] Delegate compression to servlet container (#557) --- README.md | 1 - .../org/kohsuke/stapler/ResponseImpl.java | 46 +------ .../java/org/kohsuke/stapler/Stapler.java | 37 +----- .../org/kohsuke/stapler/StaplerResponse.java | 14 +- .../stapler/UncaughtExceptionFilter.java | 62 +++++++++ .../UncaughtExceptionHandler.java | 8 +- .../compression/CompressionFilter.java | 124 ------------------ .../CompressionServletResponse.java | 76 ----------- .../FilterServletOutputStream.java | 101 -------------- .../stapler/framework/io/LargeText.java | 7 +- .../stapler/json/JsonHttpResponse.java | 2 +- .../compression/CompressionFilterTest.java | 81 ------------ .../kohsuke/stapler/json/JsonBodyTest.java | 5 - docs/compression.adoc | 39 ------ .../kohsuke/stapler/jelly/CompressTag.java | 2 + .../stapler/jelly/DefaultScriptInvoker.java | 40 +----- 16 files changed, 80 insertions(+), 565 deletions(-) create mode 100644 core/src/main/java/org/kohsuke/stapler/UncaughtExceptionFilter.java rename core/src/main/java/org/kohsuke/stapler/{compression => }/UncaughtExceptionHandler.java (87%) delete mode 100644 core/src/main/java/org/kohsuke/stapler/compression/CompressionFilter.java delete mode 100644 core/src/main/java/org/kohsuke/stapler/compression/CompressionServletResponse.java delete mode 100644 core/src/main/java/org/kohsuke/stapler/compression/FilterServletOutputStream.java delete mode 100644 core/src/test/java/org/kohsuke/stapler/compression/CompressionFilterTest.java delete mode 100644 docs/compression.adoc diff --git a/README.md b/README.md index 6f90872871..f00a99dd0e 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,6 @@ * [Getting Started](docs/getting-started.adoc) * Developer Reference * [URL binding reference](docs/reference.adoc) - * [HTTP compression](docs/compression.adoc) * [Internationalization support](docs/i18n.adoc) * [Javadoc](https://javadoc.jenkins.io/component/stapler/) * [Stapler jelly tag library reference and XML Schema](docs/jelly-taglib-ref.adoc) diff --git a/core/src/main/java/org/kohsuke/stapler/ResponseImpl.java b/core/src/main/java/org/kohsuke/stapler/ResponseImpl.java index 1d1e4ab5e2..98a00ec723 100644 --- a/core/src/main/java/org/kohsuke/stapler/ResponseImpl.java +++ b/core/src/main/java/org/kohsuke/stapler/ResponseImpl.java @@ -39,7 +39,6 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; -import java.util.zip.GZIPOutputStream; import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletRequest; @@ -47,8 +46,6 @@ import javax.servlet.http.HttpServletResponseWrapper; import net.sf.json.JsonConfig; import org.apache.commons.io.IOUtils; -import org.kohsuke.stapler.compression.CompressionFilter; -import org.kohsuke.stapler.compression.FilterServletOutputStream; import org.kohsuke.stapler.export.DataWriter; import org.kohsuke.stapler.export.ExportConfig; import org.kohsuke.stapler.export.Flavor; @@ -293,7 +290,7 @@ public void serveExposedBean(StaplerRequest req, Object exposedBean, ExportConfi String pad = null; Flavor flavor = config.getFlavor(); setContentType(flavor.contentType); - Writer w = getCompressedWriter(req); + Writer w = getWriter(); if (flavor == Flavor.JSON || flavor == Flavor.JSONP) { // for compatibility reasons, accept JSON for JSONP as well. @@ -347,51 +344,14 @@ private void writeOne(TreePruner pruner, DataWriter dw, Object item) throws IOEx p.writeTo(item, pruner, dw); } - private boolean acceptsGzip(HttpServletRequest req) { - String acceptEncoding = req.getHeader("Accept-Encoding"); - return acceptEncoding != null && acceptEncoding.contains("gzip"); - } - @Override public OutputStream getCompressedOutputStream(HttpServletRequest req) throws IOException { - if (mode != null) { // we already made the call and created OutputStream/Writer - return getOutputStream(); - } - - if (!acceptsGzip(req)) { - return getOutputStream(); // compression not applicable here - } - - if (CompressionFilter.activate(req)) { - return getOutputStream(); // CompressionFilter will set up compression. no need to do anything - } - - // CompressionFilter not available, so do it on our own. - // see CompressionFilter for why this is not desirable - setHeader("Content-Encoding", "gzip"); - return recordOutput( - new FilterServletOutputStream(new GZIPOutputStream(super.getOutputStream()), super.getOutputStream())); + return getOutputStream(); } @Override public Writer getCompressedWriter(HttpServletRequest req) throws IOException { - if (mode != null) { - return getWriter(); - } - - if (!acceptsGzip(req)) { - return getWriter(); // compression not available - } - - if (CompressionFilter.activate(req)) { - return getWriter(); // CompressionFilter will set up compression. no need to do anything - } - - // CompressionFilter not available, so do it on our own. - // see CompressionFilter for why this is not desirable - setHeader("Content-Encoding", "gzip"); - return recordOutput(new PrintWriter( - new OutputStreamWriter(new GZIPOutputStream(super.getOutputStream()), getCharacterEncoding()))); + return getWriter(); } @Override diff --git a/core/src/main/java/org/kohsuke/stapler/Stapler.java b/core/src/main/java/org/kohsuke/stapler/Stapler.java index 9c639657f0..a63bd98622 100644 --- a/core/src/main/java/org/kohsuke/stapler/Stapler.java +++ b/core/src/main/java/org/kohsuke/stapler/Stapler.java @@ -49,7 +49,6 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -611,21 +610,6 @@ boolean serveStaticResource( // a comprehensive discussion on this topic rsp.setHeader("X-Content-Type-Options", "nosniff"); - int idx = fileName.lastIndexOf('.'); - String ext = fileName.substring(idx + 1); - - OutputStream out = null; - if (mimeType.startsWith("text/") || TEXT_FILES.contains(ext)) { - // Need to duplicate this logic from ResponseImpl.getCompressedOutputStream, - // since we want to set content length if we are not using encoding. - String acceptEncoding = req.getHeader("Accept-Encoding"); - if (acceptEncoding != null && acceptEncoding.contains("gzip")) { - // with gzip compression, Content-Length header needs to indicate the # of bytes after compression, - // so we can't compute it upfront. - out = rsp.getCompressedOutputStream(req); - } - } - // somewhat limited implementation of the partial GET String range = req.getHeader("Range"); if (range != null @@ -664,18 +648,11 @@ boolean serveStaticResource( } } - if (out == null) { - if (contentLength != -1) { - rsp.setHeader("Content-Length", Long.toString(contentLength)); - } - out = rsp.getOutputStream(); - } - - byte[] buf = new byte[1024]; - int len; - while ((len = in.read(buf)) > 0) { - out.write(buf, 0, len); + if (contentLength != -1) { + rsp.setHeader("Content-Length", Long.toString(contentLength)); } + OutputStream out = rsp.getOutputStream(); + in.transferTo(out); out.close(); return true; } finally { @@ -1109,12 +1086,6 @@ public static Stapler getCurrent() { private static final Logger LOGGER = Logger.getLogger(Stapler.class.getName()); - /** - * Extensions that look like text files. - */ - private static final Set TEXT_FILES = new HashSet<>( - Arrays.asList("css", "js", "html", "txt", "java", "htm", "c", "cpp", "h", "rb", "pl", "py", "xml", "json")); - /** * Get raw servlet path (decoded in TokenList). */ diff --git a/core/src/main/java/org/kohsuke/stapler/StaplerResponse.java b/core/src/main/java/org/kohsuke/stapler/StaplerResponse.java index fe43374f1e..8282b7c238 100644 --- a/core/src/main/java/org/kohsuke/stapler/StaplerResponse.java +++ b/core/src/main/java/org/kohsuke/stapler/StaplerResponse.java @@ -221,21 +221,15 @@ default void serveExposedBean(StaplerRequest req, Object exposedBean, ExportConf } /** - * Works like {@link #getOutputStream()} but tries to send the response - * with gzip compression if the client supports it. - * - *

- * This method is useful for sending out a large text content. - * - * @param req - * Used to determine whether the client supports compression + * @deprecated use {@link #getOutputStream} */ + @Deprecated OutputStream getCompressedOutputStream(HttpServletRequest req) throws IOException; /** - * Works like {@link #getCompressedOutputStream(HttpServletRequest)} but this - * method is for {@link #getWriter()}. + * @deprecated use {@link #getWriter} */ + @Deprecated Writer getCompressedWriter(HttpServletRequest req) throws IOException; /** diff --git a/core/src/main/java/org/kohsuke/stapler/UncaughtExceptionFilter.java b/core/src/main/java/org/kohsuke/stapler/UncaughtExceptionFilter.java new file mode 100644 index 0000000000..cf1715f0fb --- /dev/null +++ b/core/src/main/java/org/kohsuke/stapler/UncaughtExceptionFilter.java @@ -0,0 +1,62 @@ +package org.kohsuke.stapler; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.io.IOException; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletContext; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +public class UncaughtExceptionFilter implements Filter { + private ServletContext context; + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + context = filterConfig.getServletContext(); + } + + @Override + public void doFilter(ServletRequest req, ServletResponse rsp, FilterChain filterChain) + throws IOException, ServletException { + try { + filterChain.doFilter(req, rsp); + } catch (IOException | Error | RuntimeException | ServletException e) { + if (DISABLED) { + throw e; + } + reportException(e, (HttpServletRequest) req, (HttpServletResponse) rsp); + } + } + + private void reportException(Throwable e, HttpServletRequest req, HttpServletResponse rsp) + throws IOException, ServletException { + getUncaughtExceptionHandler(context).reportException(e, context, req, rsp); + } + + @Override + public void destroy() {} + + public static void setUncaughtExceptionHandler(ServletContext context, UncaughtExceptionHandler handler) { + context.setAttribute(UncaughtExceptionHandler.class.getName(), handler); + } + + public static UncaughtExceptionHandler getUncaughtExceptionHandler(ServletContext context) { + UncaughtExceptionHandler h = + (UncaughtExceptionHandler) context.getAttribute(UncaughtExceptionHandler.class.getName()); + if (h == null) { + h = UncaughtExceptionHandler.DEFAULT; + } + return h; + } + + /** + * Disables the effect of {@link #setUncaughtExceptionHandler}, letting all errors be rethrown. + */ + @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "for script console") + public static boolean DISABLED = Boolean.getBoolean(UncaughtExceptionFilter.class.getName() + ".disabled"); +} diff --git a/core/src/main/java/org/kohsuke/stapler/compression/UncaughtExceptionHandler.java b/core/src/main/java/org/kohsuke/stapler/UncaughtExceptionHandler.java similarity index 87% rename from core/src/main/java/org/kohsuke/stapler/compression/UncaughtExceptionHandler.java rename to core/src/main/java/org/kohsuke/stapler/UncaughtExceptionHandler.java index efdeef72f1..b6b3aa8117 100644 --- a/core/src/main/java/org/kohsuke/stapler/compression/UncaughtExceptionHandler.java +++ b/core/src/main/java/org/kohsuke/stapler/UncaughtExceptionHandler.java @@ -1,4 +1,4 @@ -package org.kohsuke.stapler.compression; +package org.kohsuke.stapler; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.IOException; @@ -10,13 +10,9 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.kohsuke.stapler.Stapler; /** - * Handles an exception caught by {@link CompressionFilter}. - * - * See {@link CompressionFilter} javadoc for why this exception needs to be handled - * by us and can't just be handled by the servlet container like it does all others. + * Handles an exception caught by {@link UncaughtExceptionFilter}. * * @author Kohsuke Kawaguchi */ diff --git a/core/src/main/java/org/kohsuke/stapler/compression/CompressionFilter.java b/core/src/main/java/org/kohsuke/stapler/compression/CompressionFilter.java deleted file mode 100644 index dbc60eb14e..0000000000 --- a/core/src/main/java/org/kohsuke/stapler/compression/CompressionFilter.java +++ /dev/null @@ -1,124 +0,0 @@ -package org.kohsuke.stapler.compression; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import java.io.IOException; -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.ServletContext; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -/** - * Pimps up {@link HttpServletResponse} so that it understands "Content-Encoding: gzip" and compress the response. - * - *

- * When exceptions are processed within web applications, different unrelated parts of the webapp can end up calling - * {@link HttpServletResponse#getOutputStream()}. This fundamentally doesn't work with the notion that the application - * needs to process "Content-Encoding:gzip" on its own. Such app has to maintain a GZIP output stream on its own, - * since {@link HttpServletResponse} doesn't know that its output is written through a compressed stream. - * - *

- * Another place this break-down can be seen is when a servlet throws an exception that the container handles. - * It tries to render an error page, but of course it wouldn't know that "Content-Encoding:gzip" is set, so it - * will fail to write in the correct format. - * - *

- * The only way to properly fix this is to make {@link HttpServletResponse} smart enough that it returns - * the compression-transparent stream from {@link HttpServletResponse#getOutputStream()} (and it would also - * have to process the exception thrown from the app.) This filter does exactly that. - * - * @author Kohsuke Kawaguchi - */ -public class CompressionFilter implements Filter { - private ServletContext context; - - @Override - public void init(FilterConfig filterConfig) throws ServletException { - context = filterConfig.getServletContext(); - } - - @Override - public void doFilter(ServletRequest _req, ServletResponse _rsp, FilterChain filterChain) - throws IOException, ServletException { - Object old1 = swapAttribute(_req, CompressionFilter.class, true); - - CompressionServletResponse rsp = new CompressionServletResponse((HttpServletResponse) _rsp); - Object old2 = swapAttribute(_req, CompressionServletResponse.class, rsp); - - try { - filterChain.doFilter(_req, rsp); - } catch (IOException | Error | RuntimeException | ServletException e) { - if (DISABLED) { - throw e; - } - reportException(e, (HttpServletRequest) _req, rsp); - } finally { - rsp.close(); - - _req.setAttribute(CompressionFilter.class.getName(), old1); - _req.setAttribute(CompressionServletResponse.class.getName(), old2); - } - } - - private Object swapAttribute(ServletRequest req, Class key, Object value) { - Object old = req.getAttribute(key.getName()); - req.setAttribute(key.getName(), value); - return old; - } - - private void reportException(Throwable e, HttpServletRequest req, HttpServletResponse rsp) - throws IOException, ServletException { - getUncaughtExceptionHandler(context).reportException(e, context, req, rsp); - } - - @Override - public void destroy() {} - - public static void setUncaughtExceptionHandler(ServletContext context, UncaughtExceptionHandler handler) { - context.setAttribute(UncaughtExceptionHandler.class.getName(), handler); - } - - public static UncaughtExceptionHandler getUncaughtExceptionHandler(ServletContext context) { - UncaughtExceptionHandler h = - (UncaughtExceptionHandler) context.getAttribute(UncaughtExceptionHandler.class.getName()); - if (h == null) { - h = UncaughtExceptionHandler.DEFAULT; - } - return h; - } - - /** - * Is this request already wrapped into {@link CompressionFilter}? - */ - public static boolean has(ServletRequest req) { - return req.getAttribute(CompressionServletResponse.class.getName()) != null; - } - - /** - * Is this request already wrapped into {@link CompressionFilter}, - * activate that, so that {@link ServletResponse#getOutputStream()} will return - * a stream that automatically handles compression. - */ - public static boolean activate(ServletRequest req) throws IOException { - CompressionServletResponse rsp = - (CompressionServletResponse) req.getAttribute(CompressionServletResponse.class.getName()); - if (rsp != null) { - rsp.activate(); - return true; - } else { - return false; - } - } - - /** - * Disables the effect of {@link #setUncaughtExceptionHandler}, letting all errors be rethrown. - * Despite its name, this flag does not disable {@link CompressionFilter} itself! - * Rather use {@code DefaultScriptInvoker.COMPRESS_BY_DEFAULT}. - */ - @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Legacy switch.") - public static boolean DISABLED = Boolean.getBoolean(CompressionFilter.class.getName() + ".disabled"); -} diff --git a/core/src/main/java/org/kohsuke/stapler/compression/CompressionServletResponse.java b/core/src/main/java/org/kohsuke/stapler/compression/CompressionServletResponse.java deleted file mode 100644 index 17f76e42b0..0000000000 --- a/core/src/main/java/org/kohsuke/stapler/compression/CompressionServletResponse.java +++ /dev/null @@ -1,76 +0,0 @@ -package org.kohsuke.stapler.compression; - -import java.io.IOException; -import java.io.OutputStreamWriter; -import java.io.PrintWriter; -import java.util.zip.GZIPOutputStream; -import javax.servlet.ServletOutputStream; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpServletResponseWrapper; - -/** - * {@link HttpServletResponse} that recognizes Content-Encoding: gzip in the response header - * and acts accordingly. - * - * @author Kohsuke Kawaguchi - */ -public class CompressionServletResponse extends HttpServletResponseWrapper { - /** - * If not null, we are compressing the stream. - */ - private ServletOutputStream stream; - - private PrintWriter writer; - - public CompressionServletResponse(HttpServletResponse response) { - super(response); - } - - /** - * If we've already inserted gzip compression filter, then we can't let the content length set from the app - * because that refers to the size of the uncompressed content, where we actually need the size of the compressed - * content. - * - * For the same reason, if the content length is set before we can decide whether to - */ - @Override - public void setContentLength(int len) { - if (stream != null) { - return; - } - super.setContentLength(len); - } - - @Override - public PrintWriter getWriter() throws IOException { - if (writer != null) { - return writer; - } - if (stream != null) { - writer = new PrintWriter(new OutputStreamWriter(stream, getCharacterEncoding())); - return writer; - } - return super.getWriter(); - } - - @Override - public ServletOutputStream getOutputStream() throws IOException { - if (stream != null) { - return stream; - } - return super.getOutputStream(); - } - - public void activate() throws IOException { - if (stream == null) { - super.setHeader("Content-Encoding", "gzip"); - stream = new FilterServletOutputStream(new GZIPOutputStream(super.getOutputStream()), stream); - } - } - - public void close() throws IOException { - if (stream != null) { - stream.close(); - } - } -} diff --git a/core/src/main/java/org/kohsuke/stapler/compression/FilterServletOutputStream.java b/core/src/main/java/org/kohsuke/stapler/compression/FilterServletOutputStream.java deleted file mode 100644 index 7bcf804473..0000000000 --- a/core/src/main/java/org/kohsuke/stapler/compression/FilterServletOutputStream.java +++ /dev/null @@ -1,101 +0,0 @@ -package org.kohsuke.stapler.compression; - -import java.io.FilterOutputStream; -import java.io.IOException; -import java.io.OutputStream; -import javax.servlet.ServletOutputStream; -import javax.servlet.WriteListener; - -/** - * {@link ServletOutputStream} that writes to the specified output stream. - * - * @author Kohsuke Kawaguchi - */ -public class FilterServletOutputStream extends ServletOutputStream { - private final OutputStream out; - private final ServletOutputStream realSream; - - /** - * Whether the stream is closed; implicitly initialized to false. - */ - private volatile boolean closed; - - /** - * Object used to prevent a race on the 'closed' instance variable. - */ - private final Object closeLock = new Object(); - - /** - * Constructs a new {@link FilterOutputStream}. - * @param out the stream that sits above the realStream, performing some filtering. This must be eventually delegating eventual writes to {@code realStream}. - * @param realStream the actual underlying ServletOutputStream from the container. Used to check the {@link #isReady()} state and to add {@link WriteListener}s. - */ - public FilterServletOutputStream(OutputStream out, ServletOutputStream realStream) { - this.out = out; - this.realSream = realStream; - } - - @Override - public void write(int b) throws IOException { - out.write(b); - } - - @Override - public void write(byte[] b) throws IOException { - out.write(b); - } - - @Override - public void write(byte[] b, int off, int len) throws IOException { - out.write(b, off, len); - } - - @Override - public void close() throws IOException { - if (closed) { - return; - } - synchronized (closeLock) { - if (closed) { - return; - } - closed = true; - } - - Throwable flushException = null; - try { - flush(); - } catch (Throwable e) { - flushException = e; - throw e; - } finally { - if (flushException == null) { - out.close(); - } else { - try { - out.close(); - } catch (Throwable closeException) { - if (flushException != closeException) { - closeException.addSuppressed(flushException); - } - throw closeException; - } - } - } - } - - @Override - public void flush() throws IOException { - out.flush(); - } - - @Override - public boolean isReady() { - return realSream.isReady(); - } - - @Override - public void setWriteListener(WriteListener writeListener) { - realSream.setWriteListener(writeListener); - } -} diff --git a/core/src/main/java/org/kohsuke/stapler/framework/io/LargeText.java b/core/src/main/java/org/kohsuke/stapler/framework/io/LargeText.java index 2ef9083930..48485082ea 100644 --- a/core/src/main/java/org/kohsuke/stapler/framework/io/LargeText.java +++ b/core/src/main/java/org/kohsuke/stapler/framework/io/LargeText.java @@ -302,12 +302,7 @@ protected void setContentType(StaplerResponse rsp) { } protected Writer createWriter(StaplerRequest req, StaplerResponse rsp, long size) throws IOException { - // when sending big text, try compression. don't bother if it's small - if (size > 4096) { - return rsp.getCompressedWriter(req); - } else { - return rsp.getWriter(); - } + return rsp.getWriter(); } /** diff --git a/core/src/main/java/org/kohsuke/stapler/json/JsonHttpResponse.java b/core/src/main/java/org/kohsuke/stapler/json/JsonHttpResponse.java index 0d2e6479f6..040f5b62a8 100644 --- a/core/src/main/java/org/kohsuke/stapler/json/JsonHttpResponse.java +++ b/core/src/main/java/org/kohsuke/stapler/json/JsonHttpResponse.java @@ -50,7 +50,7 @@ public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object nod } if (responseJson != null) { rsp.setContentType("application/json;charset=UTF-8"); - try (Writer w = rsp.getCompressedWriter(req)) { + try (Writer w = rsp.getWriter()) { responseJson.write(w); } } diff --git a/core/src/test/java/org/kohsuke/stapler/compression/CompressionFilterTest.java b/core/src/test/java/org/kohsuke/stapler/compression/CompressionFilterTest.java deleted file mode 100644 index e70f6fc4b6..0000000000 --- a/core/src/test/java/org/kohsuke/stapler/compression/CompressionFilterTest.java +++ /dev/null @@ -1,81 +0,0 @@ -package org.kohsuke.stapler.compression; - -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.net.http.HttpClient; -import java.net.http.HttpRequest; -import java.net.http.HttpResponse; -import java.util.Arrays; -import java.util.EnumSet; -import java.util.zip.GZIPInputStream; -import java.util.zip.GZIPOutputStream; -import javax.servlet.DispatcherType; -import org.apache.commons.io.IOUtils; -import org.eclipse.jetty.servlet.ServletContextHandler; -import org.kohsuke.stapler.StaplerRequest; -import org.kohsuke.stapler.StaplerResponse; -import org.kohsuke.stapler.test.JettyTestCase; - -public class CompressionFilterTest extends JettyTestCase { - - @Override - protected void configure(ServletContextHandler context) { - super.configure(context); - context.addFilter(CompressionFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST)); - } - - public void testDoubleCompression() throws Exception { - for (String endpoint : Arrays.asList("autoZip", "ownZip")) { - HttpRequest httpRequest = HttpRequest.newBuilder(this.url.toURI().resolve(endpoint)) - .GET() - .header("Accept-Encoding", "gzip") - .build(); - HttpClient httpClient = HttpClient.newHttpClient(); - HttpResponse response = httpClient.send(httpRequest, HttpResponse.BodyHandlers.ofByteArray()); - assertEquals(200, response.statusCode()); - byte[] data = IOUtils.toByteArray(new GZIPInputStream(new ByteArrayInputStream(response.body()))); - assertEquals(new String(data), CONTENT); - } - } - - /** - * Turns out {@link #testDoubleCompression()} was insufficient to properly test the case, - * as HttpURLConnection appears to ignores the content-length header and read the response to the end. - */ - public void testDoubleCompression2() throws Exception { - for (String endpoint : Arrays.asList("autoZip", "ownZip")) { - HttpRequest httpRequest = HttpRequest.newBuilder(this.url.toURI().resolve(endpoint)) - .GET() - .header("Accept-Encoding", "gzip") - .build(); - HttpClient httpClient = HttpClient.newHttpClient(); - HttpResponse response = httpClient.send(httpRequest, HttpResponse.BodyHandlers.ofByteArray()); - assertEquals(200, response.statusCode()); - byte[] data = IOUtils.toByteArray(new GZIPInputStream(new ByteArrayInputStream(response.body()))); - assertEquals(new String(data), CONTENT); - } - } - - /** - * Simulate servlets that tries to handle Content-Encoding on its own - */ - public void doOwnZip(StaplerResponse rsp) throws IOException { - rsp.setHeader("Content-Encoding", "gzip"); - byte[] content = CONTENT.getBytes(); - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream o = new GZIPOutputStream(baos); - o.write(content); - o.close(); - - rsp.setContentLength(baos.size()); - rsp.getOutputStream().write(baos.toByteArray()); - } - - public void doAutoZip(StaplerRequest req, StaplerResponse rsp) throws IOException { - byte[] content = CONTENT.getBytes(); - rsp.getCompressedOutputStream(req).write(content); - } - - private static final String CONTENT = "Hello World"; -} diff --git a/core/src/test/java/org/kohsuke/stapler/json/JsonBodyTest.java b/core/src/test/java/org/kohsuke/stapler/json/JsonBodyTest.java index 87ee2072bc..51f49a3ffa 100644 --- a/core/src/test/java/org/kohsuke/stapler/json/JsonBodyTest.java +++ b/core/src/test/java/org/kohsuke/stapler/json/JsonBodyTest.java @@ -15,11 +15,6 @@ public void testSmokes() throws Exception { WebResponse response = createWebClient().getPage(req).getWebResponse(); assertEquals("application/json", response.getContentType()); assertEquals("{\"x\":20,\"y\":10}", response.getContentAsString()); - // and then with compression: - req.setAdditionalHeader("Accept-Encoding", "gzip"); - response = createWebClient().getPage(req).getWebResponse(); - assertEquals("application/json", response.getContentType()); - assertEquals("{\"x\":20,\"y\":10}", response.getContentAsString()); } @JsonResponse diff --git a/docs/compression.adoc b/docs/compression.adoc deleted file mode 100644 index f3f3ebb591..0000000000 --- a/docs/compression.adoc +++ /dev/null @@ -1,39 +0,0 @@ -== Static resources are sent with gzip - -When clients request static resources, stapler will send it with gzip as -the content encoding whenever it can, to minimize the size of the data -to be transferred. This only happens when the MIME type of the resource -is "text/*", or the file extension is one of the well-known text file -type. - -This happens all transparently, so there's nothing you nor the browser -have to do. - -== Sending compressed stream from the `doXXX` methods - -If you are writing action methods that serve text content, and if you'd -like to compress the stream, you can call -`StaplerResponse.getCompressedOutputStream` (instead of -`StaplerResponse.getOutputStream`) or -`StaplerResponse.getCompressedWriter` (instead of -`StaplerResponse.getWriter`) and then write to them normally. - -If the client doesn't support HTTP compression, these methods will -silently revert to the normal uncompressed streams. - -== Using compression from Jelly view - -If your view script produces text content, and if you'd like to compress -the stream, put `` as the root -element of your view, like this: - -[source,xml] ----- - - - - ... ----- - -If the client doesn't support HTTP compression, stapler will silently -revert to the normal uncompressed stream. diff --git a/jelly/src/main/java/org/kohsuke/stapler/jelly/CompressTag.java b/jelly/src/main/java/org/kohsuke/stapler/jelly/CompressTag.java index e4b673c74d..64680704ef 100644 --- a/jelly/src/main/java/org/kohsuke/stapler/jelly/CompressTag.java +++ b/jelly/src/main/java/org/kohsuke/stapler/jelly/CompressTag.java @@ -34,7 +34,9 @@ * for this output. * * @author Kohsuke Kawaguchi + * @deprecated removed without replacement */ +@Deprecated public class CompressTag extends AbstractStaplerTag { /** * Doesn't particularly do anything as the actual processing diff --git a/jelly/src/main/java/org/kohsuke/stapler/jelly/DefaultScriptInvoker.java b/jelly/src/main/java/org/kohsuke/stapler/jelly/DefaultScriptInvoker.java index 59d1bb864e..199a8453fd 100644 --- a/jelly/src/main/java/org/kohsuke/stapler/jelly/DefaultScriptInvoker.java +++ b/jelly/src/main/java/org/kohsuke/stapler/jelly/DefaultScriptInvoker.java @@ -24,7 +24,6 @@ package org.kohsuke.stapler.jelly; import edu.umd.cs.findbugs.annotations.NonNull; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.BufferedOutputStream; import java.io.IOException; import java.io.OutputStream; @@ -36,7 +35,6 @@ import org.apache.commons.jelly.Script; import org.apache.commons.jelly.XMLOutput; import org.apache.commons.jelly.XMLOutputFactory; -import org.apache.commons.jelly.impl.TagScript; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; @@ -80,18 +78,6 @@ protected XMLOutput createXMLOutput(StaplerRequest req, StaplerResponse rsp, Scr return output; } - private boolean doCompression(Script script) { - if (COMPRESS_BY_DEFAULT) { - return true; - } - if (script instanceof TagScript ts) { - if (ts.getLocalName().equals("compress")) { - return true; - } - } - return false; - } - private interface OutputStreamSupplier { @NonNull OutputStream get() throws IOException; @@ -123,9 +109,7 @@ protected OutputStream createOutputStream(StaplerRequest req, StaplerResponse rs throws IOException { OutputStreamSupplier out = new LazyOutputStreamSupplier(() -> { req.getWebApp().getDispatchValidator().requireDispatchAllowed(req, rsp); - return doCompression(script) - ? rsp.getCompressedOutputStream(req) - : new BufferedOutputStream(rsp.getOutputStream()); + return new BufferedOutputStream(rsp.getOutputStream()); }); return new OutputStream() { @Override @@ -196,26 +180,4 @@ public XMLOutput createXMLOutput(Writer writer, boolean escapeText) { } return HTMLWriterOutput.create(writer, escapeText); } - - /** - * Whether gzip compression of the dynamic content is enabled by default or not. - * - *

- * For non-trivial web applications, where the performance matters, it is normally a good trade-off to spend - * a bit of CPU cycles to compress data. This is because: - * - *

- * - * Stuff rendered by Jelly is predominantly text, so the compression would work well. - * - * @see Latency Trumps All - */ - @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Legacy switch.") - public static boolean COMPRESS_BY_DEFAULT = - Boolean.parseBoolean(System.getProperty(DefaultScriptInvoker.class.getName() + ".compress", "true")); }