felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "J.W. Janssen (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FELIX-3988) FilterHandler does not handle return of context.handleSecurity() correctly
Date Thu, 13 Feb 2014 09:19:19 GMT

    [ https://issues.apache.org/jira/browse/FELIX-3988?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13900150#comment-13900150

J.W. Janssen commented on FELIX-3988:

In case of {{Filter}}s, always setting the status to {{SC_FORBIDDEN}} prior to calling {{HttpContext#handleSecurity}}
is also wrong in my opinion, as we're not sure that there is only *one* filter present in
the chain. It could be that another filter already set a "non default" status code which we
would overwrite in that case.

To send an HTML form to enter user credentials with a status code of {{SC_OK}}, I think it
is "defendable" to state that an {{HttpContext#handleSecurity}} should commit its own response.
Perhaps this could be addressed to the JavaDoc of {{HttpContext}} or in the HTTP service update?

> FilterHandler does not handle return of context.handleSecurity() correctly
> --------------------------------------------------------------------------
>                 Key: FELIX-3988
>                 URL: https://issues.apache.org/jira/browse/FELIX-3988
>             Project: Felix
>          Issue Type: Bug
>          Components: HTTP Service
>    Affects Versions: http-2.2.0
>            Reporter: Kaspar von Gunten
>            Assignee: J.W. Janssen
>         Attachments: FilterHandler.patch
> This is somewhat similar to FELIX-2768, but not resolved.
> The JavaDoc for HttpContext.handleSecurity states: 
> * When this method returns false, the Http Service will send the response back to the
client, thereby
> * completing the request. When this method returns true, the Http Service will proceed
with servicing the 
> * request.
> The current FilterHandler impl is as follows:
> if (!getContext().handleSecurity(req, res)) {
>   res.sendError(HttpServletResponse.SC_FORBIDDEN);
> } else {
>   this.filter.doFilter(req, res, chain);
> }
> 1) This does not comply to the above doc, because the context might have set a status/error
code and prepared everything correctly to be returned. Sending an error will overwrite the
prepared headers and status.
> 2) Some context implementations are a bit eager and already send the response back before
they return "false" from the above method. The current implementation will then lead to an
IllegalStateException, because the response has already been committed. In order to be more
robust, there should be a if(!res.isCommitted())-block around the handling of the "false"
return value.
> I attached a patch for the second aspect of the issue, but couldn't resolve the first
part, because - as it is at the moment - there seems to be no way to check if there is already
a status set on the resource (no getStatus() or isStatusSet() method).
> Ideally the fix should be something like:
> if (!res.isCommitted() && !res.isStatusSet()) {
>   res.sendError(SC_FORBIDDEN);
> }

This message was sent by Atlassian JIRA

View raw message