You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by si...@apache.org on 2013/03/12 10:05:06 UTC

svn commit: r1455456 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java

Author: simonetripodi
Date: Tue Mar 12 09:05:06 2013
New Revision: 1455456

URL: http://svn.apache.org/r1455456
Log:
trivial: extracted POST string constant

Modified:
    commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java

Modified: commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java
URL: http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java?rev=1455456&r1=1455455&r2=1455456&view=diff
==============================================================================
--- commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java (original)
+++ commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java Tue Mar 12 09:05:06 2013
@@ -45,6 +45,8 @@ import org.apache.commons.fileupload.Fil
  */
 public class ServletFileUpload extends FileUpload {
 
+    private static final String POST_METHOD = "post";
+
     // ---------------------------------------------------------- Class methods
 
     /**
@@ -58,7 +60,7 @@ public class ServletFileUpload extends F
      */
     public static final boolean isMultipartContent(
             HttpServletRequest request) {
-        if (!"post".equals(request.getMethod().toLowerCase())) {
+        if (!POST_METHOD.equals(request.getMethod().toLowerCase())) {
             return false;
         }
         String contentType = request.getContentType();



Re: svn commit: r1455456 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java

Posted by Simone Tripodi <si...@apache.org>.
>
> Indeed.
>
> <quote rfc="2616", section="19.3">
> Although this document specifies the requirements for the generation of
> HTTP/1.1 messages, not all applications will be correct in their
> implementation. We therefore recommend that operational applications be
> tolerant of deviations whenever those deviations can be interpreted
> unambiguously.
> </quote>
>
> Using equalsIgnoreCase() is the better solution in this case.

OK thanks, I am going to relax the strict check.
-Simo

http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/

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


Re: svn commit: r1455456 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java

Posted by Mark Thomas <ma...@apache.org>.
On 13/03/2013 09:12, Jörg Schaible wrote:
> Hi Simo,
> 
> Simone Tripodi wrote:
> 
>> Hi Felix!
>>
>>>
>>> See RFC-2616 5.1.1 Method.
>>>
>>
>> the paragraph is indeed clear about that:
>>
>>    The Method  token indicates the method to be performed on the
>>    resource identified by the Request-URI. The method is case-sensitive.
>>
>>        Method         = "OPTIONS"                ; Section 9.2
>>                       | "GET"                    ; Section 9.3
>>                       | "HEAD"                   ; Section 9.4
>>                       | "POST"                   ; Section 9.5
>>                       | "PUT"                    ; Section 9.6
>>                       | "DELETE"                 ; Section 9.7
>>                       | "TRACE"                  ; Section 9.8
>>                       | "CONNECT"                ; Section 9.9
>>                       | extension-method
>>        extension-method = token
>>
>>> So IMHO there is no need for equalsIgnoreCase or toLowerCase at all and
>>> "POST".equals(...) should just do it.
>>>
>>
>> I'm by your side, I switch my opinion to having just equals() here.
>>
>> What other people think?
> 
> Follow the RFC.

Indeed.

<quote rfc="2616", section="19.3">
Although this document specifies the requirements for the generation of
HTTP/1.1 messages, not all applications will be correct in their
implementation. We therefore recommend that operational applications be
tolerant of deviations whenever those deviations can be interpreted
unambiguously.
</quote>

Using equalsIgnoreCase() is the better solution in this case.

Mark


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


Re: svn commit: r1455456 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java

Posted by Simone Tripodi <si...@apache.org>.
Hallo Jörg,

> Follow the RFC.

will do, I am updating FILEUPLOAD-229 then will provide the fix.

Alles Gute!
-Simo

http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/


On Wed, Mar 13, 2013 at 10:12 AM, Jörg Schaible
<Jo...@scalaris.com> wrote:
> Hi Simo,
>
> Simone Tripodi wrote:
>
>> Hi Felix!
>>
>>>
>>> See RFC-2616 5.1.1 Method.
>>>
>>
>> the paragraph is indeed clear about that:
>>
>>    The Method  token indicates the method to be performed on the
>>    resource identified by the Request-URI. The method is case-sensitive.
>>
>>        Method         = "OPTIONS"                ; Section 9.2
>>                       | "GET"                    ; Section 9.3
>>                       | "HEAD"                   ; Section 9.4
>>                       | "POST"                   ; Section 9.5
>>                       | "PUT"                    ; Section 9.6
>>                       | "DELETE"                 ; Section 9.7
>>                       | "TRACE"                  ; Section 9.8
>>                       | "CONNECT"                ; Section 9.9
>>                       | extension-method
>>        extension-method = token
>>
>>> So IMHO there is no need for equalsIgnoreCase or toLowerCase at all and
>>> "POST".equals(...) should just do it.
>>>
>>
>> I'm by your side, I switch my opinion to having just equals() here.
>>
>> What other people think?
>
> Follow the RFC.
>
> - Jörg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

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


Re: svn commit: r1455456 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java

Posted by Jörg Schaible <Jo...@scalaris.com>.
Hi Simo,

Simone Tripodi wrote:

> Hi Felix!
> 
>>
>> See RFC-2616 5.1.1 Method.
>>
> 
> the paragraph is indeed clear about that:
> 
>    The Method  token indicates the method to be performed on the
>    resource identified by the Request-URI. The method is case-sensitive.
> 
>        Method         = "OPTIONS"                ; Section 9.2
>                       | "GET"                    ; Section 9.3
>                       | "HEAD"                   ; Section 9.4
>                       | "POST"                   ; Section 9.5
>                       | "PUT"                    ; Section 9.6
>                       | "DELETE"                 ; Section 9.7
>                       | "TRACE"                  ; Section 9.8
>                       | "CONNECT"                ; Section 9.9
>                       | extension-method
>        extension-method = token
> 
>> So IMHO there is no need for equalsIgnoreCase or toLowerCase at all and
>> "POST".equals(...) should just do it.
>>
> 
> I'm by your side, I switch my opinion to having just equals() here.
> 
> What other people think?

Follow the RFC.

- Jörg


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


Re: svn commit: r1455456 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java

Posted by Simone Tripodi <si...@apache.org>.
Hi Felix!

>
> See RFC-2616 5.1.1 Method.
>

the paragraph is indeed clear about that:

   The Method  token indicates the method to be performed on the
   resource identified by the Request-URI. The method is case-sensitive.

       Method         = "OPTIONS"                ; Section 9.2
                      | "GET"                    ; Section 9.3
                      | "HEAD"                   ; Section 9.4
                      | "POST"                   ; Section 9.5
                      | "PUT"                    ; Section 9.6
                      | "DELETE"                 ; Section 9.7
                      | "TRACE"                  ; Section 9.8
                      | "CONNECT"                ; Section 9.9
                      | extension-method
       extension-method = token

> So IMHO there is no need for equalsIgnoreCase or toLowerCase at all and "POST".equals(...) should just do it.
>

I'm by your side, I switch my opinion to having just equals() here.

What other people think?
TIA!
-Simo

http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/

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


Re: svn commit: r1455456 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java

Posted by Felix Meschberger <fm...@adobe.com>.
Actually: The method is in fact case-sensitive and must be POST in uppercase.

See RFC-2616 5.1.1 Method.

So IMHO there is no need for equalsIgnoreCase or toLowerCase at all and "POST".equals(...) should just do it.

Regards
Felix

Am 13.03.2013 um 08:51 schrieb Simone Tripodi:

>> It would be safer to use equalsIgnoreCase(), for two reasons:
>> - POST_METHOD case is then irrelevant (methods are usually shown as uppercase)
>> - toLowerCase() is Locale-dependent.
> 
> +1 and it would be even faster in therms of complexity - I already
> voted to the issue you filled, are you going to take care of it or I
> can pick it up and resolve?
> TIA!
> -Simo
> 
> http://people.apache.org/~simonetripodi/
> http://simonetripodi.livejournal.com/
> http://twitter.com/simonetripodi
> http://www.99soft.org/
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 


--
Felix Meschberger | Principal Scientist | Adobe








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


Re: svn commit: r1455456 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java

Posted by Simone Tripodi <si...@apache.org>.
> It would be safer to use equalsIgnoreCase(), for two reasons:
> - POST_METHOD case is then irrelevant (methods are usually shown as uppercase)
> - toLowerCase() is Locale-dependent.

+1 and it would be even faster in therms of complexity - I already
voted to the issue you filled, are you going to take care of it or I
can pick it up and resolve?
TIA!
-Simo

http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/

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


Re: svn commit: r1455456 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java

Posted by sebb <se...@gmail.com>.
On 12 March 2013 09:05,  <si...@apache.org> wrote:
> Author: simonetripodi
> Date: Tue Mar 12 09:05:06 2013
> New Revision: 1455456
>
> URL: http://svn.apache.org/r1455456
> Log:
> trivial: extracted POST string constant
>
> Modified:
>     commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java
>
> Modified: commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java
> URL: http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java?rev=1455456&r1=1455455&r2=1455456&view=diff
> ==============================================================================
> --- commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java (original)
> +++ commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/servlet/ServletFileUpload.java Tue Mar 12 09:05:06 2013
> @@ -45,6 +45,8 @@ import org.apache.commons.fileupload.Fil
>   */
>  public class ServletFileUpload extends FileUpload {
>
> +    private static final String POST_METHOD = "post";
> +
>      // ---------------------------------------------------------- Class methods
>
>      /**
> @@ -58,7 +60,7 @@ public class ServletFileUpload extends F
>       */
>      public static final boolean isMultipartContent(
>              HttpServletRequest request) {
> -        if (!"post".equals(request.getMethod().toLowerCase())) {
> +        if (!POST_METHOD.equals(request.getMethod().toLowerCase())) {

It would be safer to use equalsIgnoreCase(), for two reasons:
- POST_METHOD case is then irrelevant (methods are usually shown as uppercase)
- toLowerCase() is Locale-dependent.

[Note: this is not a criticism of the commit itself which does not
change the original behaviour]

>              return false;
>          }
>          String contentType = request.getContentType();
>
>

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