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

Bug with unpack chaining #12

Open
aleclarson opened this issue Sep 3, 2021 · 4 comments
Open

Bug with unpack chaining #12

aleclarson opened this issue Sep 3, 2021 · 4 comments

Comments

@aleclarson
Copy link

The issue is with this line here:

https://github.com/Simonwep/nason/blob/951aa9a4d809fa378d341211fb9951f6b6d1e442/src/convert.ts#L29

It's using .buffer which returns the original buffer, but uses the wrong byte offset.

I tried changing it to new Uint8Array(val.buffer, val.byteOffset + 1) but that still had a bug. For example, if decoding a string, the decoded string would have a leading space for some reason. (eg: "foo" would be " foo").

Next, I tried doing val.slice(1) instead, and that works as expected. Is using slice worse for performance?

@aleclarson
Copy link
Author

For context, here's what I mean by "unpack chaining"

https://github.com/alloc/ople/blob/99ccaa09a9e8aa0e02f156022719c6ee59d5f5e0/packages/nason/src/batch.ts#L40-L48

It's unpacking a series of encoded function calls, and val.buffer refers to the entire buffer, so the byte offset needs to stay relative to the specific call being decoded.

@simonwep
Copy link
Owner

simonwep commented Sep 4, 2021

Hi! Since you already spend a good amount of time figuring out what might be the problem, would you be okay with submitting a PR with a test that fails? It'd go into /test/custom-encoder.spec.ts... I'd take a look at it then :)

@aleclarson
Copy link
Author

Possibly. I'm using a fork that uses the val.slice(1) workaround, so it's not a big priority for me right now. ^^

@aleclarson
Copy link
Author

aleclarson commented Sep 4, 2021

Writing an encoder that supports nested Map would probably reproduce the issue, but don't quote me on that

Basically, it happens when unpacking in a loop and interweaving nested decode calls between those unpack calls.

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

No branches or pull requests

2 participants