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

Posting FormData as body (for file uploads) not possible anymore #174

Closed
codejet opened this issue Mar 17, 2023 · 7 comments
Closed

Posting FormData as body (for file uploads) not possible anymore #174

codejet opened this issue Mar 17, 2023 · 7 comments

Comments

@codejet
Copy link

codejet commented Mar 17, 2023

Hi,

we currently still use apollo-datasource-rest, with which it’s possible to pass a body that is FormData as a second argument to the post method of a data source. That’s what we did for file uploads in our app, which also seems to be the most straight-forward approach. But with @apollo/datasource-rest that’s not possible anymore.
Looking at the source code of the fetch method of both packages, it seems like now everything that is not passing the following conditions (which are present in both) is being ignored:

    if (
      request.body !== undefined &&
      request.body !== null &&
      (request.body.constructor === Object ||
        Array.isArray(request.body) ||
        ((request.body as any).toJSON &&
          typeof (request.body as any).toJSON === 'function'))
    )

With apollo-datasource-rest a body which fell through that if statement was simply passed on as is.

What is the reasoning behind not allowing something non-stringified to be sent? Security?

@glasser
Copy link
Member

glasser commented Mar 17, 2023

What version of @apollo/datasource-rest are you running? The current version changes the code you quote to specifically support FormData (see #90).

@codejet
Copy link
Author

codejet commented Mar 17, 2023

Currently 4.3.2, but I also briefly tried with 5.x which seemed to result in the same 🤔

@codejet
Copy link
Author

codejet commented Mar 17, 2023

OK, thanks for the hint, @glasser. I will look into shouldJSONSerializeBody.

@glasser
Copy link
Member

glasser commented Mar 17, 2023

5.x is the version where we addressed a several year of bug reports. It ought to work with FormData out of the box, as long as you're using a FormData implementation that's compatible with your fetch implementation. The default fetch implementation is node-fetch which works with the FormData implementation in the form-data package.

@glasser glasser closed this as not planned Won't fix, can't repro, duplicate, stale Mar 17, 2023
@codejet
Copy link
Author

codejet commented Mar 20, 2023

@glasser I now tried with v5 and the file upload still doesn't work. I used willSendRequest and didEncounterError to get more info. In willSendRequest I see the following content of the FormData object's _streams property (created by form-data):

    _streams: [
      '----------------------------221712049102902487997513\r\n' +
        'Content-Disposition: form-data; name="categoryId"\r\n' +
        '\r\n',
      '1566',
      [Function: bound ],
      '----------------------------221712049102902487997513\r\n' +
        'Content-Disposition: form-data; name="categoryId"\r\n' +
        '\r\n',
      '1566',
      [Function: bound ],
      '----------------------------221712049102902487997513\r\n' +
        'Content-Disposition: form-data; name="adsFile"; filename="export_category_2632.csv"\r\n' +
        'Content-Type: text/csv\r\n' +
        '\r\n',
      <Buffer 69 64 2c 74 69 74 6c 65 2c 64 65 73 63 72 69 70 74 69 6f 6e 2c 65 78 74 65 72 6e 61 6c 5f 69 64 2c 6c 6f 63 61 74 69 6f 6e 5f 63 69 74 79 2c 6c 6f 63 ... 511 more bytes>,
      [Function: bound ]
    ],

But didEncounterError gives me a GraphQLError: 400: Bad Request (and our Spring Boot/Kotlin server logs a MissingServletRequestPartException) and the outgoingRequest has an empty _streams property:

    _streams: [],

This makes it look like the contents of that property are somehow removed before sent to the data source, no? Atm I just wouldn't know why.

@glasser
Copy link
Member

glasser commented Mar 20, 2023

If you provide a full reproduction with no creativity required to try it out, I'd be happy to give it a look. This stuff is subtle enough that it can't really be debugged from snippets.

@codejet
Copy link
Author

codejet commented Mar 21, 2023

Thanks @glasser, but I found the issue. It was a discrepancy between the boundary of the FormData and the boundary in the Content-Type header (due to trying to manually create the boundary for the header after switching to v4 of the package; now using form-data's getHeaders).

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