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

accept and rollback error message should be the same as c hooks #325

Open
dangell7 opened this issue May 29, 2024 · 1 comment
Open

accept and rollback error message should be the same as c hooks #325

dangell7 opened this issue May 29, 2024 · 1 comment
Labels

Comments

@dangell7
Copy link
Collaborator

When doing accept or rollback in jshooks you cannot send in data and have it return the hex value. If you send in a hex, it will hex that again.

Current Behavior:

accept('DEADBEEF', 18)
// "HookReturnString": "4445414442454546"
accept(SBUF("DEADBEEF"), 18)
// "HookReturnString": "4445414442454546"
let acc = hook_account()
accept(acc, 18)
// "HookReturnString": ""
uint8_t hook_acc[20];
hook_account(SBUF(hook_acc));
accept(SBUF(hook_acc), 18)
// "HookReturnString": "6DF90ECA2B5C0B45AA2E1D6E993BF7D9DCB226B3"
let acc = hook_account()
accept(toHex(acc), 18)
// "HookReturnString": "36444639304543413242354330423435414132453144364539393342463744394443423232364233"

Conclusion:

I think we should allow the ability to send in the buffer/byte array and have it return the hex of that. To match the behavior of the c hooks.

If I update the macro with the following then I can send the byte array into the accept, with as_hex=true and then get the account id as the HookReturnString.

if (JS_IsBool(as_hex) && !!JS_ToBool(ctx, as_hex))\
    {\
        auto in = FromJSIntArrayOrHexString(ctx, error_msg, 64*1024);\
        if (in.has_value())\
        {\
            if (in->size() > 1024)\
                in->resize(1024);\
            hookCtx.result.exitReason = std::string((const char*)in->data(), in->size());\
        }\
    }\

There is also a function is_UTF16LE in the c path and I think we should decide if we want to incorporate this as well.

@dangell7 dangell7 added Med Medium Priority Bug (should be fixed within next few releases) JSHooks and removed Med Medium Priority Bug (should be fixed within next few releases) labels May 29, 2024
@RichardAH
Copy link
Contributor

This seems reasonable, feel free to make the change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants