[
https://issues.apache.org/jira/browse/SLING-4143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14208038#comment-14208038
]
Felix Meschberger commented on SLING-4143:
------------------------------------------
bq. Removing the Java 6 requirement shouldn't be hard, but do we really still
care about pre-Java 6?
Probably not, but why change something that is irrelevant to this issue and
actually does not cause any harm at all ?
bq. Thinking about it you're right that catching both Error and Exception does
not make much sense. I'm always wary of catching Throwable though what do you
suggest?
If you treat Errors at all and don't discriminate between Error and Exception,
catching Throwable is the best you can do. Of course it is discutable to catch
Error at all, since for example OutOfMemoryError is an Error as well and there
is potentially not very much that you can do after that ...
bq. can you clarify with an example?
Example:
* Client requests /a/b/cyz.html
* Resource does not exist, 404/NOT FOUND raised
* 404 error handler is selected but fails
* failure is caught and 500/INTERNAL SERVER ERROR returned
This is confusing to the client and exposes an implementation detail (an error
handler which fails). Rather a not-so-nice-but-correct 404/NOT FOUND default
response should be sent back.
> Exception or Error in error handler script/servlet should cause HTTP status
> 500
> -------------------------------------------------------------------------------
>
> Key: SLING-4143
> URL: https://issues.apache.org/jira/browse/SLING-4143
> Project: Sling
> Issue Type: Bug
> Components: Engine
> Affects Versions: Servlets Resolver 2.3.2, Engine 2.3.8
> Reporter: Bertrand Delacretaz
> Assignee: Bertrand Delacretaz
> Fix For: Servlets Resolver 2.3.4, Engine 2.3.10
>
>
> It looks like Sling does not set the response status if a custom error
> handler script is present which does not set the status itself. See examples
> below.
> I think Sling should detect this and set a sensible non-200 status in this
> case.
> Example 1: script doesn't set status _(update: after discussion on list we
> won't change this)_
> {code}
> $ cat > 404.jsp
> <html>Custom 404 page</html>
> $ curl -u admin:admin -T 404.jsp
> http://localhost:8888/apps/sling/servlet/errorhandler/404.jsp
> $ curl -D - http://localhost:8888/nonexisting
> HTTP/1.1 200 OK
> ...
> <html>Custom 404 page</html>
> {code}
> Example 2, scripts throws a RuntimeException - blank response with 200 status
> - this is wrong
> {code}
> $ cat > 404.jsp
> <% if(true) throw new RuntimeException("404 page failed"); %>
> $ curl -u admin:admin -T 404.jsp
> http://localhost:8888/apps/sling/servlet/errorhandler/404.jsp
> $ curl -D - http://localhost:8888/nonexisting
> HTTP/1.1 200 OK
> ...
> Content-Length: 0
> Server: Jetty(7.x.y-SNAPSHOT)
> {code}
> Example 3: scripts sets status - this one looks correct.
> {code}
> $ cat > 404.jsp
> <% response.setStatus(421); %>
> Custom 421 page
> $ curl -u admin:admin -T 404.jsp
> http://localhost:8888/apps/sling/servlet/errorhandler/404.jsp
> $ curl -D - http://localhost:8888/nonexisting
> HTTP/1.1 421 421
> ...
> Server: Jetty(7.x.y-SNAPSHOT)
> Custom 421 page
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
