[jira] [Updated] (SLING-8469) Sling forward clears response headers added by sling authentication and sling filters

["Andrew Khoury (JIRA)" <ji...@apache.org>]

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

Andrew Khoury updated SLING-8469:
---------------------------------
    Description: 
In the Sling Engine the RequestDispatcher.forward method, it calls response.reset() \[1] which clears all headers, but in Apache Tomcat it calls response.resetBuffer() \[2] which preserves the response headers.  I suspect that the tomcat behavior response.resetBuffer() is the correct behavior.

The problem with this behavior comes into play when there is a REQUEST scope javax.servlet.Filter or an AuthenticationHandler that adds response headers (such as Set-Cookie).  Those headers aren't preserved when the response is forwarded.

\[1] https://github.com/apache/sling-org-apache-sling-engine/blob/org.apache.sling.engine-2.6.18/src/main/java/org/apache/sling/engine/impl/request/SlingRequestDispatcher.java#L133

\[2] https://github.com/apache/tomcat/blob/9.0.20/java/org/apache/catalina/core/ApplicationDispatcher.java#L326
Clears buffer w/ out clearing all headers:
https://tomcat.apache.org/tomcat-5.5-doc/servletapi/javax/servlet/ServletResponse.html#resetBuffer()

The Java Servlet specification is unclear on this as there is no mention of the response headers:
https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf
{quote}
9.4 The Forward Method
The forward method of the RequestDispatcher interface may be called by the
calling servlet only when no output has been committed to the client. If output data
exists in the response buffer that has not been committed, the content must be
cleared before the target servlet’s service method is called. If the response has been
committed, an IllegalStateException must be thrown.
The path elements of the request object exposed to the target servlet must reflect the
path used to obtain the RequestDispatcher.
The only exception to this is if the RequestDispatcher was obtained via the
getNamedDispatcher method. In this case, the path elements of the request object
must reflect those of the original request.
Before the forward method of the RequestDispatcher interface returns without
exception, the response content must be sent and committed, and closed by the
servlet container, unless the request was put into the asynchronous mode. If an error
occurs in the target of the RequestDispatcher.forward() the exception may be
propagated back through all the calling filters and servlets and eventually back to
the container
{quote}

  was:
In the Sling Engine the RequestDispatcher.forward method, it calls response.reset() \[1] which clears all headers, but in Apache Tomcat it calls response.resetBuffer() \[2] which preserves the response headers.  I suspect that the tomcat behavior response.resetBuffer() is the correct behavior.

[1] https://github.com/apache/sling-org-apache-sling-engine/blob/org.apache.sling.engine-2.6.18/src/main/java/org/apache/sling/engine/impl/request/SlingRequestDispatcher.java#L133

[2] https://github.com/apache/tomcat/blob/9.0.20/java/org/apache/catalina/core/ApplicationDispatcher.java#L326
Clears buffer w/ out clearing all headers:
https://tomcat.apache.org/tomcat-5.5-doc/servletapi/javax/servlet/ServletResponse.html#resetBuffer()

Servlet specification is unclear on this as there is no mention of the response headers:
https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf
{quote}
9.4 The Forward Method
The forward method of the RequestDispatcher interface may be called by the
calling servlet only when no output has been committed to the client. If output data
exists in the response buffer that has not been committed, the content must be
cleared before the target servlet’s service method is called. If the response has been
committed, an IllegalStateException must be thrown.
The path elements of the request object exposed to the target servlet must reflect the
path used to obtain the RequestDispatcher.
The only exception to this is if the RequestDispatcher was obtained via the
getNamedDispatcher method. In this case, the path elements of the request object
must reflect those of the original request.
Before the forward method of the RequestDispatcher interface returns without
exception, the response content must be sent and committed, and closed by the
servlet container, unless the request was put into the asynchronous mode. If an error
occurs in the target of the RequestDispatcher.forward() the exception may be
propagated back through all the calling filters and servlets and eventually back to
the container
{quote}


> Sling forward clears response headers added by sling authentication and sling filters
> -------------------------------------------------------------------------------------
>
>                 Key: SLING-8469
>                 URL: https://issues.apache.org/jira/browse/SLING-8469
>             Project: Sling
>          Issue Type: Bug
>          Components: Engine
>    Affects Versions: Engine 2.6.18
>            Reporter: Andrew Khoury
>            Priority: Major
>
> In the Sling Engine the RequestDispatcher.forward method, it calls response.reset() \[1] which clears all headers, but in Apache Tomcat it calls response.resetBuffer() \[2] which preserves the response headers.  I suspect that the tomcat behavior response.resetBuffer() is the correct behavior.
> The problem with this behavior comes into play when there is a REQUEST scope javax.servlet.Filter or an AuthenticationHandler that adds response headers (such as Set-Cookie).  Those headers aren't preserved when the response is forwarded.
> \[1] https://github.com/apache/sling-org-apache-sling-engine/blob/org.apache.sling.engine-2.6.18/src/main/java/org/apache/sling/engine/impl/request/SlingRequestDispatcher.java#L133
> \[2] https://github.com/apache/tomcat/blob/9.0.20/java/org/apache/catalina/core/ApplicationDispatcher.java#L326
> Clears buffer w/ out clearing all headers:
> https://tomcat.apache.org/tomcat-5.5-doc/servletapi/javax/servlet/ServletResponse.html#resetBuffer()
> The Java Servlet specification is unclear on this as there is no mention of the response headers:
> https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf
> {quote}
> 9.4 The Forward Method
> The forward method of the RequestDispatcher interface may be called by the
> calling servlet only when no output has been committed to the client. If output data
> exists in the response buffer that has not been committed, the content must be
> cleared before the target servlet’s service method is called. If the response has been
> committed, an IllegalStateException must be thrown.
> The path elements of the request object exposed to the target servlet must reflect the
> path used to obtain the RequestDispatcher.
> The only exception to this is if the RequestDispatcher was obtained via the
> getNamedDispatcher method. In this case, the path elements of the request object
> must reflect those of the original request.
> Before the forward method of the RequestDispatcher interface returns without
> exception, the response content must be sent and committed, and closed by the
> servlet container, unless the request was put into the asynchronous mode. If an error
> occurs in the target of the RequestDispatcher.forward() the exception may be
> propagated back through all the calling filters and servlets and eventually back to
> the container
> {quote}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)