You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2013/09/25 19:23:53 UTC

[Bug 55595] New: Request parameter parsing is not thread-safe

https://issues.apache.org/bugzilla/show_bug.cgi?id=55595

            Bug ID: 55595
           Summary: Request parameter parsing is not thread-safe
           Product: Tomcat 8
           Version: trunk
          Hardware: PC
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Catalina
          Assignee: dev@tomcat.apache.org
          Reporter: tajuddin@amazon.com

Created attachment 30882
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=30882&action=edit
Proposed patch to trunk

I have a use case for accessing the parameters of a request in multiple threads
in order to generate the response. In that situation, there is a race condition
in org.apache.catalina.connector.Request when attempting to access parameters. 

The first thread in will flag the parameters as parsed while successive threads
may retrieve partially-constructed parameters. I can retrieve the parameters in
the request thread before dispatching to the other threads, and this problem
does not occur. However, that imposes overhead on my environment where it may
not be required.

A little synchronization should handle this case without creating any overhead
for the standard single-threaded request handling use cases. I have attached a
patch with a proposed solution. Any feedback is welcome.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 55595] Request parameter parsing is not thread-safe

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55595

--- Comment #4 from Brian Tajuddin <ta...@amazon.com> ---
We are actually trying to avoid the simple user land solution. This would
prevent any processing that does not depend on the parameters from starting
before the parameters are parsed. Since parsing parameters can be a
heavy-weight operation, especially in the case of a chunked post, we would like
to allow some processing to start before parsing.

Wrapping the request seems reasonable on the surface, but it forces the
application to assume that the data structures underneath the parameters cannot
be changed by the container without going through one of the getParameter...()
methods. Is this an assumption that can be safely made?

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 55595] Request parameter parsing is not thread-safe

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55595

--- Comment #5 from Chuck Caldarale <ch...@unisys.com> ---
(In reply to Brian Tajuddin from comment #4)
> Wrapping the request seems reasonable on the surface, but it forces the
> application to assume that the data structures underneath the parameters
> cannot be changed by the container without going through one of the
> getParameter...() methods. Is this an assumption that can be safely made?

I think you're missing the point: _none_ of the APIs exposed by
HttpServletRequest are thread-safe, regardless of how innocuous they might
seem.  It's up to the application to insure that only one thread at a time
utilizes the HttpServletRequest object.  (The same applies to the response
object.)  Having a filter wrap the request and response with your own objects
that synchronize internally is the way to go.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 55595] Request parameter parsing is not thread-safe

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55595

--- Comment #2 from Christopher Schultz <ch...@christopherschultz.net> ---
A bit more commentary to avoid grumbling:

1. While uncontended locks are cheap, they still have a cost that nobody else
wants to pay.

2. If you need to access request parameters from multiple threads, you can do
so yourself without having to modify the container to do it for you. Two
examples I can think of are to wrap the request in an HttpServletRequestWrapper
that you write which includes synchronization for any of the getParameter...()
family of methods, or to use a similar but simpler interface (e.g. just not
HttpServletRequestWrapper).

In any case, you will need to take special care not to retain references to the
request or response in those other threads. Otherwise, you risk a security
vulnerability at worst and mass confusion at the least.

If you don't want to complicate your own servlet code with this kind of thing,
you could even write a Filter to apply a wrapper for those servlets for which
it is appropriate.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 55595] Request parameter parsing is not thread-safe

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55595

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |WONTFIX

--- Comment #1 from Mark Thomas <ma...@apache.org> ---
See section 2.3.3.4 of the Servlet 3.1 specification.

Adding synchronization would add unnecessary overhead for the significant
majority of users who do not need multi-threaded access to the parameters.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 55595] Request parameter parsing is not thread-safe

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55595

--- Comment #3 from Tim Funk <fu...@apache.org> ---
In user land - the simple solution is to perform a
request.getParameter("anything") before the multithreaded logic is executed.

This will trigger the parsing of the request parameters.

Then the multithreaded logic can run. (But it is still a use at your own risk
since HttpServletRequest was never meant to be thread safe)

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org