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

Redirect body larger than maxBodyLength not correctly caught #1362

Open
albertogasparin opened this issue Feb 13, 2018 · 9 comments

Comments

Projects
None yet
8 participants
@albertogasparin
Copy link

commented Feb 13, 2018

Summary

Recently #1323 has been fixed, however follow-redirects is still not correctly implemented: current version does not handle errors correctly.
Indeed, if maxBodyLength is 10MB and the resource fetched after the redirect is bigger, follow-redirects will emit an error but that error will occur outside of axios promise chain.

Error: Request body larger than maxBodyLength limit
      at RedirectableRequest.write (./node_modules/follow-redirects/index.js:66:24)
      at FormData.CombinedStream.write (./node_modules/combined-stream/lib/combined_stream.js:118:8)
      at FormData.CombinedStream._pipeNext (./node_modules/combined-stream/lib/combined_stream.js:106:8)
      at FormData.CombinedStream._getNext (./node_modules/combined-stream/lib/combined_stream.js:79:10)
      at FormData.CombinedStream._pipeNext (./node_modules/combined-stream/lib/combined_stream.js:107:8)
      at FormData.CombinedStream._getNext (./node_modules/combined-stream/lib/combined_stream.js:79:10)
      at FormData.CombinedStream.resume (./node_modules/combined-stream/lib/combined_stream.js:134:10)
      at FormData.CombinedStream.pipe (./node_modules/combined-stream/lib/combined_stream.js:64:8)
      at dispatchHttpRequest (./node_modules/axios/lib/adapters/http.js:227:12)
      at httpAdapter (./node_modules/axios/lib/adapters/http.js:18:10)
      at dispatchRequest (./node_modules/axios/lib/core/dispatchRequest.js:59:10)

Axios should intercept that error and reject the promise

Context

  • axios version: v0.17.1 + master
  • Environment: node v8.4.0
@emilyemorehouse

This comment has been minimized.

Copy link
Member

commented Feb 16, 2018

I'm having trouble duplicating this locally (and I actually went down quite a rabbit-hole of finding cases that get caught by Axios from maxContentLength but not follow-redirects maxBodyLength). 😅

Can you share an example that demonstrates the error?

@scragg0x

This comment has been minimized.

Copy link

commented Feb 20, 2018

Keep in mind that follow-redirects's maxBodyLength is in bytes and the Axios example on the README can be confusing because it uses an example of 2000 for maxContentLength. No mention of units and 2KB is an odd example, I assumed it was 2MB at first. maxContentLength is assigned directly to maxBodyLength when specified.

@albertogasparin

This comment has been minimized.

Copy link
Author

commented Feb 25, 2018

I tried to create a reproducible scenario, however the only way I'm able to trigger this error is while downloading images > 10MB from Gettyimages (which is an authenticated endpoint).
My suggestion is that, as a precaution, axios should default follow-redirect maxBodyLength limit to Infinity, for the same reason as maxContentLength default is -1.

Currently, maxBodyLength and maxContentLength are synched only when maxContentLength is explicitly set, leaving the default scenario uncovered (maxBodyLength default to 10MB, which is quite low).

@gordonk

This comment has been minimized.

Copy link

commented Mar 27, 2018

For benefit of anyone landing here, @scragg0x is correct, the configuration param is in bytes.
E.g. for 50MB request/response upper limit,

maxContentLength: 52428890

@shubhi15

This comment has been minimized.

Copy link

commented May 3, 2018

Facing same issue . Even after adding maxContentLength: 52428890 to axios post request has not fixed it.

@RikkiGibson

This comment has been minimized.

Copy link
Contributor

commented May 21, 2018

Looks like the noting maxContentLength being in bytes is in the README now, which is good:

#1463

It might be useful to indicate what the default for maxContentLength is. But it seems like it might just be inherited from follow-redirects..

@amjadalibb

This comment has been minimized.

Copy link

commented May 25, 2018

Anyone interested in fixing the issue ? I have below code but still getting same error.

const config = {
headers: {
'Content-Type': mime.getType(fullBinaryPath),
'Content-MD5': testData.md5,
},
maxContentLength: 52428890,
ssl: true,
};

const fileBuffer = fse.readFileSync(fullBinaryPath);

axios.put(testData.uploadUrl, fileBuffer, config)
.then(resp => resolve(resp.status), (err) => {
return reject(err);
});

@amjadalibb

This comment has been minimized.

Copy link

commented May 25, 2018

Worked for me. I just changed the version in package.json

"axios": "0.18.0",

@roccomuso

This comment has been minimized.

Copy link

commented Jul 6, 2018

axios should default follow-redirect maxBodyLength limit to Infinity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.