From 0ad6c824b9f9602ca35280be4b16831fdcd1c33b Mon Sep 17 00:00:00 2001 From: sepehr-safari Date: Mon, 21 Sep 2026 17:22:09 +0300 Subject: [PATCH] fix: escape control bytes the way the rest of the network escapes them 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. --- src/event.zig | 86 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 7 deletions(-) diff --git a/src/event.zig b/src/event.zig index d86248c..6ebabff 100644 --- a/src/event.zig +++ b/src/event.zig @@ -24,12 +24,26 @@ pub const Event = struct { sig: [64]u8, }; -/// Escapes a string per the NIP-01 id-serialization rule: only `\n \" \\ \r -/// \t \b \f` are escaped; every other byte (including other control -/// characters and raw UTF-8) is copied verbatim. This is deliberately -/// stricter than general-purpose JSON escaping, which the spec forbids — -/// using a generic JSON encoder here would produce a different byte -/// sequence, and therefore a different id, than other implementations. +/// Escapes a string for the id serialization and for the wire JSON, which +/// want the same bytes. +/// +/// NIP-01 names seven escapes (`\n \" \\ \r \t \b \f`) and says all other +/// characters must be included verbatim. Read literally that leaves a byte +/// below 0x20 sitting raw inside a JSON string, and RFC 8259 forbids exactly +/// that, so the wire form would not be JSON and no relay could parse it. +/// +/// The sentence also does not describe what the network does. 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 instead of the +/// implementations produces an id nothing else reproduces, which turns a +/// correctly signed event from any JS or Go client into a bad signature here +/// and makes our own ids unverifiable everywhere else. +/// +/// So the remaining control bytes are escaped as `\u00XX`. Everything from +/// 0x20 up, raw UTF-8 included, is still copied verbatim, which is where the +/// warning against a general-purpose encoder still holds: escaping non-ASCII +/// would change the id. fn appendJsonString(list: *std.ArrayList(u8), allocator: std.mem.Allocator, s: []const u8) std.mem.Allocator.Error!void { try list.append(allocator, '"'); for (s) |c| { @@ -41,7 +55,13 @@ fn appendJsonString(list: *std.ArrayList(u8), allocator: std.mem.Allocator, s: [ '\t' => try list.appendSlice(allocator, "\\t"), 0x08 => try list.appendSlice(allocator, "\\b"), 0x0C => try list.appendSlice(allocator, "\\f"), - else => try list.append(allocator, c), + else => |b| { + if (b < 0x20) { + try list.print(allocator, "\\u{x:0>4}", .{b}); + } else { + try list.append(allocator, b); + } + }, } } try list.append(allocator, '"'); @@ -282,6 +302,58 @@ test "canonical serialization: empty tags, mixed escapes" { try std.testing.expectEqualSlices(u8, &expected_id, &id); } +test "control bytes escape the way the rest of the network escapes them" { + const allocator = std.testing.allocator; + const pubkey = try hexToBytes32("3bf0c63fcb93463407af97a5e5ee64fa883d107ef9e558472c4eb9aaaefa459d"); + const tags = [_]Tag{&[_][]const u8{ "e", "a\x02b" }}; + const content = "a\x01b\x1bc"; + + // These bytes are what `JSON.stringify` produces, which is how nostr-tools + // builds the preimage and what go-nostr's `escapeString` writes by hand. + // Derived from an independent implementation rather than from this code, so + // it fails if this file ever drifts back to the spec's literal wording. + const serialized = try serializeCanonical(allocator, pubkey, 1700000000, 1, &tags, content); + defer allocator.free(serialized); + const expected = "[0,\"3bf0c63fcb93463407af97a5e5ee64fa883d107ef9e558472c4eb9aaaefa459d\",1700000000,1,[[\"e\",\"a\\u0002b\"]],\"a\\u0001b\\u001bc\"]"; + try std.testing.expectEqualStrings(expected, serialized); + + const id = try computeId(allocator, pubkey, 1700000000, 1, &tags, content); + const expected_id = try hexToBytes32("d70263901ff294ef8559ce5626d7c3b216877255385fb23ba00c4b60d583fd00"); + try std.testing.expectEqualSlices(u8, &expected_id, &id); +} + +test "an event carrying control bytes survives its own wire format" { + const allocator = std.testing.allocator; + const pubkey = try hexToBytes32("3bf0c63fcb93463407af97a5e5ee64fa883d107ef9e558472c4eb9aaaefa459d"); + const tags = [_]Tag{&[_][]const u8{ "e", "a\x02b" }}; + const content = "a\x01b\x1bc"; + + const id = try computeId(allocator, pubkey, 1700000000, 1, &tags, content); + const event = Event{ + .id = id, + .pubkey = pubkey, + .created_at = 1700000000, + .kind = 1, + .tags = &tags, + .content = content, + .sig = [_]u8{0xab} ** 64, + }; + + const json_text = try toJson(allocator, event); + defer allocator.free(json_text); + + // The point of the test: the wire form used to carry the control bytes raw, + // which is not JSON, so this parse failed on our own output. + try std.testing.expect(std.mem.indexOfScalar(u8, json_text, 0x01) == null); + try std.testing.expect(std.mem.indexOfScalar(u8, json_text, 0x1b) == null); + + var parsed = try fromJson(allocator, json_text); + defer parsed.deinit(); + try std.testing.expectEqualStrings(content, parsed.value.content); + try std.testing.expectEqualStrings("a\x02b", parsed.value.tags[0][1]); + try std.testing.expectEqualSlices(u8, &event.id, &parsed.value.id); +} + test "toJson / fromJson round trip" { const allocator = std.testing.allocator; const pubkey = try hexToBytes32("f7234bd4c1394dda46d09f35bd384dd30cc552ad5541990f98844fb06676e9ca");