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

Parse body of PATCH requests #426

Merged
merged 2 commits into from Aug 16, 2018

Conversation

Projects
None yet
3 participants
@calve
Contributor

calve commented Jun 14, 2018

Title say it all; allow to parse body of PATCH requests

should fix #358

@buixor

This comment has been minimized.

Show comment
Hide comment
@buixor

buixor Jun 19, 2018

Member

Thanks for the MR, giving it a look :)

Member

buixor commented Jun 19, 2018

Thanks for the MR, giving it a look :)

@buixor

This comment has been minimized.

Show comment
Hide comment
@buixor

buixor Jun 19, 2018

Member

@calve would you mind adding some unit tests ? In the meanwhile I'm going to check if there are some pitfalls we might take care of when dealing with PATCH method.

Member

buixor commented Jun 19, 2018

@calve would you mind adding some unit tests ? In the meanwhile I'm going to check if there are some pitfalls we might take care of when dealing with PATCH method.

@jvoisin jvoisin added this to the 0.56 milestone Jun 19, 2018

@@ -37,9 +37,8 @@ is to add required ACCEPT rules for the target website to work properly.
Contrary to most Web Application Firewalls, Naxsi doesn't rely on a
signature base like an antivirus, and thus cannot be circumvented by an
"unknown" attack pattern. Another main difference between Naxsi and other
WAFs, Naxsi filters only GET and POST requests,

This comment has been minimized.

@jvoisin

jvoisin Jun 19, 2018

Collaborator

Only GET, POST and PATCH. We're not handling HEAD, DELETE, …
Shouldn't we keep this part?

@jvoisin

jvoisin Jun 19, 2018

Collaborator

Only GET, POST and PATCH. We're not handling HEAD, DELETE, …
Shouldn't we keep this part?

@buixor buixor merged commit 6db970b into nbs-system:master Aug 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@calve

This comment has been minimized.

Show comment
Hide comment
@calve

calve Aug 16, 2018

Contributor

Thanks for the merge. Sorry I did not took time to add proper tests. On the other hand, there is no test for PUT requests aswell. Shall we copy/paste the tests for POST or factorise them ?

Contributor

calve commented Aug 16, 2018

Thanks for the merge. Sorry I did not took time to add proper tests. On the other hand, there is no test for PUT requests aswell. Shall we copy/paste the tests for POST or factorise them ?

@jvoisin

This comment has been minimized.

Show comment
Hide comment
@jvoisin

jvoisin Aug 19, 2018

Collaborator

Yup, this is the way to go :)

Collaborator

jvoisin commented Aug 19, 2018

Yup, this is the way to go :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment