-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Cast string bodies to buffers to ensure correct encoding used #1303
Conversation
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.
Just one change requested to protect against the hard removal of using the Buffer constructor in later versions of node.js
@@ -100,6 +100,9 @@ AWS.NodeHttpClient = AWS.util.inherit({ | |||
total: totalBytes | |||
}); | |||
}); | |||
if (typeof body === 'string') { | |||
body = new Buffer(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.
Since the Buffer constructors are deprecated, we might want to add some feature detection to try Buffer.from
first, then new Buffer
.
Also, can you add a changelog entry using the |
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.
1 similar comment
@@ -100,6 +100,10 @@ AWS.NodeHttpClient = AWS.util.inherit({ | |||
total: totalBytes | |||
}); | |||
}); | |||
if (typeof body === 'string') { | |||
body = typeof Buffer.from === 'function' ? |
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 unfortunately is broken on Node 4.4.5 :-( Buffer.from
is a function but fails (because it is actually Uint8Array.from
)
root@45d8cedbf66f:/app# node --version
v4.4.5
root@45d8cedbf66f:/app# node
> Buffer.from === Uint8Array.from
true
> typeof Buffer.from
'function'
> Buffer.from("foo")
TypeError: this is not a typed array.
at Function.from (native)
...
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
This change ensures that bodies are always converted to byte buffers before being passed to an
http.ClientRequest
. It looks like the code that packs headers and short bodies into the same outgoing HTTP packet has a slightly different codepath for non-binary strings, which this change lets the SDK avoid.Resolves #1297.