Escape control bytes the way the rest of the network escapes them - #103
Merged
sepehr-safari merged 1 commit intoSep 21, 2026
Merged
Conversation
Two faults, one line of code between them. The wire form was not JSON. `toJson` escapes content and tag fields with the id escaper, which handles the seven characters NIP-01 names and copies every other byte through untouched. RFC 8259 forbids a raw byte below 0x20 inside a JSON string, so an event whose content carried one was a byte sequence no JSON parser would accept. Our own parser included: `fromJson(toJson(ev))` failed on our own output, and any relay would have rejected the event on the wire. The id did not match the network's. NIP-01 says the seven escapes are the only ones and all other characters go in verbatim, and this file followed that sentence deliberately. The implementations do not. nostr-tools builds the preimage with `JSON.stringify` and go-nostr writes the same behaviour by hand in `escapeString`, so both escape every remaining control byte as `\u00XX`. Following the sentence produced an id nothing else reproduces: a correctly signed event from any JS or Go client read as a bad signature here, and an id computed here was unverifiable everywhere else. Escaping the remaining control bytes as `\u00XX` fixes both, because the id form and the wire form want the same bytes. Everything from 0x20 up, raw UTF-8 included, is still copied verbatim, so the warning against reaching for a general-purpose encoder still holds: escaping non-ASCII would change the id. This changes the id computed for content or tags containing a control byte. Nothing else moves, and the existing vectors are unaffected because none of them carry one. The new vector is derived from an independent implementation rather than from this code, so it fails if the escaping ever drifts back. The round-trip test asserts the wire form carries no raw control byte at all, which is the thing that used to make it unparseable.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #102.
Two faults, one line of code between them.
The wire form was not JSON.
toJsonescapes content and tag fields with the id escaper, which handles the seven characters NIP-01 names and copies every other byte through untouched. RFC 8259 forbids a raw byte below 0x20 inside a JSON string, so an event whose content carried one was a byte sequence no JSON parser would accept. Ours included:fromJson(toJson(ev))failed on our own output, and a relay would have rejected the event on the wire.The id did not match the network. NIP-01 says the seven escapes are the only ones and all other characters go in verbatim, and this file followed that sentence deliberately. The implementations do not. nostr-tools builds the preimage with
JSON.stringify; go-nostr writes the same behaviour by hand inescapeString. Both escape every remaining control byte as\u00XX. Following the sentence produced an id nothing else reproduces, so a correctly signed event from any JS or Go client read as a bad signature here, and an id computed here was unverifiable everywhere else.Escaping the remaining control bytes as
\u00XXfixes both, because the id form and the wire form want the same bytes. Everything from 0x20 up, raw UTF-8 included, is still copied verbatim, so the warning against reaching for a general-purpose encoder still holds: escaping non-ASCII would change the id.What moves
The id computed for content or tags containing a control byte. Nothing else. The existing vectors are untouched because none of them carry one, and all 203 tests pass.
Tests
control bytes escape the way the rest of the network escapes thempins the serialization and the id against a vector derived from an independent implementation rather than from this code, so it fails if the escaping drifts back.an event carrying control bytes survives its own wire formatasserts the wire form contains no raw control byte and thatfromJsonaccepts it. That parse is what used to fail.Note for the reader
This reads against the literal wording of NIP-01, which is why the doc comment now says so at length. The choice is interoperability over the sentence: an id only means something if the rest of the network computes the same one.