Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jul 26, 2017
Merged

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

merged 2 commits into from
Jul 26, 2017

Conversation

sdutry
Copy link
Member

@sdutry sdutry commented Jul 25, 2017

No description provided.

@@ -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})?";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to escape . as it now means Any single character

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also - has to be escaped as it is treated as a group separator

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not within the square brackets according to my knowledge

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

o! didn't know that :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two test cases that you can extend or another one - see DispatcherTest

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, i'll add some tests to confirm the pattern is correct.
(Or if needed to correct it)

@sdutry
Copy link
Member Author

sdutry commented Jul 25, 2017

@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?)

@lukaszlenart
Copy link
Member

Looks good 👍 LGTM :)

@sdutry
Copy link
Member Author

sdutry commented Jul 25, 2017

@lukaszlenart
Am i allowed to merge this or is there more work/checks that needs to happen first?

@lukaszlenart
Copy link
Member

@asfgit asfgit merged commit bbbe2a8 into apache:master Jul 26, 2017
@sdutry sdutry deleted the WW-4818 branch July 26, 2017 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants