[ 
https://issues.apache.org/jira/browse/FELIX-3988?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Kaspar von Gunten updated FELIX-3988:
-------------------------------------

    Description: 
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);
}




  was:
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 an 
if(!response.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 is no way to check if there 
is already a status set on the resource. I.e. something like:

if (!res.isCommitted() && !res.isStatusSet()) {
  res.sendError(SC_FORBIDDEN);
}

is not possible.




    
> 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
>
> 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 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

Reply via email to