Skip to content

Preserve embedded NUL bytes in TEXT values - #331

Open
sbking wants to merge 1 commit into
rhashimoto:masterfrom
sbking:fix/embedded-nul-text-round-trip
Open

Preserve embedded NUL bytes in TEXT values#331
sbking wants to merge 1 commit into
rhashimoto:masterfrom
sbking:fix/embedded-nul-text-round-trip

Conversation

@sbking

@sbking sbking commented Jul 16, 2026

Copy link
Copy Markdown

Summary

  • pass the encoded UTF-8 byte length to sqlite3_bind_text so embedded NUL bytes are not treated as a C-string terminator
  • read sqlite3_column_text with its sqlite3_column_bytes length and preserve embedded NULs without changing the existing Emscripten UTF-8 decoding behavior
  • add independent regression coverage for the bind and read paths

Testing

  • yarn test under Node 20 and Chrome: 2,993 passed, 0 failed
  • with the original implementation, both new tests fail across the storage/build matrix while the existing 2,114 assertions pass
  • node --check src/sqlite-api.js

References

Checklist

  • I grant to recipients of this Project distribution a perpetual,
    non-exclusive, royalty-free, irrevocable copyright license to reproduce, prepare
    derivative works of, publicly display, sublicense, and distribute this
    Contribution and such derivative works.
  • I certify that I am legally entitled to grant this license, and that this
    Contribution contains no content requiring a license from any third party.

@rhashimoto rhashimoto left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I have one requested change and one question inline.

Comment thread src/sqlite-api.js
verifyStatement(stmt);
const result = f(stmt, iCol);
return result;
const address = f(stmt, iCol);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

address can be 0 if the column is SQL NULL, in which case we should return null.

Comment thread src/sqlite-api.js
return result;
const address = f(stmt, iCol);
const nBytes = sqlite3.column_bytes(stmt, iCol);
const parts = [];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I understand this part. Why wouldn't we use TextDecoder on the whole buffer here?

@rhashimoto rhashimoto added the response requested Further information is requested label Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

response requested Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants