-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[core-rest-pipeline] Fix handling of typed arrays in request bodies #15904
Conversation
NodeJS does not support directly passing typed arrays or ArrayBuffers to `http.ClientRequest` streams. This means we must correctly wrap these types in a `Buffer` for them to be serialized correctly.
} else if (isArrayBuffer(body)) { | ||
req.end(ArrayBuffer.isView(body) ? Buffer.from(body.buffer) : Buffer.from(body)); | ||
} else { | ||
logger.error("Unrecognized body type", body); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically a "never" situation since:
FormData
would be handled by theisReadableStream
case aboveBlob
is browser only
There are no other supported body types after that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is FormData handled by isReadableStream? It doesn't have pipe
function. also I had thought it's browser only (go to definition opens lib.dom.d.ts) and I didn't find it in nodejs docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FormData
(the type you saw) gets converted by this policy when in Node:
request.body = requestForm; |
It uses an npm package form-data
to create another object type (confusingly also called FormData
) that implements the Stream interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix! I did not know about typed arrays until now, found this to be a useful intro: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray
As you might expect, they're useful for code that is performance-critical since they should have compact memory representation and operations on them should be fast.
delete python track1 config (Azure#15904)
delete python track1 config (Azure#15904)
delete python track1 config (Azure#15904)
NodeJS does not support directly passing typed arrays or ArrayBuffers to
http.ClientRequest
streams. This means we must correctly wrap these types in aBuffer
for them to be serialized correctly.