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

Why misorder of operations and map will cause error thrown? #372

Closed
Sczlog opened this issue Jun 21, 2023 · 3 comments
Closed

Why misorder of operations and map will cause error thrown? #372

Sczlog opened this issue Jun 21, 2023 · 3 comments

Comments

@Sczlog
Copy link

Sczlog commented Jun 21, 2023

I'm not quitely sure if formdata has order, but some http client implementation, like go http client, use map[string][]string as formdata's data structure which has no order, may cause the order is unreliable on client side.

image

I read source code and found parsing map require operations, but if make a change like this(a tempo workaround, test not updated and may need update), it can whatever operations come first or map come first.

4a79f7d

ff we get map first, just delay resolve after we get operations, if we get operations first, work like normal.

@jaydenseric
Copy link
Owner

jaydenseric commented Jun 21, 2023

Multipart HTTP requests definitely support ordering fields in the request; they are streamed one after another in the request body. If certain clients don't allow you to specify the order of fields, then I would consider that an issue with the client that needs to be improved. Even if we relax this graphql-upload implementation of the GraphQL multipart request spec to allow miss ordering the operations field before the map field, that won't solve your problem of the client sending the fields in an unpredictable order because it definitely matters a lot for performance if the file fields are not streamed after operations and map.

@Sczlog
Copy link
Author

Sczlog commented Jun 21, 2023

In this scenario, seems like file is always composing in the last position. (idk why).

And I do some research and w3c form spec said

The parts are sent to the processing agent in the same order the corresponding controls appear in the document stream

So it looks like golang's stdlib didn't implement formdata correctly, try to find some other solution.

@jaydenseric
Copy link
Owner

Closing because graphql-upload is functioning correctly according to the GraphQL multipart request specification. Any changes to the that specification would have to happen in the spec repo first, then once there is an updated spec, server and client implementations can be updated to conform to the new spec. But I can tell right away, that the ordering of fields is not something likely to ever change in the spec.

@jaydenseric jaydenseric closed this as not planned Won't fix, can't repro, duplicate, stale Aug 27, 2023
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