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

GraphQLView should detect batch requests #967

Open
McPo opened this issue May 21, 2020 · 7 comments
Open

GraphQLView should detect batch requests #967

McPo opened this issue May 21, 2020 · 7 comments

Comments

@McPo
Copy link

McPo commented May 21, 2020

Is there a valid reason why the GraphQLView input is validated to either only ever accept batched requests or never accept batched requests. Is it not reasonable to accept both?

@jkimbo
Copy link
Member

jkimbo commented Jun 6, 2020

I'm not sure why it was setup that way @McPo but I guess the view could handle both kinds of requests. I would expect a client to either always batch requests or not though. Do you have a client that will sometimes batch requests?

@McPo
Copy link
Author

McPo commented Jun 8, 2020

Yes,

So if you use apollo-upload-client, it doesn't support batching. The solution is to then deactivate batching when uploading file jaydenseric/apollo-upload-client#34 (comment).

As such I think this would be a fairly common use-case, and if theres no advantage to the server enforcing one or the other, it should be flexible. I see some people are making a separate /batch endpoint to support this, which seems redundant.

Anyway, Ive managed to monkey patch it for the time being.

class CustomGraphQLView(GraphQLView):
    schema = schema
    graphiql = settings.DEBUG

    def parse_body(self, request):
        # Allow for variable batch
        try:
            body = request.body.decode("utf-8")
            request_json = json.loads(body)
            self.batch = isinstance(request_json, list)
        except:
            self.batch = False
        return super().parse_body(request)

@Eraldo
Copy link

Eraldo commented Jun 25, 2020

I am using apollo's upload client as well.
Same (or similar) challenge.
Thanks for bringing this up @McPo. :)

@jkimbo
Copy link
Member

jkimbo commented Jun 25, 2020

Makes sense. Sounds like this would be a good enhancement if anyone wants to create a PR.

@Eraldo
Copy link

Eraldo commented Jun 25, 2020

Makes sense. Sounds like this would be a good enhancement if anyone wants to create a PR.

While at it.

I am actually using this:

import json

from graphene_file_upload.django import FileUploadGraphQLView


class GraphQLView(FileUploadGraphQLView):
    """Handles multipart/form-data content type in django views"""

    def parse_body(self, request):
        """
        Allow for variable batch
        https://github.com/graphql-python/graphene-django/issues/967#issuecomment-640480919
        :param request:
        :return:
        """
        try:
            body = request.body.decode("utf-8")
            request_json = json.loads(body)
            self.batch = isinstance(request_json, list)
        except:
            self.batch = False
        return super().parse_body(request)

=> To support batching and file multipart file uploads.
Would be awesome to include both capabilities. 👍

@jaydenseric
Copy link

To support batching and file multipart file uploads.

Note that the GraphQL multipart request spec supports file uploads with batching:

https://github.com/jaydenseric/graphql-multipart-request-spec#batching

apollo-upload-client only doesn't do this because an Apollo Client implementation is tricky (see jaydenseric/apollo-upload-client#34 (comment)), I don't have the time to work on it and no one else has stepped up to the plate (jaydenseric/apollo-upload-client#34 (comment)).

A GraphQL server will ideally accept batched and unbatched requests, for both regular and multipart requests.

@jkimbo jkimbo changed the title Overzealous input validation for batching GraphQLView should detect batch requests Jun 28, 2020
@Eraldo
Copy link

Eraldo commented Mar 25, 2024

My linter (rightfully) complains about except: being to generic.
Are there any specific anticipated/expected exception types?
If I don't get an answer, I will remove the catch and see what I get. ^^

Found so far: RawPostDataException

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants