From f70f57590e167a21d7bf05aa3d4694ed1286e043 Mon Sep 17 00:00:00 2001 From: Kai Moritz Date: Thu, 2 Aug 2012 08:40:23 +0200 Subject: [PATCH] =?utf8?q?Grundlegende=20=C3=9Cberarbeitung=20des=20Accele?= =?utf8?q?ratorFilter?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Es wurden diverse schwere Fehler im Zusammenhang mit der automatischen Komprimierung von Antworten und dem Puffern der Ausgabe behoben. --- .../src/main/resources/config.xml | 4 - .../juplo/cachecontrol/AcceleratorFilter.java | 447 +++++++----------- cachecontrol/src/test/resources/config.xml | 4 - 3 files changed, 171 insertions(+), 284 deletions(-) diff --git a/cachecontrol-example-jsp/src/main/resources/config.xml b/cachecontrol-example-jsp/src/main/resources/config.xml index 951402f0..d4c56560 100644 --- a/cachecontrol-example-jsp/src/main/resources/config.xml +++ b/cachecontrol-example-jsp/src/main/resources/config.xml @@ -10,10 +10,6 @@ - - - - diff --git a/cachecontrol/src/main/java/de/halbekunst/juplo/cachecontrol/AcceleratorFilter.java b/cachecontrol/src/main/java/de/halbekunst/juplo/cachecontrol/AcceleratorFilter.java index 501c3a27..f0037bf1 100644 --- a/cachecontrol/src/main/java/de/halbekunst/juplo/cachecontrol/AcceleratorFilter.java +++ b/cachecontrol/src/main/java/de/halbekunst/juplo/cachecontrol/AcceleratorFilter.java @@ -1,8 +1,8 @@ package de.halbekunst.juplo.cachecontrol; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.text.ParseException; import java.text.SimpleDateFormat; @@ -40,16 +40,17 @@ public class AcceleratorFilter implements Filter { private final static Map EMPTY = Collections.unmodifiableMap(new HashMap()); + public final static Integer DEFAULT_BUFFER_SIZE = 1024; public final static String REQUEST_URI_ATTRIBUTE = "javax.servlet.include.request_uri"; public final static String RESPONSE_WRAPPER = AcceleratorFilter.class.getName() + ".RESPONSE_WRAPPER"; @Autowired CacheControl cacheControl; - @Autowired(required=true) Integer buffer; - @Autowired(required=true) String eTag; - @Autowired(required=true) Boolean weak; - @Autowired(required=true) Long lastModified; - @Autowired(required=true) Integer cacheSeconds; + @Autowired(required=false) Integer defaultBufferSize = DEFAULT_BUFFER_SIZE; + @Autowired String eTag; + @Autowired Boolean weak; + @Autowired Long lastModified; + @Autowired Integer cacheSeconds; @Override @@ -60,6 +61,7 @@ public class AcceleratorFilter implements Filter { return; } + // TODO: Das macht so wahrscheinlich keinen Sinn... /** Prüfen, ob es sich um eine Anfrage für einen JSP-Include handelt */ if (request.getAttribute(REQUEST_URI_ATTRIBUTE) != null) { log.debug("Includes cannot be accelerated"); @@ -90,14 +92,13 @@ public class AcceleratorFilter implements Filter { private final HttpServletRequest request; private final HttpServletResponse response; - boolean zipped; // CacheControll greift direkt auf dieses Flag zu! - boolean forceCompression = false; + private final AcceleratorServletOutputStream out; - private CountingServletOutputStream out; private ServletOutputStream stream; private PrintWriter writer; private boolean guessing = true; + protected boolean zipped = false; // << CacheControll greift direkt auf diese Variable zu! private long now; private int status; @@ -108,6 +109,15 @@ public class AcceleratorFilter implements Filter { private boolean weak; private Map cacheParams; + /** Für den AcceleratorOutputStream */ + private boolean committed = false; + private OutputStream os = null; + private int bufferSize; + private byte[] buffer; + private int pos = 0; + private int size = 0; + + AccelerationWrapper(HttpServletRequest request, HttpServletResponse response) throws IOException { @@ -122,7 +132,6 @@ public class AcceleratorFilter implements Filter { weak = AcceleratorFilter.this.weak; cacheParams = new HashMap(); - zipped = false; Enumeration values = request.getHeaders(Headers.HEADER_ACCEPT_ENCODING); while (values.hasMoreElements()) { String value = (String) values.nextElement(); @@ -131,39 +140,26 @@ public class AcceleratorFilter implements Filter { break; } } - if (zipped) - out = new GZIPServletOutputStream(); - else - out = new CountingServletOutputStream(); + + out = new AcceleratorServletOutputStream(); } - public void finish() throws IOException { + + private void finish() throws IOException { + flushBuffer(); out.close(); } - @Override public void setStatus(int sc) { response.setStatus(sc); status = sc; - try { - cacheControl.decorate(request, response, this); - } - catch (Exception e) { - log.error("Error while decorating response", e); - } } @Override public void setStatus(int sc, String sm) { response.setStatus(sc,sm); status = sc; - try { - cacheControl.decorate(request, response, this); - } - catch (Exception e) { - log.error("Error while decorating response", e); - } } @Override @@ -290,7 +286,7 @@ public class AcceleratorFilter implements Filter { throw new IllegalStateException("ServletOutputStream and PrintWriter cannot be requested both!"); if (writer == null) { - writer = new PrintWriter(out); + writer = new PrintWriter(new OutputStreamWriter(out, response.getCharacterEncoding())); } return writer; @@ -305,63 +301,140 @@ public class AcceleratorFilter implements Filter { } @Override - public void setBufferSize(int size) { - - out.setBuffer(size); - response.setBufferSize(size); + public int getBufferSize() { + return bufferSize; } @Override - public void flushBuffer() throws IOException { - - forceCompression = true; - cacheControl.decorate(request, response, this); - response.flushBuffer(); + public void setBufferSize(int size) { + if (this.size > 0) + throw new IllegalStateException("cannot change buffer size, because content was already written!"); + bufferSize = size; + buffer = new byte[size]; } @Override public void resetBuffer() { - - response.resetBuffer(); - try { - if (zipped) - out = new GZIPServletOutputStream(); - else - out = new CountingServletOutputStream(); - } - catch (IOException e) { - throw new IllegalStateException(e); - } + if (committed) + throw new IllegalStateException("cannot reset buffer, because response is already commited!"); + pos = 0; stream = null; writer = null; } @Override public void reset() { - + if (committed) + throw new IllegalStateException("cannot reset response, because response is already commited!"); + /** + * Da committed==false gilt, wurde die Dekoration noch nicht angestßen + * und muss entsprechend auch nicht rückgängig gemacht werden! + */ response.reset(); - try { - if (zipped) - out = new GZIPServletOutputStream(); - else - out = new CountingServletOutputStream(); - } - catch (IOException e) { - throw new IllegalStateException(e); - } + pos = 0; stream = null; writer = null; + } + + @Override + public void flushBuffer() throws IOException { + if (writer != null) + writer.flush(); + else if (stream != null) + stream.flush(); - /** Cookies has been cleared! Reinitialize decorator... */ - forceCompression = false; - cacheControl.init(this); + out.flush(); + } + + @Override + public void addCookie(Cookie cookie) { + // TODO: Je nach Vary-Einstellung ETag anpassen? + response.addCookie(cookie); + } + + @Override + public boolean containsHeader(String name) { + return response.containsHeader(name); + } + + @Override + public String encodeURL(String url) { + return response.encodeURL(url); + } + + @Override + public String encodeRedirectURL(String url) { + return response.encodeRedirectURL(url); + } + + @Override + public String encodeUrl(String url) { + return response.encodeUrl(url); + } + + @Override + public String encodeRedirectUrl(String url) { + return response.encodeRedirectUrl(url); + } + + @Override + public void sendError(int sc, String msg) throws IOException { + response.sendError(sc,msg); + } + + @Override + public void sendError(int sc) throws IOException { + response.sendError(sc); + } + + @Override + public void sendRedirect(String location) throws IOException { + response.sendRedirect(location); + } + + @Override + public String getCharacterEncoding() { + return response.getCharacterEncoding(); + } + + @Override + public String getContentType() { + return response.getContentType(); + } + + @Override + public void setCharacterEncoding(String charset) { + // TODO: Je nach Vary-Einstellung ETag anpassen? + response.setCharacterEncoding(charset); + } + + @Override + public void setContentType(String type) { + // TODO: Je nach Vary-Einstellung ETag anpassen? + response.setContentType(type); + } + + @Override + public boolean isCommitted() { + return committed; + } + + @Override + public void setLocale(Locale loc) { + // TODO: Je nach Vary-Einstellung ETag anpassen? + response.setLocale(loc); + } + + @Override + public Locale getLocale() { + return getLocale(); } @Override public boolean isZipped() { - return out.isZipped(); + return zipped; } @Override @@ -552,239 +625,61 @@ public class AcceleratorFilter implements Filter { } } - @Override - public void addCookie(Cookie cookie) { - // TODO: Je nach Vary-Einstellung ETag anpassen? - response.addCookie(cookie); - } - @Override - public boolean containsHeader(String name) { - return response.containsHeader(name); - } - - @Override - public String encodeURL(String url) { - return response.encodeURL(url); - } - - @Override - public String encodeRedirectURL(String url) { - return response.encodeRedirectURL(url); - } - - @Override - public String encodeUrl(String url) { - return response.encodeUrl(url); - } - - @Override - public String encodeRedirectUrl(String url) { - return response.encodeRedirectUrl(url); - } - - @Override - public void sendError(int sc, String msg) throws IOException { - // TODO: Decoration anpassen/anstoßen?!? - response.sendError(sc,msg); - } - - @Override - public void sendError(int sc) throws IOException { - // TODO: Decoration anpassen/anstoßen?!? - response.sendError(sc); - } - - @Override - public void sendRedirect(String location) throws IOException { - // TODO: Decoration anpassen/anstoßen?!? - response.sendRedirect(location); - } - - @Override - public String getCharacterEncoding() { - return response.getCharacterEncoding(); - } - - @Override - public String getContentType() { - return response.getContentType(); - } - - @Override - public void setCharacterEncoding(String charset) { - // TODO: Je nach Vary-Einstellung ETag anpassen? - response.setCharacterEncoding(charset); - } - - @Override - public void setContentType(String type) { - // TODO: Je nach Vary-Einstellung ETag anpassen? - response.setContentType(type); - } - - @Override - public int getBufferSize() { - return response.getBufferSize(); - } - - @Override - public boolean isCommitted() { - // TODO: Eigene commit-Kontrolle wegen Dekorations-Einstiegspunkt?!? - return response.isCommitted(); - } - - @Override - public void setLocale(Locale loc) { - // TODO: Je nach Vary-Einstellung ETag anpassen? - response.setLocale(loc); - } - - @Override - public Locale getLocale() { - return getLocale(); - } - - - class CountingServletOutputStream extends ServletOutputStream { - - private OutputStream out; - int left; - boolean empty; - - - CountingServletOutputStream() throws IOException { - out = response.getOutputStream(); - left = buffer; - empty = true; - } + private class AcceleratorServletOutputStream extends ServletOutputStream { + private final ServletOutputStream sos; - void setBuffer(int size) throws IllegalStateException {} - boolean isZipped() { - return false; + private AcceleratorServletOutputStream() throws IOException { + bufferSize = defaultBufferSize; + buffer = new byte[bufferSize]; + sos = AccelerationWrapper.this.response.getOutputStream(); } - void finish() throws IOException { - decorate(); - } - - void decorate() throws IOException { - try { - AcceleratorFilter.this.cacheControl.decorate(AccelerationWrapper.this.request, response, AccelerationWrapper.this); - } - catch (Exception e) { - log.error("Error while guessing Cache-Header's", e); - response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - } - } - - - @Override - public void close() throws IOException { - decorate(); - out.close(); - } - @Override - public void flush() throws IOException { - decorate(); - out.flush(); + private OutputStream out() throws IOException { + if (os == null) + os = zipped ? new GZIPOutputStream(sos) : sos; + return os; } @Override public void write(int i) throws IOException { - empty = false; - if (left == 0) - decorate(); - left--; - out.write(i); - } - } - - - final class GZIPServletOutputStream extends CountingServletOutputStream { - - final static int MINIMAL_BUFFER_SIZE = 128; - - private ByteArrayOutputStream buffer; - private OutputStream out; - private GZIPOutputStream zout; - private int bufferSize; - private boolean decorated = false; - - - public GZIPServletOutputStream() throws IOException { - setBuffer(AcceleratorFilter.this.buffer); - } - - - @Override - void finish() throws IOException { - decorate(); - zout.finish(); - super.out.write(buffer.toByteArray()); - } - - @Override - boolean isZipped() { - if (decorated) - return true; - return !empty; - } - - @Override - void setBuffer(int size) throws IllegalStateException { - if (!empty) - throw new IllegalStateException("attemp to change buffer size after writing data to response!"); - - if (size > MINIMAL_BUFFER_SIZE) { - left = size; - try { - bufferSize = size; - out = response.getOutputStream(); - buffer = new ByteArrayOutputStream(size); - zout = new GZIPOutputStream(buffer, size); - super.out = zout; - } - catch (IOException e) { - throw new IllegalStateException(e); - } + if (pos == bufferSize) { + out().write(buffer); + pos = 0; + committed = true; + cacheControl.decorate(request, response, this); } - } - - - - @Override - public void close() throws IOException { - decorate(); - zout.close(); - out.write(buffer.toByteArray()); - out.close(); + buffer[pos++] = (byte) i; + size++; } @Override public void flush() throws IOException { - decorate(); - out.write(buffer.toByteArray()); - out.flush(); + if (pos == 0) + return; + + committed = true; + cacheControl.decorate(request, response, this); + out().write(buffer, 0, pos); + out().flush(); + pos = 0; } @Override - public void write(int i) throws IOException { - empty = false; - if (left == 0) { - if (!decorated) { - decorate(); - decorated = true; - } - out.write(buffer.toByteArray()); - buffer.reset(); - left = bufferSize; + public void close() throws IOException { + if (size == 0) { + committed = true; + zipped = false; + cacheControl.decorate(request, response, this); + sos.close(); + } + else { + flush(); + out().close(); } - left--; - zout.write(i); } } } diff --git a/cachecontrol/src/test/resources/config.xml b/cachecontrol/src/test/resources/config.xml index 33382bd8..f9e3e0fc 100644 --- a/cachecontrol/src/test/resources/config.xml +++ b/cachecontrol/src/test/resources/config.xml @@ -11,10 +11,6 @@ - - - - -- 2.20.1