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

multipart_uploads_enabled not propagated in AsyncGraphQLView, causing file uploads to fail #3713

Open
BranDavidSebastian opened this issue Nov 29, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@BranDavidSebastian
Copy link

BranDavidSebastian commented Nov 29, 2024

Describe the Bug

In strawberry-graphql==0.253.0 and strawberry-graphql-django==0.50.0, setting multipart_uploads_enabled=True in the AsyncGraphQLView does not enable multipart uploads as expected. The self.multipart_uploads_enabled attribute remains False, causing file uploads via multipart/form-data to fail with a 400 Bad Request error.

To Reproduce

  1. Configure the GraphQL view in Django:
path(
    'graphql/',
    AsyncGraphQLView.as_view(
        schema=schema,
        graphiql=settings.DEBUG,
        multipart_uploads_enabled=True,
    ),
    name='graphql',
),
  1. Attempt to perform a file upload mutation from the client.

Example cURL Command:

curl -X POST -H "Content-Type: multipart/form-data" \
  -F 'operations={"query":"mutation createImages($data: [ImageInput!]!) { createImages(data: $data) { id imageWebUrl }}","variables":{"data":[{"image":null,"imageType":"PACK"}]}}' \
  -F 'map={"0":["variables.data.0.image"]}' \
  -F '0=@/path/to/logo.png' \
  http://localhost:8080/graphql
  1. Observe that the server responds with a 400 Bad Request error stating "Unsupported content type".
  2. Inspect the self.multipart_uploads_enabled attribute inside the AsyncGraphQLView and find that it is False.

Expected Behavior

Setting multipart_uploads_enabled=True should set self.multipart_uploads_enabled to True in the AsyncGraphQLView, enabling multipart uploads and allowing file uploads to work correctly.

Actual Behavior

Despite setting multipart_uploads_enabled=True, self.multipart_uploads_enabled remains False, causing the server to reject multipart/form-data requests.

Additional Context

  • This issue did not occur in previous versions:
  • strawberry-graphql==0.219.1
  • strawberry-graphql-django==0.32.1
  • According to the documentation:
  • The issue seems to be that multipart_uploads_enabled is not properly propagated to the AsyncGraphQLView instance.

If I force it to True in this method everthing works fine:

    async def parse_http_body(
        self, request: AsyncHTTPRequestAdapter
    ) -> GraphQLRequestData:
        headers = {key.lower(): value for key, value in request.headers.items()}
        content_type, _ = parse_content_type(request.content_type or "")
        accept = headers.get("accept", "")

        protocol: Literal["http", "multipart-subscription"] = "http"
        if self._is_multipart_subscriptions(*parse_content_type(accept)):
            protocol = "multipart-subscription"

        if request.method == "GET":
            data = self.parse_query_params(request.query_params)
        elif "application/json" in content_type:
            data = self.parse_json(await request.get_body())
        elif self.multipart_uploads_enabled and content_type == "multipart/form-data":
            data = await self.parse_multipart(request)
        else:
            raise HTTPException(400, "Unsupported content type")

        return GraphQLRequestData(
            query=data.get("query"),
            variables=data.get("variables"),
            operation_name=data.get("operationName"),
            protocol=protocol,
        )

Question

Am I misconfiguring something, or is this a bug in strawberry-graphql? Any guidance on how to fix or work around this issue would be appreciated.

@BranDavidSebastian BranDavidSebastian added the bug Something isn't working label Nov 29, 2024
@patrick91 patrick91 transferred this issue from strawberry-graphql/strawberry-django Nov 30, 2024
@DoctorJohn DoctorJohn self-assigned this Dec 9, 2024
@DoctorJohn
Copy link
Member

Hi @BranDavidSebastian, I'm having a hard time reproducing this. We have (passing) tests for this and I just created an example repository containing a minimal (working) demo for this.

Can you take a look at the demo repository and confirm I understood your issue/setup correctly?

Besides that only two things come to my mind at this point:

  1. Are you possibly subclassing AsyncGraphQLView and not calling super().__init__(*args, **kwargs) from your subclass?
  2. Since you said multipart_uploads_enabled is False in your case it shouldn't make a difference, but does using the csrf_exempt decorator as suggested in the docs help?

@DoctorJohn DoctorJohn added the info-needed Needs more info from OP label Dec 23, 2024
@BranDavidSebastian
Copy link
Author

BranDavidSebastian commented Dec 31, 2024

Hello @DoctorJohn. thank you for the example repository and the help. After I played it for a while and started to set up closer to my actual environment I realized I forgot to mention an essential detail in the initial issue: I’m using Strawberry in an ASGI + Channels+Daphne setup.

I’ve forked your example to show, step by step, how we can end up with “Unsupported content type” in that environment here.

In essence the big difference is the asgi.py that looks like this:

import os
from django.conf import settings
from django.core.asgi import get_asgi_application
from strawberry_django.routers import AuthGraphQLProtocolTypeRouter
from starlette.middleware.cors import CORSMiddleware

os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'example.settings')
django_application = get_asgi_application()


# Import your Strawberry schema after creating the django ASGI application
# This ensures django.setup() has been called before any ORM models are imported
# for the schema.
from .schema import schema  # NOQA
application = AuthGraphQLProtocolTypeRouter(
    schema,
    django_application=django_application,
)

application = CORSMiddleware(
    application,
    allow_origins=settings.CORS_ALLOWED_ORIGINS,
    allow_headers=settings.CORS_ALLOWED_HEADERS,
    allow_methods=settings.CORS_ALLOWED_METHODS,
    allow_credentials=settings.CORS_ALLOW_CREDENTIALS
)

After installing Channels and Daphne, you’ll see the issue. I noticed that in AuthGraphQLProtocolTypeRouter:

http_urls: list[URLPattern | URLResolver] = [
            re_path(url_pattern, GraphQLHTTPConsumer.as_asgi(schema=schema)),
        ]

So requests end up at GraphQLHTTPConsumer, where multipart_uploads_enabled defaults to False if it isn’t passed (and is never passed). Since they never reach AsyncGraphQLView, the file upload config doesn’t apply, leading to the error.

For a fix I found out that the most straight forward one will be to be able to config the multipart_uploads_enabled inside the AuthGraphQLProtocolTypeRouter like this:

class AuthGraphQLProtocolTypeRouter(ProtocolTypeRouter):
    """Convenience class to set up GraphQL on both HTTP and Websocket.

     This convenience class will include AuthMiddlewareStack and the
     AllowedHostsOriginValidator to ensure you have user object available.

    ```
    from strawberry_django.routers import AuthGraphQLProtocolTypeRouter
    from django.core.asgi import get_asgi_application.

    django_asgi = get_asgi_application()

    from myapi import schema

    application = AuthGraphQLProtocolTypeRouter(
        schema,
        django_application=django_asgi,
    )
    ```

    This will route all requests to /graphql on either HTTP or websockets to us,
    and everything else to the Django application.
    """

    def __init__(
        self,
        schema: BaseSchema,
        django_application: ASGIHandler | None = None,
        url_pattern: str = "^graphql",
        multipart_uploads_enabled: bool = False,
    ):
        http_urls: list[URLPattern | URLResolver] = [
            re_path(url_pattern, GraphQLHTTPConsumer.as_asgi(schema=schema, multipart_uploads_enabled=multipart_uploads_enabled)),
        ]
        if django_application is not None:
            http_urls.append(re_path(r"^", django_application))

        super().__init__(
            {
                "http": AuthMiddlewareStack(
                    URLRouter(
                        http_urls,
                    ),
                ),
                "websocket": AllowedHostsOriginValidator(
                    AuthMiddlewareStack(
                        URLRouter(
                            [
                                re_path(
                                    url_pattern,
                                    GraphQLWSConsumer.as_asgi(schema=schema),
                                ),
                            ],
                        ),
                    ),
                ),
            },
        )

And then we can simply config like this:

application = AuthGraphQLProtocolTypeRouter(
    schema,
    django_application=django_application,
    multipart_uploads_enabled=True
)

application = CORSMiddleware(
    application,
    allow_origins=settings.CORS_ALLOWED_ORIGINS,
    allow_headers=settings.CORS_ALLOWED_HEADERS,
    allow_methods=settings.CORS_ALLOWED_METHODS,
    allow_credentials=settings.CORS_ALLOW_CREDENTIALS
)

It works well but requires setting multipart_uploads_enabled in two places, which might not be ideal. I’m not entirely familiar with the codebase, so there may be a better approach

Thank you for your assistance! I’m looking forward to getting this resolved, as it’s currently blocking a major Strawberry upgrade release in one of our projects. In the worst-case scenario, we can override the AuthGraphQLProtocolTypeRouter in our project until the fix is applied in the main repository. Please let me know if there’s anything I can do to help and sorry again for the time lost due to my oversight in not mentioning the ASGI part.

@github-actions github-actions bot removed the info-needed Needs more info from OP label Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants