Return-Path: X-Original-To: apmail-sling-commits-archive@www.apache.org Delivered-To: apmail-sling-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id A96B010B4F for ; Wed, 5 Jun 2013 16:45:55 +0000 (UTC) Received: (qmail 51778 invoked by uid 500); 5 Jun 2013 16:45:55 -0000 Delivered-To: apmail-sling-commits-archive@sling.apache.org Received: (qmail 51675 invoked by uid 500); 5 Jun 2013 16:45:54 -0000 Mailing-List: contact commits-help@sling.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@sling.apache.org Delivered-To: mailing list commits@sling.apache.org Received: (qmail 51637 invoked by uid 99); 5 Jun 2013 16:45:53 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 05 Jun 2013 16:45:53 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 05 Jun 2013 16:45:49 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id ABF8A2388A91; Wed, 5 Jun 2013 16:45:28 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1489950 - in /sling/trunk: bundles/engine/src/main/java/org/apache/sling/engine/impl/ bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/ launchpad/builder/src/main/bundles/ Date: Wed, 05 Jun 2013 16:45:28 -0000 To: commits@sling.apache.org From: cziegeler@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20130605164528.ABF8A2388A91@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: cziegeler Date: Wed Jun 5 16:45:28 2013 New Revision: 1489950 URL: http://svn.apache.org/r1489950 Log: SLING-2724 : Error handling doesn't respect servlet spec Modified: sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingHttpServletResponseImpl.java sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java sling/trunk/launchpad/builder/src/main/bundles/list.xml Modified: sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java?rev=1489950&r1=1489949&r2=1489950&view=diff ============================================================================== --- sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java (original) +++ sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java Wed Jun 5 16:45:28 2013 @@ -172,6 +172,8 @@ public class DefaultErrorHandler impleme // commit the response response.flushBuffer(); + // close the response (SLING-2724) + pw.close(); } } } Modified: sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingHttpServletResponseImpl.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingHttpServletResponseImpl.java?rev=1489950&r1=1489949&r2=1489950&view=diff ============================================================================== --- sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingHttpServletResponseImpl.java (original) +++ sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingHttpServletResponseImpl.java Wed Jun 5 16:45:28 2013 @@ -19,6 +19,9 @@ package org.apache.sling.engine.impl; import java.io.IOException; +import java.io.PrintWriter; +import java.util.Locale; + import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; @@ -27,12 +30,19 @@ import org.apache.sling.engine.impl.requ public class SlingHttpServletResponseImpl extends HttpServletResponseWrapper implements SlingHttpServletResponse { + public static class WriterAlreadyClosedException extends IllegalStateException { + // just a marker class. + } + private final RequestData requestData; + private final boolean firstSlingResponse; + public SlingHttpServletResponseImpl(RequestData requestData, HttpServletResponse response) { super(response); this.requestData = requestData; + this.firstSlingResponse = !(response instanceof SlingHttpServletResponse); } protected final RequestData getRequestData() { @@ -98,8 +108,236 @@ public class SlingHttpServletResponseImp eh.handleError(status, message, requestData.getSlingRequest(), this); } + // ---------- Internal helper --------------------------------------------- + @Override + public PrintWriter getWriter() throws IOException { + PrintWriter result = super.getWriter(); + if ( firstSlingResponse ) { + final PrintWriter delegatee = result; + result = new PrintWriter(result) { + + private boolean isClosed = false; + + private void checkClosed() { + if ( this.isClosed ) { + throw new WriterAlreadyClosedException(); + } + } + + @Override + public PrintWriter append(final char arg0) { + this.checkClosed(); + return delegatee.append(arg0); + } + + @Override + public PrintWriter append(final CharSequence arg0, final int arg1, final int arg2) { + this.checkClosed(); + return delegatee.append(arg0, arg1, arg2); + } + + @Override + public PrintWriter append(final CharSequence arg0) { + this.checkClosed(); + return delegatee.append(arg0); + } + + @Override + public boolean checkError() { + this.checkClosed(); + return delegatee.checkError(); + } + + @Override + public void close() { + this.checkClosed(); + this.isClosed = true; + delegatee.close(); + } + + @Override + public void flush() { + this.checkClosed(); + delegatee.flush(); + } + + @Override + public PrintWriter format(final Locale arg0, final String arg1, + final Object... arg2) { + this.checkClosed(); + return delegatee.format(arg0, arg1, arg2); + } + + @Override + public PrintWriter format(final String arg0, final Object... arg1) { + this.checkClosed(); + return delegatee.format(arg0, arg1); + } + + @Override + public void print(final boolean arg0) { + this.checkClosed(); + delegatee.print(arg0); + } + + @Override + public void print(final char arg0) { + this.checkClosed(); + delegatee.print(arg0); + } + + @Override + public void print(final char[] arg0) { + this.checkClosed(); + delegatee.print(arg0); + } + + @Override + public void print(final double arg0) { + this.checkClosed(); + delegatee.print(arg0); + } + + @Override + public void print(final float arg0) { + this.checkClosed(); + delegatee.print(arg0); + } + + @Override + public void print(final int arg0) { + this.checkClosed(); + delegatee.print(arg0); + } + + @Override + public void print(final long arg0) { + this.checkClosed(); + delegatee.print(arg0); + } + + @Override + public void print(final Object arg0) { + this.checkClosed(); + delegatee.print(arg0); + } + + @Override + public void print(final String arg0) { + this.checkClosed(); + delegatee.print(arg0); + } + + @Override + public PrintWriter printf(final Locale arg0, final String arg1, + final Object... arg2) { + this.checkClosed(); + return delegatee.printf(arg0, arg1, arg2); + } + + @Override + public PrintWriter printf(final String arg0, final Object... arg1) { + this.checkClosed(); + return delegatee.printf(arg0, arg1); + } + + @Override + public void println() { + this.checkClosed(); + delegatee.println(); + } + + @Override + public void println(final boolean arg0) { + this.checkClosed(); + delegatee.println(arg0); + } + + @Override + public void println(final char arg0) { + this.checkClosed(); + delegatee.println(arg0); + } + + @Override + public void println(final char[] arg0) { + this.checkClosed(); + delegatee.println(arg0); + } + + @Override + public void println(final double arg0) { + this.checkClosed(); + delegatee.println(arg0); + } + + @Override + public void println(final float arg0) { + this.checkClosed(); + delegatee.println(arg0); + } + + @Override + public void println(final int arg0) { + this.checkClosed(); + delegatee.println(arg0); + } + + @Override + public void println(final long arg0) { + this.checkClosed(); + delegatee.println(arg0); + } + + @Override + public void println(final Object arg0) { + this.checkClosed(); + delegatee.println(arg0); + } + + @Override + public void println(final String arg0) { + this.checkClosed(); + delegatee.println(arg0); + } + + @Override + public void write(final char[] arg0, final int arg1, final int arg2) { + this.checkClosed(); + delegatee.write(arg0, arg1, arg2); + } + + @Override + public void write(final char[] arg0) { + this.checkClosed(); + delegatee.write(arg0); + } + + @Override + public void write(final int arg0) { + this.checkClosed(); + delegatee.write(arg0); + } + + @Override + public void write(final String arg0, final int arg1, final int arg2) { + this.checkClosed(); + delegatee.write(arg0, arg1, arg2); + } + + @Override + public void write(final String arg0) { + this.checkClosed(); + delegatee.write(arg0); + } + + }; + } + return result; + } + private void checkCommitted() { if (isCommitted()) { throw new IllegalStateException( Modified: sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java?rev=1489950&r1=1489949&r2=1489950&view=diff ============================================================================== --- sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java (original) +++ sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java Wed Jun 5 16:45:28 2013 @@ -157,6 +157,8 @@ public class SlingRequestProcessorImpl i } + } catch ( final SlingHttpServletResponseImpl.WriterAlreadyClosedException wace ) { + log.error("Writer has already been closed.", wace); } catch (ResourceNotFoundException rnfe) { // send this exception as a 404 status @@ -371,6 +373,7 @@ public class SlingRequestProcessorImpl i super(wrappedResponse); } + @Override public PrintWriter getWriter() throws IOException { if (writer == null) { try { Modified: sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java?rev=1489950&r1=1489949&r2=1489950&view=diff ============================================================================== --- sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java (original) +++ sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java Wed Jun 5 16:45:28 2013 @@ -490,6 +490,9 @@ public class SlingServletResolver } } + /** + * @see org.apache.sling.engine.servlets.ErrorHandler#handleError(java.lang.Throwable, org.apache.sling.api.SlingHttpServletRequest, org.apache.sling.api.SlingHttpServletResponse) + */ public void handleError(Throwable throwable, SlingHttpServletRequest request, SlingHttpServletResponse response) throws IOException { // do not handle, if already handling .... @@ -761,7 +764,7 @@ public class SlingServletResolver return fallbackErrorServlet; } - private void handleError(Servlet errorHandler, HttpServletRequest request, HttpServletResponse response) + private void handleError(final Servlet errorHandler, final HttpServletRequest request, final HttpServletResponse response) throws IOException { request.setAttribute(SlingConstants.ERROR_REQUEST_URI, request.getRequestURI()); @@ -774,10 +777,14 @@ public class SlingServletResolver try { errorHandler.service(request, response); - } catch (IOException ioe) { - // forware the IOException + // commit the response + response.flushBuffer(); + // close the response (SLING-2724) + response.getWriter().close(); + } catch (final IOException ioe) { + // forward the IOException throw ioe; - } catch (Throwable t) { + } catch (final Throwable t) { LOGGER.error("Calling the error handler resulted in an error", t); LOGGER.error("Original error " + request.getAttribute(SlingConstants.ERROR_EXCEPTION_TYPE), (Throwable) request.getAttribute(SlingConstants.ERROR_EXCEPTION)); Modified: sling/trunk/launchpad/builder/src/main/bundles/list.xml URL: http://svn.apache.org/viewvc/sling/trunk/launchpad/builder/src/main/bundles/list.xml?rev=1489950&r1=1489949&r2=1489950&view=diff ============================================================================== --- sling/trunk/launchpad/builder/src/main/bundles/list.xml (original) +++ sling/trunk/launchpad/builder/src/main/bundles/list.xml Wed Jun 5 16:45:28 2013 @@ -106,7 +106,7 @@ org.apache.sling org.apache.sling.engine - 2.2.8 + 2.2.9-SNAPSHOT org.apache.sling @@ -131,7 +131,7 @@ org.apache.sling org.apache.sling.servlets.resolver - 2.2.4 + 2.2.5-SNAPSHOT org.apache.sling