[jira] [Comment Edited] (SLING-2724) Error handling doesn't respect servlet spec

["Carsten Ziegeler (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13575266#comment-13575266 ] 

Carsten Ziegeler edited comment on SLING-2724 at 2/9/13 9:23 PM:
-----------------------------------------------------------------

Ok, I think what we could do is:
- the first response wrapper we create, returns a special print writer for getWriter()
- after the error handler is called from either DefaultErrorHandler or SlingServletResolver, the code calling the error handlers calls flushBuffer() to write bytes to the output stream, followed by close() on the writer
- the writer keeps track if it is closed and checks in every method the flag
- if the flag is set (by calling close() ) and someone tries to append content, an illegal state exception is thrown from within the writer
- such an exception would be caught by the sling main servlet and just logged, so it's not propagated to the servlet engine

This should solve the problem - as we first call flushBuffer() the response is flagged as committed, so calls to the response like setting a header etc. would throw an exception anyway.

WDYT?
                
      was (Author: cziegeler):
    Ok, I think what we could do is:
- the first response wrapper we create, returns a special print writer for getWriter()
- after the error handler is called from either DefaultErrorHandler or SlingServletResolver, these objects call flushBuffer() to write bytes to the output stream, followed by close() on the writer
- the writer keeps track if it is closed and checks in every method the flag
- if the flag is set (by calling close() ) and someone tries to append content, an illegal state exception is thrown from within the writer
- such an exception would be caught by the sling main servlet and just logged, so it's not propagated to the servlet engine

This should solve the problem - as we first call flushBuffer() the response is flagged as committed, so calls to the response like setting a header etc. would throw an exception anyway.

WDYT?
                  
> Error handling doesn't respect servlet spec
> -------------------------------------------
>
>                 Key: SLING-2724
>                 URL: https://issues.apache.org/jira/browse/SLING-2724
>             Project: Sling
>          Issue Type: Bug
>          Components: Servlets
>            Reporter: Antonio Sanso
>
> If there is some servlet that is sending a 500 error (response.sendError(500)) and output something after e.g. "All good" (maybe this is the wrong part...) and a 500.jsp exists, Sling will "shows" both:
> - the error page 500.jsp
>  - and the the extra output (e.g. All good) 
> This seems to violate the spec:
> "These methods [sendError, sendRedirect] will have the side effect of committing the response, if it has not already been committed, and terminating it. No further output to the client should be made by the servlet after these methods are called. If data is written to the response after these methods are called, the data is ignored."
> Integration test showing the issue in [0]
> Mailing list thread in [1]
> [0] https://svn.apache.org/repos/asf/sling/trunk/launchpad/integration-tests/src/main/java/org/apache/sling/launchpad/webapp/integrationtest/servlets/resolver/errorhandler/ErrorHandlingTest.java
> [1] sling.markmail.org/thread/k2t7p5mhi4hgelwb

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira