[GitHub] struts pull request #151: WW-4818 change default Multipart validation regex ...

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #151: WW-4818 change default Multipart validation regex ...

cnenning
GitHub user sdutry opened a pull request:

    https://github.com/apache/struts/pull/151

    WW-4818 change default Multipart validation regex to comply with RFC1341

   

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/sdutry/struts WW-4818

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/struts/pull/151.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #151
   
----
commit 68d52dbe42aebc8e24379ebfaf4f306dd261b91c
Author: Stefaan Dutry <[hidden email]>
Date:   2017-07-25T11:05:07Z

    WW-4818 change default Multipart validation regex to comply with RFC1341

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #151: WW-4818 change default Multipart validation regex ...

cnenning
Github user lukaszlenart commented on a diff in the pull request:

    https://github.com/apache/struts/pull/151#discussion_r129280692
 
    --- Diff: core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java ---
    @@ -88,7 +88,7 @@
          */
         public static final String REQUEST_POST_METHOD = "POST";
     
    -    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart\\/form-data(; boundary=[\\-a-zA-Z0-9]{1,70})?";
    +    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart/form-data(; boundary=[0-9a-zA-Z'()+_,\\-./:=?]{1,70})?";
    --- End diff --
   
    You have to escape `.` as it now means `Any single character`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #151: WW-4818 change default Multipart validation regex ...

cnenning
In reply to this post by cnenning
Github user lukaszlenart commented on a diff in the pull request:

    https://github.com/apache/struts/pull/151#discussion_r129281026
 
    --- Diff: core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java ---
    @@ -88,7 +88,7 @@
          */
         public static final String REQUEST_POST_METHOD = "POST";
     
    -    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart\\/form-data(; boundary=[\\-a-zA-Z0-9]{1,70})?";
    +    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart/form-data(; boundary=[0-9a-zA-Z'()+_,\\-./:=?]{1,70})?";
    --- End diff --
   
    Also `-` has to be escaped as it is treated as a group separator


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #151: WW-4818 change default Multipart validation regex ...

cnenning
In reply to this post by cnenning
Github user sdutry commented on a diff in the pull request:

    https://github.com/apache/struts/pull/151#discussion_r129281083
 
    --- Diff: core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java ---
    @@ -88,7 +88,7 @@
          */
         public static final String REQUEST_POST_METHOD = "POST";
     
    -    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart\\/form-data(; boundary=[\\-a-zA-Z0-9]{1,70})?";
    +    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart/form-data(; boundary=[0-9a-zA-Z'()+_,\\-./:=?]{1,70})?";
    --- End diff --
   
    not within the square brackets according to my knowledge


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #151: WW-4818 change default Multipart validation regex ...

cnenning
In reply to this post by cnenning
Github user lukaszlenart commented on a diff in the pull request:

    https://github.com/apache/struts/pull/151#discussion_r129281192
 
    --- Diff: core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java ---
    @@ -88,7 +88,7 @@
          */
         public static final String REQUEST_POST_METHOD = "POST";
     
    -    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart\\/form-data(; boundary=[\\-a-zA-Z0-9]{1,70})?";
    +    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart/form-data(; boundary=[0-9a-zA-Z'()+_,\\-./:=?]{1,70})?";
    --- End diff --
   
    o! didn't know that :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #151: WW-4818 change default Multipart validation regex ...

cnenning
In reply to this post by cnenning
Github user sdutry commented on a diff in the pull request:

    https://github.com/apache/struts/pull/151#discussion_r129282110
 
    --- Diff: core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java ---
    @@ -88,7 +88,7 @@
          */
         public static final String REQUEST_POST_METHOD = "POST";
     
    -    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart\\/form-data(; boundary=[\\-a-zA-Z0-9]{1,70})?";
    +    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart/form-data(; boundary=[0-9a-zA-Z'()+_,\\-./:=?]{1,70})?";
    --- End diff --
   
    you are correct about the `-` needing to be escaped , but that's the only one i escaped. (they're in the order they're mentioned in RFC1341 (so i moved the `-` backwards in the pattern)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #151: WW-4818 change default Multipart validation regex ...

cnenning
In reply to this post by cnenning
Github user lukaszlenart commented on a diff in the pull request:

    https://github.com/apache/struts/pull/151#discussion_r129282870
 
    --- Diff: core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java ---
    @@ -88,7 +88,7 @@
          */
         public static final String REQUEST_POST_METHOD = "POST";
     
    -    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart\\/form-data(; boundary=[\\-a-zA-Z0-9]{1,70})?";
    +    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart/form-data(; boundary=[0-9a-zA-Z'()+_,\\-./:=?]{1,70})?";
    --- End diff --
   
    There are two test cases that you can extend or another one - see `DispatcherTest`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #151: WW-4818 change default Multipart validation regex ...

cnenning
In reply to this post by cnenning
Github user sdutry commented on a diff in the pull request:

    https://github.com/apache/struts/pull/151#discussion_r129283402
 
    --- Diff: core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java ---
    @@ -88,7 +88,7 @@
          */
         public static final String REQUEST_POST_METHOD = "POST";
     
    -    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart\\/form-data(; boundary=[\\-a-zA-Z0-9]{1,70})?";
    +    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart/form-data(; boundary=[0-9a-zA-Z'()+_,\\-./:=?]{1,70})?";
    --- End diff --
   
    Ok, i'll add some tests to confirm the pattern is correct.
    (Or if needed to correct it)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #151: WW-4818 change default Multipart validation regex to comp...

cnenning
In reply to this post by cnenning
Github user sdutry commented on the issue:

    https://github.com/apache/struts/pull/151
 
    @lukaszlenart
    I added 2 simple tests.
    - one containing all the special allowed characters
    - another one containing a single not-allowed character
   
    Please feel free to tell me what other test-cases you want added.
    (for example, any specific characters you want tested?)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #151: WW-4818 change default Multipart validation regex to comp...

cnenning
In reply to this post by cnenning
Github user lukaszlenart commented on the issue:

    https://github.com/apache/struts/pull/151
 
    Looks good 👍  LGTM :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #151: WW-4818 change default Multipart validation regex to comp...

cnenning
In reply to this post by cnenning
Github user sdutry commented on the issue:

    https://github.com/apache/struts/pull/151
 
    @lukaszlenart
    Am i allowed to merge this or is there more work/checks that needs to happen first?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #151: WW-4818 change default Multipart validation regex to comp...

cnenning
In reply to this post by cnenning
Github user lukaszlenart commented on the issue:

    https://github.com/apache/struts/pull/151
 
    @sdutry yes, you are :)
    http://struts.apache.org/submitting-patches.html#how-to-merge-pull-requests


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #151: WW-4818 change default Multipart validation regex ...

cnenning
In reply to this post by cnenning
Github user asfgit closed the pull request at:

    https://github.com/apache/struts/pull/151


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Loading...