[
https://issues.apache.org/jira/browse/SLING-4143?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Felix Meschberger reopened SLING-4143:
--------------------------------------
Sorry to reopen.
I basically like this restructuring to the DefaultErrorHandler and the
forwarding. I have just concerns with:
* Is Java 6 really required for this change ? Else, lets not change something
that is not required to be changed.
* Special casing of Throwable: To my knowledge there is no Throwable which is
neither an Error nor an Exception. Hence not handling Throwable but Error and
Exception is pointless (in the SlingServletResolver)
* Likewise catchting Error and Exception but not Throwable is also pointless
because all of them are Throwable and there are not other Throwables (in the
DefaultErrorHandler)
* The biggest issue, though, I have is that if the actual error handler fails,
an error is sent about the failing error handler instead of the actual error
having happened (in the DefaultErrorHandler). This is really not very helpful:
If an error happens that error should be reported. And if the selected error
handler fails, the fallback reporting should take place. Of course, logging
failure of the selected error handler makes perfect sense.
> 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)
