vacuum icon indicating copy to clipboard operation
vacuum copied to clipboard

Enhancement ; Rules for detecting non recommended HTTP DELETE + Payload

Open LasneF opened this issue 1 year ago • 10 comments

implement a warning rules when a DELETE has a requestBody as even it it can be supported is not a recommended pattern

could be set to INFO or Warning

LasneF avatar Nov 15 '24 17:11 LasneF

I have written a custom plugin rule that checks that delete operations return 204 and contain no response body. I can contribute it as a PR on Monday

lobocv avatar Nov 16 '24 15:11 lobocv

bump @lobocv

daveshanley avatar Nov 21 '24 15:11 daveshanley

@lobocv as the rules will looks similar solution than for DELETE

can you also push a rules for GET + payload as warning

that would be great

btw here is a conversation for the rules https://stackoverflow.com/questions/978061/http-get-with-request-body

LasneF avatar Nov 21 '24 16:11 LasneF

Thanks for the bump! Sorry, this fell off my radar. I will try and get a PR up today or tomorrow. I have a few other things on my plate right now I need to prepare for. I can definitely do that too @LasneF!

lobocv avatar Nov 21 '24 16:11 lobocv

Forgive me, It turns out that I did not write a custom Go plugin for this. I was able to do it in the DSL with two rules. There doesn't seem to be any examples of loading a function from the DSL in functions.go. If I need to write these as Go functions, it's going to take me a bit more time.

Here are the rules:

  delete-returns-http-204:
    id: delete-returns-http-204
    description: DELETE methods should return HTTP 204 and not HTTP 200
    type: style
    severity: error
    given: "$.paths[*].delete.responses"
    then:
      - field: '200'
        function: undefined
      - field: '204'
        function: defined

  http-204-has-no-content:
    id: http-204-has-no-content
    description: HTTP 204 (No Content) responses should not define content
    type: style
    severity: error
    given: "$.paths[*].*.responses['204']"
    then:
      - field: 'content'
        function: undefined

lobocv avatar Nov 21 '24 17:11 lobocv

🤔 interesting , by no content i was not looking for HTTP response code that is here subject to debate toward simplicity and 'preference'

i was more thinking about the presence of the requestBody

LasneF avatar Nov 22 '24 08:11 LasneF

Ah I see. I misunderstood. Yes the HTTP DELETE and GET operations should not have a request payload. I was thinking of response payload. You can do this with the DSL easily:


  http-delete-no-request-body:
    id: http-delete-no-request-body
    description: HTTP DELETE operations should not accept a request body
    type: style
    severity: error
    given: "$.paths[*].delete"
    then:
      - field: 'requestBody'
        function: undefined

  http-get-no-request-body:
    id: http-delete-no-request-body
    description: HTTP GET operations should not accept a request body
    type: style
    severity: error
    given: "$.paths[*].get"
    then:
      - field: 'requestBody'
        function: undefined

lobocv avatar Nov 25 '24 15:11 lobocv

@daveshanley this might be interesting to have it "by default" at part of core vacuum ; at least as a warning , if not as an error

LasneF avatar Nov 25 '24 16:11 LasneF

I am just finishing up a PR to add these functions. It was pretty easy. I'll post it soon. It handles get and delete methods

lobocv avatar Nov 25 '24 16:11 lobocv

https://github.com/daveshanley/vacuum/pull/588

lobocv avatar Nov 25 '24 17:11 lobocv

QA tested 👍 ,

@daveshanley closing the issue, as the PR from @lobocv is merged

LasneF avatar Jul 07 '25 13:07 LasneF