fix: show 405 error if request is GET and queries are not allowed

[alexei]

Description

When allow_queries_via_get is False, GET requests are processed despite the fact that queries are disallowed. Currently the response is "400: No GraphQL query found in the request" which is raised at a deeper level. I believe such requests should be rejected immediately as they needlessly consume resources.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Documentation

Issues Fixed or Closed by This PR

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Summary by Sourcery

Fix the handling of GET requests by returning a 404 error when queries are disallowed, instead of processing them and returning a 400 error at a deeper level.

Bug Fixes:

• Return a 404 error for GET requests when queries are not allowed, preventing unnecessary resource consumption.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request implements a fix to show a 404 error if a GET request is received when queries are not allowed. The changes are made in both the asynchronous and synchronous base view classes.

File-Level Changes

Change	Details	Files
Add a check for GET requests when queries are not allowed	Introduce a condition to check if the request method is GET and queries via GET are not allowed Raise a 404 HTTPException if the condition is met	strawberry/http/async_base_view.py strawberry/http/sync_base_view.py

Sequence Diagram

sequenceDiagram
    participant Client
    participant BaseView
    participant RequestAdapter

    Client->>BaseView: Send request
    BaseView->>RequestAdapter: Get request method
    alt Request method is GET and queries via GET not allowed
        BaseView-->>Client: Return 404 Not Found
    else Request is valid
        BaseView->>BaseView: Continue processing
    end

Loading

---

Tips

• Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
• Continue your discussion with Sourcery by replying directly to review comments.
• You can change your review settings at any time by accessing your dashboard:
  • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
  • Change the review language;
• You can always contact us if you have any questions or feedback.

[sourcery-ai]

Hey @alexei - I've reviewed your changes - here's some feedback:

Overall Comments:

• Consider adding unit tests to cover the new condition for both GET and non-GET requests, with queries allowed and disallowed.
• The error message 'Not Found' might be too generic. Consider using a more specific message like 'GET queries not allowed' to help developers understand why their request was rejected.

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

[botberry]

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

---

This release fixes an issue where a GET request is processed despite it being disallowed.

Here's the tweet text:

🆕 Release (next) is out! Thanks to Alexandru Mărășteanu for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.75%. Comparing base (18f0f5d) to head (c1ac8c9).
Report is 5 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #3646       +/-   ##
===========================================
+ Coverage   72.55%   96.75%   +24.20%     
===========================================
  Files         518      522        +4     
  Lines       32647    33831     +1184     
  Branches     3772     5637     +1865     
===========================================
+ Hits        23687    32734     +9047     
+ Misses       8532      865     -7667     
+ Partials      428      232      -196     

[codspeed-hq]

CodSpeed Performance Report

Merging #3646 will not alter performance

_{Comparing alexei:fix-skip_parsing_get (c1ac8c9) with main (8e92e2b)}

Summary

✅ 15 untouched benchmarks

[erikwrede]

Let's be more specific here

Suggested change

	This release fixes an issue where a GET request is processed despite it being disallowed.
	This release changes the default behavior on queries via GET requests to a 405 error. To enable queries via get, please use `self.allow_queries_via_get`.

[alexei]

Hey @erikwrede I appreciate your reviewing this. I agree the message can be improved, but let's be clear about the change. Queries via GET are still allowed by default just like before - I did not touch that, though I believe the default should be more conservative. I changed the behavior when it's turned off:

• before this change you get a GraphQL query not found error;
• after this change you get a HTTP method not allowed error.

[bellini666]

@alexei so probably we can go with an explanation like that, saying that before, even though queries via GET are allowed by default, when disallowing them you would only get a GraphQL query not found error, but now you should get a 405 error instead.