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

Handle custom stream #382

Merged
merged 4 commits into from Feb 15, 2021
Merged

Handle custom stream #382

merged 4 commits into from Feb 15, 2021

Conversation

wxt2005
Copy link
Contributor

@wxt2005 wxt2005 commented Mar 27, 2018

Recently I run into a problem while using Request with form-data, I tried to submit a custom stream provided by another library, but then I found form-data incorrectly calculated the content length.
I searched for issues and PRs, then I found #70, for unfortunately it's not be merged. So I made this new PR, all codes except minor changes are come from #70.

@coveralls
Copy link

coveralls commented Mar 27, 2018

Coverage Status

Coverage increased (+0.5%) to 98.655% when pulling e705c0a on wxt2005:custom-stream into 55d90ce on form-data:master.

@alexindigo
Copy link
Member

Thank you for the pull-request. Do you mind to update Readme with new state of things?

I restarted broken windows build, seems like it was an environment issue.

@wxt2005
Copy link
Contributor Author

wxt2005 commented Mar 28, 2018

Of course. I've added notes about those new behaviors.

@giautm
Copy link

giautm commented Jun 28, 2018

I ran into the same issues when I trying forward incoming stream from Multer to the storage service. I have used the fork of @wxt2005 and its working as perfect. 👍Pleaseeee, merge this soon. Tks. :)

var crypto = require('crypto');
var FormData = require('form-data');
var fetch = require('node-fetch');

function getFilename (req, file, cb) {
  crypto.pseudoRandomBytes(16, function (err, raw) {
    cb(err, err ? undefined : raw.toString('hex'));
  });
}

function getBucketTmp (req, file, cb) {
  cb(null, 'tmp');
}

function TalariaStorage(opts) {
  this.serviceUrl = opts.serviceUrl;
  if (!this.serviceUrl) {
    throw new Error('Missing `serviceUrl` options');
  }

  this.getFilename = (opts.filename || getFilename);
  if (typeof opts.bucket === 'string') {
    this.getBucket = ($0, $1, cb) => cb(null, opts.bucket);
  } else {
    this.getBucket = (opts.bucket || getBucketTmp);
  }
}

TalariaStorage.prototype._handleFile = function _handleFile (req, file, cb) {
  this.getBucket(req, file, (err, bucket) => {
    if (err) return cb(err);

    this.getFilename(req, file, (err, filename) => {
      if (err) return cb(err);

      var form = new FormData();
      form.append('bucket', bucket);
      form.append('file', file.stream, filename);

      fetch(this.serviceUrl, {
        method: 'POST',
        body: form,
      }).then(res => res.json())
        .then(result => cb(null, {
          bucket,
          filename,
          key: result.key,
          location: result.url,
        }))
        .catch(cb);
    });
  });
};

TalariaStorage.prototype._removeFile = function _removeFile (req, file, cb) {
  cb(null);
};

module.exports = function (opts) {
  return new TalariaStorage(opts);
};

@anthnyprschka
Copy link

anthnyprschka commented Jan 9, 2019

I had a similar problem and @wxt2005's fork fixed it for me. But then I discovered that request allows you to pass a knownLength parameter to the options of a formData property. This also fixes the problem for custom streams whose length you know beforehand

@giautm
Copy link

giautm commented Jan 10, 2019

@alexindigo: ping

Readme.md Outdated Show resolved Hide resolved
@jakeweary
Copy link

Any chance of merging this in near future?

jakeweary added a commit to jakeweary/discord.js that referenced this pull request Apr 15, 2020
@wiktor-obrebski
Copy link

I have same issue. do we wait 5 months just for fixing typo in README?
can we merge it already?

@wxt2005
Copy link
Contributor Author

wxt2005 commented Jul 3, 2020

Hi,@TPXP, sorry I didn't notice your comment before, now the typo has been fixed.

@nytamin
Copy link

nytamin commented Jan 27, 2021

I have had the same issue which this PR solved, can this be merged please?

@niftylettuce
Copy link
Contributor

I'm now a maintainer. v4.0.0 released https://github.com/form-data/form-data/releases/tag/v4.0.0

@gish
Copy link

gish commented Feb 22, 2021

Just out of curiosity, what warranted a major version bump? I would have assumed this bugfix should have been a patch bump.

@pluma
Copy link

pluma commented Mar 31, 2021

@gish Looks like 4.0.0 merged a branch that dropped support for older Node versions. This warrants a major version bump even if nothing else changed.

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

Successfully merging this pull request may close these issues.

None yet