Skip to content
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

Mounted App (on Starlette) Doesn't Check Path when Serving Requests #1072

Open
cancan101 opened this issue Apr 18, 2023 · 5 comments
Open

Mounted App (on Starlette) Doesn't Check Path when Serving Requests #1072

cancan101 opened this issue Apr 18, 2023 · 5 comments

Comments

@cancan101
Copy link
Contributor

Following the docs here https://ariadnegraphql.org/docs/starlette-integration#mounting-asgi-application:

app.mount("/graphql/", GraphQL(schema, debug=True))

Means the app will respond on <app>/graphql/ but also anything starting with that prefix, such as <app>/graphql/v2.

The is probably not desirable.

@cancan101 cancan101 changed the title Mounting of App (on Starlette) Doesn't Check Path Mounted of App (on Starlette) Doesn't Check Path when Serving Requests Apr 18, 2023
@cancan101 cancan101 changed the title Mounted of App (on Starlette) Doesn't Check Path when Serving Requests Mounted App (on Starlette) Doesn't Check Path when Serving Requests Apr 18, 2023
@rafalp
Copy link
Contributor

rafalp commented Apr 19, 2023

If it's not desirable, why it's not desirable? Because search engines will index GraphiQL under odd paths like /graphql/wp-admin.php?

@cancan101
Copy link
Contributor Author

Because search engines will index GraphiQL under odd paths like /graphql/wp-admin.php

That is one issue. Another being a consumer might accidentally code against the "wrong" URL not realizing that it wrong because it works. At some point in the future, if the URL is made stricter (for example a new server / proxy or even just moving to the Route approach in Starlette), all of a sudden what they thought was a working URL will cease to function.

@rafalp
Copy link
Contributor

rafalp commented Apr 20, 2023

Web crawlers can be told to bugger off via either robots.txt or disabling explorer view in Ariadne.

And how would you want to handle posts made for wrong paths? The fix would require having GraphQL URL specified on both the app you are mounting in and Ariadne's GraphQL APP, wouldn't?

@cancan101
Copy link
Contributor Author

If I understand this code in Starlette: https://github.com/encode/starlette/blob/01aa49a379520d3bbe6bc1aecc48a48169e6b004/starlette/routing.py#L386-L407, it bite off the portion of the path that it matches against. Given that, wouldn't Ariadne's GraphQL APP just have to match against /, ie the portion that remains. If that is the case then I don't think the GraphQL URL needs to be specified in both locations.

@rafalp
Copy link
Contributor

rafalp commented Apr 21, 2023

What about other ASGI applications? FastAPI? Starlite? Etc. Ect? I would suspect this is also the case for them, but would you be willing to verify this and open a PR with the change?

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

No branches or pull requests

2 participants