Skip to content
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

napi_check_object_type_tag returns false if upper in napi_type_tag is 0 #43786

Closed
kjvalencik opened this issue Jul 11, 2022 · 2 comments
Closed
Labels
node-api Issues and PRs related to the Node-API.

Comments

@kjvalencik
Copy link

kjvalencik commented Jul 11, 2022

Version

All version supporting Node-API 8+

Platform

All

Subsystem

Node-API

What steps will reproduce the bug?

Create a type tag with an upper value of 0.

static const napi_type_tag MyTypeTag = {
  0xa5ed9ce2e4c00c38, 0x0
};

Tag an object with it:

napi_type_tag_object(env, obj, &MyTypeTag);

Check the type tag:

bool is_my_type;

napi_check_object_type_tag(env, obj, &MyTypeTag, &is_my_type);

How often does it reproduce? Is there a required condition?

100% of the time

What is the expected behavior?

napi_check_object_type_tag will set is_my_type to true.

What do you see instead?

is_my_type remains false.

Additional information

Type tags are stored in a BigInt. If upper is 0, then the leading zero gets truncated and the length is 1. However, the follow check expects the length to always be 2.

if (size == 2 && sign == 0)

This could could be something like:

    napi_type_tag tag = { 0, 0 };
    
    /* ... */

    if (size <= 2 && sign == 0)
      *result = (tag.lower == type_tag->lower && tag.upper == type_tag->upper);
@daeyeon daeyeon added the node-api Issues and PRs related to the Node-API. label Jul 12, 2022
@daeyeon
Copy link
Member

daeyeon commented Jul 12, 2022

Create a type tag with an upper value of 0.

static const napi_type_tag MyTypeTag = {
  0x0, 0xa5ed9ce2e4c00c38
};

I guess you meant 0xa5ed9ce2e4c00c38, 0x0 since the first one is for a lower value.

typedef struct {
uint64_t lower;
uint64_t upper;
} napi_type_tag;

@kjvalencik
Copy link
Author

kjvalencik commented Jul 12, 2022

@daeyeon Good catch; that's correct. I was transcribing from Rust and my test code used named fields.

danielleadams pushed a commit that referenced this issue Jul 26, 2022
This fixes a comparison failure occurring when the upper value of a
type tag is 0, or a type tag value is 0.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43788
Fixes: #43786
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
targos pushed a commit that referenced this issue Jul 31, 2022
This fixes a comparison failure occurring when the upper value of a
type tag is 0, or a type tag value is 0.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43788
Fixes: #43786
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
This fixes a comparison failure occurring when the upper value of a
type tag is 0, or a type tag value is 0.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs/node#43788
Fixes: nodejs/node#43786
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants