Skip to content
New issue

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

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

Already on GitHub? # to your account

Enhancement ; Rules for detecting non recommended HTTP DELETE + Payload #582

Open
LasneF opened this issue Nov 15, 2024 · 10 comments
Open

Comments

@LasneF
Copy link

LasneF commented Nov 15, 2024

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

@lobocv
Copy link

lobocv commented Nov 16, 2024

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

@daveshanley
Copy link
Owner

bump @lobocv

@LasneF
Copy link
Author

LasneF commented Nov 21, 2024

@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

@lobocv
Copy link

lobocv commented Nov 21, 2024

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
Copy link

lobocv commented Nov 21, 2024

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

@LasneF
Copy link
Author

LasneF commented Nov 22, 2024

🤔 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

@lobocv
Copy link

lobocv commented Nov 25, 2024

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

@LasneF
Copy link
Author

LasneF commented Nov 25, 2024

@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

@lobocv
Copy link

lobocv commented Nov 25, 2024

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
Copy link

lobocv commented Nov 25, 2024

#588

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants