[
https://issues.apache.org/jira/browse/FELIX-3988?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13900150#comment-13900150
]
J.W. Janssen edited comment on FELIX-3988 at 2/13/14 9:18 AM:
--------------------------------------------------------------
In case of filters, 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?
was (Author: jajans):
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
(v6.1.5#6160)
