transloadit / uppy

The next open source file uploader for web browsers :dog:
https://uppy.io
MIT License
29.2k stars 2.01k forks source link

Adding additional parameters for XHRUpload asynchronously #785

Closed janko closed 5 years ago

janko commented 6 years ago

On each XHRUpload I would like to include a Content-MD5 request header which contains the checksum of the file. I'm calculating the checksum using the spark-md5 and chunked-file-reader libraries:

function fileMD5 (file) {
  return new Promise((resolve, reject) => {
    var spark  = new SparkMD5.ArrayBuffer(),
        reader = new ChunkedFileReader();

    reader.subscribe('chunk', (e) => {
      spark.append(e.chunk);
    });

    reader.subscribe('end', (e) => {
      var rawHash    = spark.end(true);
      var base64Hash = btoa(rawHash);

      resolve(base64Hash);
    });

    reader.readChunks(file);
  });
}

Calculating the MD5 hash is asynchronous, as it uses FileReader which is asynchronous, so I wrapped it into a Promise. However, now I'm a bit stuck, because I don't know how to tell Uppy to wait until the MD5 hash has been generated before proceeding with the upload.

Btw, for AwsS3 uploads I was able to make it work, because there I needed to calculate the MD5 hash before fetching upload parameters (which returns Content-MD5 header in headers in the response), and since getUploadParameters expects a Promise I was able to chain generating an MD5 hash before fetching upload parameters:

uppy.use(Uppy.AwsS3, {
  getUploadParameters (file) {
    return fileMD5(file.data)
      .then((hash) => { return fetch('/presign?filename='+ file.name + '&checksum=' + hash) })
      .then((response) => { return response.json() })
  }
})
janko commented 6 years ago

I've an idea: what if we moved getUploadParameters functionality into the XHRUpload plugin? It would then be optional, and the result could be merged with already specified fields and headers.

The XHRUpload plugin could then be used with Google Cloud Storage and other services (the documentation from https://github.com/transloadit/uppy/pull/777 could then be moved to XHRUpload), and the AwsS3 plugin would just add some S3-specific logic on top of XHRUpload. It's not intuitive that the AwsS3 plugin can be used services other than AWS S3, but XHRUpload already has the generic name.

I think that would make sense, because :method, :url, :fields, and :headers define the whole request, so the getUploadParameters can be supported by any service.

That would easily allow adding the Content-MD5 request header, because getUploadParameters would support Promises, so it's not limited to only HTTP requests but any operation that's async:

uppy.use(Uppy.XHRUpload, {
  getUploadParameters (file) {
    return fileMD5(file.data)
      .then((md5) => { return { headers: { 'Content-MD5': md5 } } })
  }
})

What do you think?

goto-bus-stop commented 6 years ago

Hmm yes, that doesn't sound bad at all :thinking:

AshUK commented 6 years ago

We do something like this when our access token expires.

this.uppy.setState({
  xhrUpload: {
    headers: {
      'authorization': `Bearer ${nextProps.user.access_token}`
    }
  }
});
kvz commented 5 years ago

@AshUK that's indeed an officially supported (but undocumented) feature. The S3 plugin also does it this way. We think it might be nice to support @janko's way though, because we can then enjoy prettier syntax as well as per-file headers (kinda required when sending md5s!)

We've added this feature to our backlog and will re-open it when we start working on it. In the meantime we'd very much welcome community-driven PRs!

khawarizmus commented 3 years ago

@janko @AshUK can we have getUploadParameters (file) option added to the XHRUpload plugin please?