-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
doc: add note about timingSafeEqual for TypedArray #36323
Conversation
doc/api/crypto.md
Outdated
must have the same byte length. | ||
|
||
If at least one of `a` and `b` is a `TypedArray`, the result may depend on | ||
the platform byte order. |
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.
may depend
sounds like this isn’t deterministic, but I assume it is? It’s always going to depend on the platform byte order, right?
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.
Good point! My thinking was that the result of some inputs (e.g., timingSafeEqual(new Uint8Array([1, 1]), new Uint16Array([0x0101]))
) does not depend on the platform byte order, but that's probably not what others would take away from the way I worded it.
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.
Yeah, I see … maybe I would say something like If at least one of `a` and `b` is a `TypedArray` with more than one byte per entry, like `Uint16Array`, the result will be computed using platform byte order
?
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.
Sorry about the delay, fixed!
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.
LGTM with the elimination of may
from the text.
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.
LGTM with @addaleax's suggestion
PR-URL: #36323 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Landed in 6255973, thank you for reviewing! |
PR-URL: #36323 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: #36323 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Since #29657,
timingSafeEqual
usesbyteLength
instead oflength
. When comparing different types (e.g.,Uint8Array
andUint16Array
), the result can depend on the byte order of the system architecture.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes