Skip to content

Added PATCH support #1108

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

Closed
wants to merge 1 commit into from
Closed

Conversation

culyerr
Copy link

@culyerr culyerr commented Mar 30, 2016

Adds PATCH support for IIS - relates to #1107

@csanders-git
Copy link

This is very interesting - for my own edification - where is the PATCH method used?

@culyerr
Copy link
Author

culyerr commented Mar 30, 2016

From RFC5789
"The existing HTTP PUT method only allows a complete replacement of a document.This proposal adds a new HTTP method, PATCH, to modify an existing HTTP resource."

@csanders-git
Copy link

This is interesting - i've seen the RFC - does anything use this? Just so we can test it :-P

@culyerr
Copy link
Author

culyerr commented Mar 30, 2016

We have tested this with part of our payments solution but the PATCH methods are not publicly accessible for 3rd party testing.

I guess you could test against any resource on an IIS server - without the fix if you have "detection mode = on" it will record "Guid_0.00" errors if it does not work.

@zimmerle zimmerle added this to the v2.9.2 milestone Apr 5, 2016
@zimmerle
Copy link
Contributor

zimmerle commented Apr 5, 2016

@Almamu
Copy link

Almamu commented Jun 7, 2016

@csanders-git The standard methods (GET, POST, PUT, PATCH, DELETE) are widely used in REST API interfaces, so having support to inspect those would be really helpful.

@csanders-git
Copy link

@Almamu - any idea of some examples that might be around?

@Almamu
Copy link

Almamu commented Jun 9, 2016

@csanders-git I don't have any publicly accesible example as I've always worked on REST API for private systems, but searching about API REST in google gives a handful of results of how it should be implemented and documentation that specifies how the standar HTTP verbs are used in it and what they mean, although there is no RFC for REST API (as long as I recall).

EDIT: NVM, the github API does use PATCH requests: https://developer.github.com/v3/#http-verbs

@victorhora victorhora self-assigned this Apr 13, 2017
@victorhora
Copy link
Contributor

So it looks like there's still not that many apps using PATCH these days. I could only find a few APIs to test (which were easy to deploy on Apache/Nginx, but a pain to get it working under IIS). Maybe someone could give us a hand in that direction... :)

That said, by doing some trickery with IIS WebDAV module I could run a few tests with CRS and some custom rules. As PATCH is not explicitly added to the main code base of ModSecurity looks like you can't use REQUEST_METHOD or REQUEST_LINE variables straightforward to match with PATCH requests. But It looks like you can still match stuff with ARGS, REQUEST_LINE or FULL_REQUEST variables.

On all my testes I couldn't spot any error related to "Guid_0.00" as mentioned but I'm really not familiar with this error message. It looks like that this issue could be related with PCRE limits parsing large bodies and / or XML but this is only a guess at this point.

@culyerr @Almamu maybe if you could share the rules you have loaded, some debug logs and ideally a HTTP PATCH request which triggers this issue would be great.

What I'm trying to understand is if this pull request is actually to fix a bug specific for ModSecurity on IIS or to add a functionality (PATCH verb support) =)

@zimmerle
Copy link
Contributor

Closing this pull request, I will merge it once I find myself the manners to test it.

@zimmerle zimmerle closed this May 16, 2017
@zimmerle zimmerle self-requested a review May 16, 2017 20:32
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.

5 participants