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

Fixing maxBodyLength enforcement #3786

Merged
merged 3 commits into from Dec 23, 2021

Conversation

PauloRSF
Copy link
Contributor

@PauloRSF PauloRSF commented May 7, 2021

The maxBodyLength option was only being enforced by follow-redirects, so, the "body length exceeded" error was only thrown when maxRedirects was set to 0. I added a body length validation before creating the request.

Closes #3769.

 * Removed due to the error being thrown by axios itself now, instead of follow-redirects
@PauloRSF PauloRSF changed the title Max body length fix Fixing maxBodyLength enforcement May 7, 2021
@jasonsaayman jasonsaayman merged commit c00c4dd into axios:master Dec 23, 2021
@misos1
Copy link

misos1 commented Dec 23, 2021

But what if the body comes from a stream and not a buffer like in this example?

let { PassThrough } = require("stream");
let s = new PassThrough();
s.end(Buffer.alloc(1000));
require("axios")
	.put("http://www.example.com", s, { maxRedirects: 0, maxBodyLength: 100 })
	.catch(e => console.log("axios returned error:", e));

This is how it is in follow-redirects:

  // Only write when we don't exceed the maximum body length
  if (this._requestBodyLength + data.length <= this._options.maxBodyLength) {
    this._requestBodyLength += data.length;
    this._requestBodyBuffers.push({ data: data, encoding: encoding });
    this._currentRequest.write(data, encoding, callback);
  }
  // Error when we exceed the maximum body length
  else {
    this.emit("error", new MaxBodyLengthExceededError());
    this.abort();
  }

https://github.com/follow-redirects/follow-redirects/blob/6f5029ae1a0fdab4dc25f6379a5ee303c2319070/index.js#L94-L104

mbargiel pushed a commit to mbargiel/axios that referenced this pull request Jan 27, 2022
* Adding request body length validation on HTTP adapter

* Removing error code assertion on HTTP's body length support test

 * Removed due to the error being thrown by axios itself now, instead of follow-redirects

Co-authored-by: Jay <jasonsaayman@gmail.com>
@amitzur
Copy link

amitzur commented Feb 16, 2022

Hi 👋
This is listed as a breaking change here: https://github.com/axios/axios/releases
Could you explain why this is considered a breaking change?

@misos1
Copy link

misos1 commented Feb 16, 2022

This "fix" is not complete. It will not work for streams as I wrote earlier (follow-redirects works).

@MrXyfir
Copy link

MrXyfir commented Apr 7, 2022

I'm guessing this is related to my issue... I recently upgraded from 0.24 to 0.26 and my file upload via FormData broke. Reverting solved the issue. I don't have time to chase down the issue more in-depth right now but in case anyone is curious.

export async function importAsset(uri: string, mime: string) {
  const formData = new FormData();
  const isImage = mime.startsWith('image/');

  formData.append('file', {
    name: uri.split('/').pop(),
    type: mime,
    uri,
  });
  formData.append('upload_preset', 'assets');

  return axios.post(
    `https://api.cloudinary.com/v1_1/abc123/${isImage ? 'image' : 'video'}/upload`,
    formData,
  );
}

@timoisalive
Copy link

Yeah, our file upload broke as well (on Android only). Would be nice to know what in our end might need to be changed in order to upgrade to this version.

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.

maxBodyLength does not work with disabled redirects
6 participants