Apollo-server: Update to use grahpql-upload v8

Created on 24 Oct 2018  Â·  4Comments  Â·  Source: apollographql/apollo-server

You can see in the graphql-upload (formerly apollo-upload-server) changelog that a lot of critical improvements have been made between v5 and v7. Some of the problems were very challenging to solve! It should be stable now and we have a lot of tests.

Everyone should migrate from apollo-upload-server v5 to graphql-upload v8+ ASAP.

apollo-upload-server should be updated to use latest graphql-upload

🧬 uploads

Most helpful comment

This is a pretty critical issue. Not only are people missing out on a huge amount of fixes and a much more powerful API, but the public can crash Apollo Server v2 infrastructure by sending malformed multipart requests (in theory; I have not gone around trying it). People are even talking about filing CVEs!

Here is a PR that would resolve this issue: https://github.com/apollographql/apollo-server/pull/1764.

I ❤️you all at Apollo, event if this particular issue needs work. I don't chase dramas, I'm just trying to make the world a better place and offer some transparency to the community. It's their apps that are affected, after all 🙏

I'll be in San Francisco next week for GraphQL Summit, and am happy to meet anyone that would like to talk about it in person or collaborate.

Context

Before apollo-upload-server (since renamed graphql-upload) was implemented in Apollo Server v2, unsolved challenges and their security implications were transparent to the Apollo and the public via Github issues and Apollo Slack team threads. I also messaged core Apollo team members via Slack about the challenges and gave my best estimates as to when they might be resolved. The response was that as long as they get fixed sometime soonish, not to worry.

It turns out that running resolvers before the request has finished, so that file upload streams can be handled efficiently in resolvers, is super challenging for a lot of reasons. I worked tirelessly on tests for every edge case and solid solutions to each challenge (requests aborted half way through, the same file being used as an argument for multiple mutations, files upload streams being consumed out of order, etc. etc.) with heroic contributions from @mike-marcacci who created fs-capacitor as a big part of the solution. All the issues have been resolved! Today there are 94 closed issues, and only 2 open… A question, and a request for Node.js v6 support:

screen shot 2018-10-29 at 10 46 25 pm

In the process of solving all the complicated challenges across multiple versions of Node.js (with their subtle and not-so-subtle breaking API changes to streams, etc.), Node.js support was lifted from v6 to v8.5. See https://github.com/jaydenseric/graphql-upload/issues/109. I never thought this was a big deal, since absolutely no one I know in 2018 is using Node.js v6, which is only a few months away from end-of-life, when v8 LTS and v10 LTS are available and vastly superior. It turns out that Apollo _really_ doesn't want to drop Node.js v6.

I've said that I'm willing to accept a PR adding Node.js v6 support, even though it means compromising some modern features that I've come to enjoy, such as native async/await. I've even cleared the path by dropping support back to Node.js v6 for a dev dependency.

Not everyone uses Apollo Server. express-graphql gets around 112k downloads per month while apollo-server-core gets around 150k. Personally I've used most of the GraphQL servers available on npm including Apollo Server, and currently use graphql-api-koa. graphql-upload supports most GraphQL servers. My point is, if something like Node.js v6 support is specifically important to Apollo, a for-profit company, it is the perfect opportunity to make a contribution back to the project via a PR. After all, I have contributed to Apollo repos before and have sent _hundreds_ of messages of free support for the community on Slack; I get notifications anytime the word upload is mentioned.

I've spent months of my life dedicated to this problem space at great personal cost, with no contributions from Apollo, despite it powering a major feature of Apollo Server, one of their flagship products. All along, I have been motivated by the promise that the work will help not just myself, but a lot of other people handle uploads intuitively and efficiently in their apps. It breaks my heart every day to see a horribly broken fork (@apollographql/apollo-upload-server) of the last version that happened to support Node.js v6 (that predates fixes) being forced on a trusting community to the tune of 136k installs per week:

screen shot 2018-10-29 at 10 58 16 pm

Users keep coming to me for support about issues we already worked hard to solve _months_ ago. This poor developer experience needlessly tarnishes both my own and Apollo's reputation and really hits morale.

Due to a lack of progress I've tried my best to reach out in the #apollo-maintainers Slack channel multiple times…

screen shot 2018-10-30 at 12 01 42 am

The problem and solution gets acknowledged…

screen shot 2018-10-30 at 12 03 18 am

Then ignored again…

screen shot 2018-10-29 at 10 18 55 pm

What Apollo Server users can do in the meantime

I have updated the apollo-upload-examples API to disable the outdated and dangerous Apollo Server upload handling and use graphql-upload instead; here is the commit.

To migrate your existing Apollo Server v2 project:

  1. npm install graphql-upload.
  2. Disable the Apollo Server upload handling by using uploads: false in the ApolloServer options.
  3. Setup the graphql-upload middleware.
  4. Setup the Upload scalar resolver.
  5. Migrate to the new API in your mutation resolvers:

    - const { stream, filename, mimetype } = await upload
    + const { createReadStream, filename, mimetype } = await upload
    + const stream = createReadStream()
    

    If you don't, it will still work for now but you will see this deprecation warning:

    screen shot 2018-10-29 at 10 03 43 pm

    This API change happened in v7 and was nessesary to fix a whole bunch of issues, the most obvious being that you can now use a single Upload as a variable that can be used as input for multiple mutations in one request. Previously, consuming the stream in one resolver would interfere with consuming the same stream in another.

All 4 comments

This is a pretty critical issue. Not only are people missing out on a huge amount of fixes and a much more powerful API, but the public can crash Apollo Server v2 infrastructure by sending malformed multipart requests (in theory; I have not gone around trying it). People are even talking about filing CVEs!

Here is a PR that would resolve this issue: https://github.com/apollographql/apollo-server/pull/1764.

I ❤️you all at Apollo, event if this particular issue needs work. I don't chase dramas, I'm just trying to make the world a better place and offer some transparency to the community. It's their apps that are affected, after all 🙏

I'll be in San Francisco next week for GraphQL Summit, and am happy to meet anyone that would like to talk about it in person or collaborate.

Context

Before apollo-upload-server (since renamed graphql-upload) was implemented in Apollo Server v2, unsolved challenges and their security implications were transparent to the Apollo and the public via Github issues and Apollo Slack team threads. I also messaged core Apollo team members via Slack about the challenges and gave my best estimates as to when they might be resolved. The response was that as long as they get fixed sometime soonish, not to worry.

It turns out that running resolvers before the request has finished, so that file upload streams can be handled efficiently in resolvers, is super challenging for a lot of reasons. I worked tirelessly on tests for every edge case and solid solutions to each challenge (requests aborted half way through, the same file being used as an argument for multiple mutations, files upload streams being consumed out of order, etc. etc.) with heroic contributions from @mike-marcacci who created fs-capacitor as a big part of the solution. All the issues have been resolved! Today there are 94 closed issues, and only 2 open… A question, and a request for Node.js v6 support:

screen shot 2018-10-29 at 10 46 25 pm

In the process of solving all the complicated challenges across multiple versions of Node.js (with their subtle and not-so-subtle breaking API changes to streams, etc.), Node.js support was lifted from v6 to v8.5. See https://github.com/jaydenseric/graphql-upload/issues/109. I never thought this was a big deal, since absolutely no one I know in 2018 is using Node.js v6, which is only a few months away from end-of-life, when v8 LTS and v10 LTS are available and vastly superior. It turns out that Apollo _really_ doesn't want to drop Node.js v6.

I've said that I'm willing to accept a PR adding Node.js v6 support, even though it means compromising some modern features that I've come to enjoy, such as native async/await. I've even cleared the path by dropping support back to Node.js v6 for a dev dependency.

Not everyone uses Apollo Server. express-graphql gets around 112k downloads per month while apollo-server-core gets around 150k. Personally I've used most of the GraphQL servers available on npm including Apollo Server, and currently use graphql-api-koa. graphql-upload supports most GraphQL servers. My point is, if something like Node.js v6 support is specifically important to Apollo, a for-profit company, it is the perfect opportunity to make a contribution back to the project via a PR. After all, I have contributed to Apollo repos before and have sent _hundreds_ of messages of free support for the community on Slack; I get notifications anytime the word upload is mentioned.

I've spent months of my life dedicated to this problem space at great personal cost, with no contributions from Apollo, despite it powering a major feature of Apollo Server, one of their flagship products. All along, I have been motivated by the promise that the work will help not just myself, but a lot of other people handle uploads intuitively and efficiently in their apps. It breaks my heart every day to see a horribly broken fork (@apollographql/apollo-upload-server) of the last version that happened to support Node.js v6 (that predates fixes) being forced on a trusting community to the tune of 136k installs per week:

screen shot 2018-10-29 at 10 58 16 pm

Users keep coming to me for support about issues we already worked hard to solve _months_ ago. This poor developer experience needlessly tarnishes both my own and Apollo's reputation and really hits morale.

Due to a lack of progress I've tried my best to reach out in the #apollo-maintainers Slack channel multiple times…

screen shot 2018-10-30 at 12 01 42 am

The problem and solution gets acknowledged…

screen shot 2018-10-30 at 12 03 18 am

Then ignored again…

screen shot 2018-10-29 at 10 18 55 pm

What Apollo Server users can do in the meantime

I have updated the apollo-upload-examples API to disable the outdated and dangerous Apollo Server upload handling and use graphql-upload instead; here is the commit.

To migrate your existing Apollo Server v2 project:

  1. npm install graphql-upload.
  2. Disable the Apollo Server upload handling by using uploads: false in the ApolloServer options.
  3. Setup the graphql-upload middleware.
  4. Setup the Upload scalar resolver.
  5. Migrate to the new API in your mutation resolvers:

    - const { stream, filename, mimetype } = await upload
    + const { createReadStream, filename, mimetype } = await upload
    + const stream = createReadStream()
    

    If you don't, it will still work for now but you will see this deprecation warning:

    screen shot 2018-10-29 at 10 03 43 pm

    This API change happened in v7 and was nessesary to fix a whole bunch of issues, the most obvious being that you can now use a single Upload as a variable that can be used as input for multiple mutations in one request. Previously, consuming the stream in one resolver would interfere with consuming the same stream in another.

File returns a promise and inside an array. This is a solution for those who encounter the same issue as I do
`` uploadMaterialImage: async (p, a, { models, user }) => { const { env: { PUBLICDIR } } = process; const uploadDir = "." + PUBLICDIR + "/images"; let { file: [file] } = await a; return file.then( async ({ filename, mimetype, encoding, createReadStream }) => { const stream = createReadStream(); const { ok, errors, payload } = await (async () => { const id = shortid.generate(); let filepath =${uploadDir}/${id}-${filename}`;
return new Promise((resolve, reject) => {
stream.on('error', err => {
if (stream.truncated) {
// Delete the truncated file
fs.unlinkSync(filepath);
}
reject({ok: false, errors: err});
});

              stream
                .pipe(fs.createWriteStream(filepath))
                .on('error', err => reject({ok: false, errors: err}))
                .on('finish', () => {
                  filepath = filepath.split(".")
                  filepath.shift();
                  filepath = filepath.join(".")
                  return resolve({ok: true, payload: {
                  path: filepath
                 }})});
            });
          })();
          return {
            ok,
            errors,
            payload
          }
      }
    )
}

```

This should be addressed by #2054. As I've requested in that PR, I'd really appreciate anyone who is utilizing file uploads to try upgrading to the published alpha which updates graphql-upload to v8. I've detailed the progress on this matter extensively in #2054, but the high-bit is that this should be ready to try out now in [email protected]. Please detail any problems (or successes!) you encounter with the upgrade, as the feedback will guide its final release.

Ref: https://github.com/apollographql/apollo-server/pull/2054#issuecomment-444471202

The alpha releases didn't identify any problems so I've graduated this to the official apollo-server-*@2.3.0 releases.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dbrrt picture dbrrt  Â·  3Comments

jpcbarros picture jpcbarros  Â·  3Comments

manuelfink picture manuelfink  Â·  3Comments

dupski picture dupski  Â·  3Comments

hiucimon picture hiucimon  Â·  3Comments