Skip to content

Buffer slicing not working when passed to a browser XHR #46

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

Closed
konklone opened this issue Oct 13, 2014 · 8 comments
Closed

Buffer slicing not working when passed to a browser XHR #46

konklone opened this issue Oct 13, 2014 · 8 comments

Comments

@konklone
Copy link

I ran into an issue, documented in nathanpeck/s3-upload-stream#20, where a Buffer that had been sliced and then passed onto the browser's native XHR object wasn't having its sliced view respected.

It looks like you might have already addressed and documented this in 500be9e? If so, it might be worth rephrasing with less low-level terminology or something? I think I understand what "parent tracking" means, and that this is the issue I'm seeing, but I'm not sure.

@nathanpeck
Copy link

500be9e is referring to the Node.js behavior of slice where both the original buffer and the slice are pointing at the exact same memory region, so modifying one will also modify the other. Since the fallback uses a copy approach then modifying the copy will not modify the original, so @feross is warning that you can't rely on that.

It seems that the behavior that we were seeing is that the slice was not being respected when the sliced buffer was passed to the XHR, as the XHR was uploading the entire source buffer, rather than just the sliced view.

This obviously differs from the Node.js implementation where if you pass a sliced view of a buffer to a POST request is only uploads the sliced section, rather than the entire buffer.

@konklone
Copy link
Author

500be9e is referring to the Node.js behavior of slice where both the original buffer and the slice are pointing at the exact same memory region, so modifying one will also modify the other. Since the fallback uses a copy approach then modifying the copy will not modify the original, so @feross is warning that you can't rely on that.

Ah, okay I re-read it, you're right. That's sort of addressing the opposite situation, where you want to keep a reference to the parent through a slice, and the browser doesn't support that.

In our situation, the browser implementation (I tested on the latest version of Chrome and Firefox, on Ubuntu) of xhr.send(buffer) -- where buffer is a sliced view of a parent buffer -- does not respect the sliced view, and sends the entire parent buffer.

@feross
Copy link
Owner

feross commented Oct 14, 2014

@nathanpeck is right – the broken slice behavior that old browsers get could not be the cause of this issue.

The object returned from the Buffer constructor in the browser is a Uint8Array. Are you using XMLHttpRequest directly? A module you're using could be using the buffer property of the Uint8Array which will return the whole underlying ArrayBuffer, losing the slice information.

Otherwise, this sounds like a spec and/or browser bug.

@konklone
Copy link
Author

A module you're using could be using the buffer property of the Uint8Array which will return the whole underlying ArrayBuffer, losing the slice information.

This might well be it. From aws-sdk's xhr code:

if (httpRequest.body && typeof httpRequest.body.buffer === 'object') {
  xhr.send(httpRequest.body.buffer); // typed arrays sent as ArrayBuffer
} else {
  xhr.send(httpRequest.body);
}

The whole point of this block is to make sure to call the buffer property if it exists. Will xhr.send() not work with a straight Uint8Array?

@feross
Copy link
Owner

feross commented Oct 14, 2014

xhr.send used to only work on ArrayBuffer objects, but not TypedArray objects. That has since changed. Now passing ArrayBuffer is deprecated and TypedArray is preferred. Sigh.

See: http://updates.html5rocks.com/2012/07/Arrived-xhr-send-ArrayBufferViews

@konklone
Copy link
Author

OK, so it seems clear now that the issue is not in the buffer module, so I'm closing this.

Am I correct in thinking that what I should do next is send an issue to aws-sdk, linking to that post, and suggesting they simply axe the conditional block and always do xhr.send(httpRequest.body)? Is there an effective means for them to test if the client is running in a browser that will not know what to do with a TypedArray?

@feross
Copy link
Owner

feross commented Oct 14, 2014

That sounds like a good plan to me. Feel free to reference the issue you open here, for posterity.

I suspect that trying to send an object that send can't handle will throw an exception that can be caught and handled, but that should be verified.

@konklone
Copy link
Author

This is being addressed in aws/aws-sdk-js#391, for those interested.

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

No branches or pull requests

3 participants