strawberry-graphql / strawberry

A GraphQL library for Python that leverages type annotations 🍓
https://strawberry.rocks
MIT License
4.02k stars 533 forks source link

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

Open alexei opened 1 month ago

alexei commented 1 month ago

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

Issues Fixed or Closed by This PR

*

Checklist

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:

sourcery-ai[bot] commented 1 month ago

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

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](https://app.sourcery.ai): - Enable or disable the Sourcery-generated pull request summary or reviewer's guide; - Change the review language; - You can always [contact us](mailto:support@sourcery.ai) if you have any questions or feedback.
botberry commented 1 month ago

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[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

Additional details and impacted files ```diff @@ 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[bot] commented 1 month ago

CodSpeed Performance Report

Merging #3646 will not alter performance

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

Summary

✅ 15 untouched benchmarks