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 2015/05/19 22:56:40 UTC

[Bug 57938] New: HttpServletRequest.getString causes NPE when allowCasualMultipartParsing set "true" and multipart field is empty

https://bz.apache.org/bugzilla/show_bug.cgi?id=57938

            Bug ID: 57938
           Summary: HttpServletRequest.getString causes NPE when
                    allowCasualMultipartParsing set "true" and multipart
                    field is empty
           Product: Tomcat 8
           Version: 8.0.22
          Hardware: PC
            Status: NEW
          Severity: major
          Priority: P2
         Component: Util
          Assignee: dev@tomcat.apache.org
          Reporter: onurshin@gmail.com

Created attachment 32745
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=32745&action=edit
Sample project to reproduce bug

When sending a HTML form as multipart/form-data, if allowCasualMultipartParsing
is set to True and any form field is empty, calling
HttpServletRequest.getParts() causes NullPointerException.

19-May-2015 23:30:59.912 SEVERE [http-nio-8512-exec-14]
org.apache.catalina.core.StandardWrapperValve.invoke Servlet.service() for
servlet [post] in context with path [/test] threw exception
java.lang.NullPointerException
    java.lang.String.<init>(String.java:479)
   
org.apache.tomcat.util.http.fileupload.disk.DiskFileItem.getString(DiskFileItem.java:321)
    org.apache.catalina.connector.Request.parseParts(Request.java:2758)
    org.apache.catalina.connector.Request.getParts(Request.java:2641)
   
org.apache.catalina.connector.RequestFacade.getParts(RequestFacade.java:1083)
    com.multipart.emptytest.post.doPost(post.java:25) (<---
HttpServletRequest.getParts() here)
    javax.servlet.http.HttpServlet.service(HttpServlet.java:648)
    javax.servlet.http.HttpServlet.service(HttpServlet.java:729)
    org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
    ...


To Reproduce Bug:

1) Set allowCasualMultipartParsing to "true" in context.xml,
2) Create a HTML form with enctype="multipart/form-data",
3) Submit the form it to a servlet that calls HttpServletRequest.getParts(),
4) If you leave any field empty in the form you will get a NPE.

Expected Results: 

Whether there is an empty field or not HttpServletRequest.getParts() should
complete without any exceptions. Empty fields should return empty Part objects.

Actual Results:

Got NPE.

Additional Information: 

Whether allowCasualMultipartParsing is true or false if the servlet is
annotated as @MultipartConfig, error disappears and
HttpServletRequest.getParts() behaves as expected. But since sole purpose of
allowCasualMultipartParsing is the opportunity of using getParts() in a
ServletFilter, this is not a solution.

A sample project to reproduce bug in attachment.

-- 
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 57938] HttpServletRequest.getParts causes NPE when allowCasualMultipartParsing set "true" and multipart field is empty

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

Onur <on...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|HttpServletRequest.getStrin |HttpServletRequest.getParts
                   |g causes NPE when           |causes NPE when
                   |allowCasualMultipartParsing |allowCasualMultipartParsing
                   |set "true" and multipart    |set "true" and multipart
                   |field is empty              |field is empty

-- 
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 57938] HttpServletRequest.getParts causes NPE when allowCasualMultipartParsing set "true" and multipart field is empty

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

--- Comment #4 from Mark Thomas <ma...@apache.org> ---
(In reply to Christopher Schultz from comment #3)
> (In reply to Mark Thomas from comment #2)
> > maxPostSize=0 was really a bug in how allowCasualMultipartParsing was
> > implemented but I opted to switch the meaning if zero to a limit of zero
> > since a) it is more intuitive and b) it aligns with maxSavePostSize.
> 
> I'm interested (since I wrote it), what was the "real" bug?

It didn't handle the 0 == no limit case

-- 
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 57938] HttpServletRequest.getParts causes NPE when allowCasualMultipartParsing set "true" and multipart field is empty

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

--- Comment #3 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Mark Thomas from comment #2)
> maxPostSize=0 was really a bug in how allowCasualMultipartParsing was
> implemented but I opted to switch the meaning if zero to a limit of zero
> since a) it is more intuitive and b) it aligns with maxSavePostSize.

I'm interested (since I wrote it), what was the "real" bug?

-- 
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 57938] HttpServletRequest.getParts causes NPE when allowCasualMultipartParsing set "true" and multipart field is empty

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

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

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

--- Comment #2 from Mark Thomas <ma...@apache.org> ---
Thanks for the report. These issues have been fixed in trunk (9.0.x), 8.0.x
(for 8.0.24 onwards) and 7.0.x (for 7.0.63 onwards).

I opted for a slightly different fix for the first issue. Negative values are
now converted to 0 in the MultipartConfigElement constructor.

maxPostSize=0 was really a bug in how allowCasualMultipartParsing was
implemented but I opted to switch the meaning if zero to a limit of zero since
a) it is more intuitive and b) it aligns with maxSavePostSize.

-- 
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 57938] HttpServletRequest.getString causes NPE when allowCasualMultipartParsing set "true" and multipart field is empty

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

Onur <on...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All
          Component|Util                        |Catalina

--- Comment #1 from Onur <on...@gmail.com> ---
Just figured out what the problem is. maxPostSize attribute of Connector in
server.xml was set to -1. When allowCasualMultipartParsing is true and if there
is no @MultpartConfig, a MultipartConfigElement is created in
org.apache.catalina.connector.Request.parseParts using this code:


MultipartConfigElement mce = getWrapper().getMultipartConfigElement();

if (mce == null) {
    if(context.getAllowCasualMultipartParsing()) {
        mce = new MultipartConfigElement(null,
                                         connector.getMaxPostSize(),
                                         connector.getMaxPostSize(),
                                         connector.getMaxPostSize());
    } else {
                ...

The last parameter of MultipartConfigElement above is fileSizeTreshold which is
(from javadoc) "the size threshold after which files will be written to disk". 

Although it says that "The maximum size in bytes of the POST which will be
handled by the container FORM URL parameter parsing. The limit can be disabled
by setting this attribute to a value less than or equal to 0. If not specified,
this attribute is set to 2097152 (2 megabytes)." for maxPostSize in
documents(https://tomcat.apache.org/tomcat-8.0-doc/config/http.html#Common_Attributes),
setting maxPostSize to 0 blocks all post requests for example:

HTTP Status 500 - java.lang.IllegalStateException:
org.apache.tomcat.util.http.fileupload.FileUploadBase$SizeLimitExceededException:
the request was rejected because its size (235) exceeds the configured maximum
(0)

So setting maxPostSize to 0 is not a solution for anyone who wants to allow
unlimited size file uploads.


Posible quick fix is:

MultipartConfigElement mce = getWrapper().getMultipartConfigElement();

if (mce == null) {
    if(context.getAllowCasualMultipartParsing()) {
        mce = new MultipartConfigElement(null,
                                         connector.getMaxPostSize(),
                                         connector.getMaxPostSize(),
                                        
Math.max(0,connector.getMaxPostSize())); //line 2671 in v8.0.22
    } else {
                ...



I set my maxPostSize to a reasonable value as a temporary fix. But this bug and
misleading documentation of maxPostSize should be fixed.

-- 
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 57938] HttpServletRequest.getString causes NPE when allowCasualMultipartParsing set "true" and multipart field is empty

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

Onur <on...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |onrshin@gmail.com

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