-
Notifications
You must be signed in to change notification settings - Fork 461
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
Implement data manipulation methods for dataview #218
Conversation
@mhdawson Could you please check this patch? |
napi-inl.h
Outdated
|
||
template <typename T> | ||
inline T DataView::ReadData(size_t byteOffset) const { | ||
if (byteOffset + sizeof(T) > _length) { |
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.
Looks like this could overflow and let somebody read up to 7 bytes before _data
. I'm not sure how an attacker could do anything useful with that, but it might be surprising.
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. I've addressed your comment.
Thanks.
This change implements additional methods for manipulating data in the dataview object. This change includes the following things: - Get/Set${type} methods to manipulate data - Tests for the methods
2bc2e68
to
3afe0fa
Compare
@romandev just back from being out for a week and still catching up. Will try and review later this week. |
@mhdawson Gentle ping. |
@romandev thanks for the reminder, have been pretty busy and at a conference this week but will try to find time to take a look. |
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
This change implements additional methods for manipulating data in the dataview object. This change includes the following things: - Get/Set${type} methods to manipulate data - Tests for the methods PR-URL: #218 Reviewed-By: Michael Dawson <[email protected]>
Landed as 9c4d321 |
This change implements additional methods for manipulating data in the dataview object. This change includes the following things: - Get/Set${type} methods to manipulate data - Tests for the methods PR-URL: nodejs/node-addon-api#218 Reviewed-By: Michael Dawson <[email protected]>
This change implements additional methods for manipulating data in the dataview object. This change includes the following things: - Get/Set${type} methods to manipulate data - Tests for the methods PR-URL: nodejs/node-addon-api#218 Reviewed-By: Michael Dawson <[email protected]>
This change implements additional methods for manipulating data in the dataview object. This change includes the following things: - Get/Set${type} methods to manipulate data - Tests for the methods PR-URL: nodejs/node-addon-api#218 Reviewed-By: Michael Dawson <[email protected]>
This change implements additional methods for manipulating data in the dataview object. This change includes the following things: - Get/Set${type} methods to manipulate data - Tests for the methods PR-URL: nodejs/node-addon-api#218 Reviewed-By: Michael Dawson <[email protected]>
This change implements additional methods for manipulating data in the
dataview object. This change includes the following things: