diff --git a/.gitkeep b/.gitkeep index 29a8cf9..c1a5f09 100644 --- a/.gitkeep +++ b/.gitkeep @@ -2,4 +2,5 @@ # Updated: 2026-08-20T05:25:16.696Z # Updated: 2026-08-20T06:10:07.182Z # Updated: 2026-08-20T07:45:09.136Z -# Updated: 2026-08-27T11:41:42.189Z \ No newline at end of file +# Updated: 2026-08-27T11:41:42.189Z +# Updated: 2026-08-27T14:37:49.393Z \ No newline at end of file diff --git a/README.md b/README.md index c299941..a149960 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ All implementations share the same design philosophy and provide feature parity. - **Readable by Default**: In every language `encode()` writes indented, plain-text Links Notation; the previous single-line base64 form stays available as `encode_compact()` (alias `encode_obfuscated()`) - **One Record per Line**: `encode_line()` writes the same readable document on one line and `decode_line()` reads it back exactly, so an append-only log stays greppable, tailable and countable by `wc -l` - **Object Identity**: Shared references and circular references are preserved by the compact format via object ids; the readable format is a plain tree and raises a circular-reference error instead -- **Full Unicode**: Strings are written as text; only a value that cannot be written as text (one holding control characters) is base64-encoded, and it is marked individually as `(base64 "…")` +- **Full Unicode**: Strings are always written as text — a newline stays a newline, a tab stays a tab, and every word stays greppable; only the characters a form cannot carry are percent-escaped, in a value marked individually as `(escaped "…")` - **Opt-in Tracing**: Set `LINO_CODEC_DEBUG=1` to trace encoding and decoding, the same way in every language - **Simple API**: Easy-to-use `encode()` and `decode()` functions - **JSON/Lino Conversion**: Convert between JSON and Links Notation (JavaScript) @@ -420,8 +420,17 @@ object, bare-value lines make an array: `null` are bare, so types survive a round trip - `NaN`, `Infinity` and `-Infinity` are written as such - An empty array is `()`; an empty object is `(` + newline + `)` -- Only a value that cannot be written as text (one containing control characters) - is base64-encoded, and it is marked individually as `(base64 "bGluZTEKbGluZTI=")` +- A string is written as text whatever it holds: a newline stays a newline and + a tab stays a tab, so every word stays greppable +- A string containing the quote delimiter is written between a run of at least + three of them — `"""say "hi""""` — which the notation's own parser reads back + unchanged, rather than by doubling the quote +- Only the characters this form cannot carry — a carriage return, which CRLF + normalisation would rewrite, and the remaining control characters — are + percent-escaped, in a value marked individually as `(escaped "first%0D")`. + `(base64 "…")` written by versions up to 0.6.0 is still decoded +- A value that occurs more than once is written out every time: a shared + reference would make one record depend on another - The four languages produce byte-identical output, checked by the shared fixtures in [`fixtures/readable-format/cases.json`](fixtures/readable-format/cases.json) @@ -438,7 +447,9 @@ record per line — appending is one write, compaction cuts at a newline, and - An object is `(o: (key value) …)` and an empty object is `(o:)` - An array is `(value …)` and an empty array is `()` - Scalars and strings are written exactly as in the indented form, so a string - keeps its own characters and a number keeps its type + keeps its own characters and a number keeps its type — except that a newline + would end the record, so on this form the newline, and nothing else, is + escaped: `(escaped "line one%0Aline two")` keeps both lines readable - The `o` marker is what removes the ambiguity a flat layout otherwise has: without it `((key value))` reads both as a one-pair object and as an array holding a two-element array. With it a bare `( )` on one line is always an @@ -454,7 +465,8 @@ The previous single-line form, kept for compatibility and for the object graphs the readable tree cannot express (shared and circular references): - Basic types are encoded with type markers: `(int 42)`, `(str aGVsbG8=)`, `(bool true)` -- Strings are base64-encoded to handle special characters and newlines +- Strings are base64-encoded here, and only here: this is the one form that + asks for it by name, and `encode()` never reaches for it - Collections with self-references use `(obj_id: type content...)`, e.g. `(obj_0: dict ((str c2VsZg==) obj_0))` for `{"self": obj}` - Circular references use direct object id references: `obj_0` (without a `ref` keyword) diff --git a/csharp/.changeset/20260827_120000_issue_45_plain_text_values.md b/csharp/.changeset/20260827_120000_issue_45_plain_text_values.md new file mode 100644 index 0000000..f52cc39 --- /dev/null +++ b/csharp/.changeset/20260827_120000_issue_45_plain_text_values.md @@ -0,0 +1,26 @@ +--- +'Lino.Objects.Codec': minor +--- + +`Codec.Encode` and `Codec.EncodeLine` never reach for base64. A single control +character used to turn a whole string into base64, so a log message holding one +newline hid its own text: the message, the stack trace and every word a reader +would grep for. Both readable forms now write the text as it is and escape only +what the form itself cannot carry — the newline on a single line, the carriage +return everywhere, and the remaining control characters — in a value marked +`(escaped "line one%0Aline two")` whose payload is percent-escaped, so even the +escaped part stays readable. base64 is reachable only through +`Codec.EncodeCompact` / `Codec.EncodeObfuscated`, which say so by name. + +A string containing the quote delimiter is written between a run of at least +three of them — `"""say "hi""""` — instead of by doubling the quote, which +desynchronises the notation's own parser. `Readable.EscapedMarker` names the new +`escaped` link id. + +Fixes a key holding a control character, which used to be written as +`(base64 "…")` in key position and read back as a list element, so +`{"a\nb": "a\nb"}` decoded to `["a\nb", "a\nb"]`. `(base64 "…")` is still +decoded, so every document written by an earlier version keeps reading; the +shared fixtures pin this in a `legacy` section. + +See [issue #45](https://github.com/link-foundation/lino-objects-codec/issues/45). diff --git a/csharp/README.md b/csharp/README.md index 1c007d2..7147da1 100644 --- a/csharp/README.md +++ b/csharp/README.md @@ -17,7 +17,7 @@ A C# library for working with Links Notation format. This library provides unive - **Readable by Default**: `Codec.Encode()` writes plain, indented text that can be read and reviewed - **One Record per Line**: `Codec.EncodeLine()` writes the same document on one line and `Codec.DecodeLine()` reads it back exactly, so an append-only log stays greppable, tailable and countable by `wc -l` - **Object Identity**: Shared references and circular references are preserved by the compact format (`Codec.EncodeCompact`) via object ids -- **Full Unicode**: Strings are written as text; only a value that cannot be written as text (one holding control characters) is base64-encoded, and it is marked individually as `(base64 "…")` +- **Full Unicode**: Strings are always written as text — a newline stays a newline and a tab stays a tab, so every word stays greppable; only the characters a form cannot carry are percent-escaped, in a value marked individually as `(escaped "…")` - **Opt-in Tracing**: Set `LINO_CODEC_DEBUG=1` to trace encoding and decoding, the same way in every language - **Simple API**: Easy-to-use `Codec.Encode()` and `Codec.Decode()` functions - **Thread Safe**: Each operation uses a fresh codec instance @@ -215,9 +215,16 @@ bare-value lines make a list: - Numbers, `true`, `false` and `null` are bare, so types survive a round trip - `NaN`, `Infinity` and `-Infinity` are written as such - An empty list is `()`; an empty dictionary is `(` + newline + `)` -- A value that cannot be written as text (one containing control characters) is - base64-encoded on its own and marked as `(base64 "bGluZTEKbGluZTI=")`; - everything around it stays readable +- A string is written as text whatever it holds: a newline stays a newline and + a tab stays a tab, so every word stays greppable +- A string containing the quote delimiter is written between a run of at least + three of them — `"""say "hi""""` — rather than by doubling the quote +- Only the characters this form cannot carry — a carriage return and the + remaining control characters — are percent-escaped, in a value marked on its + own as `(escaped "first%0D")`; everything around it stays readable, and + `(base64 "…")` written by earlier versions is still decoded +- A value that occurs more than once is written out every time: a shared + reference would make one record depend on another ### Single-line format (`Codec.EncodeLine`) @@ -244,7 +251,8 @@ The previous single-line form, kept for compatibility and for the object graphs the readable tree cannot express (shared and circular references): - Basic types carry a type marker: `(int 42)`, `(str SGVsbG8=)`, `(bool true)` -- Strings are base64-encoded to handle special characters and newlines +- Strings are base64-encoded here, and only here: this is the one form that + asks for it by name, and `encode()` never reaches for it - Collections with self-references use `(obj_id: type content...)`, e.g. `(obj_0: dict ((str c2VsZg==) obj_0))` for `{"self": obj}` - Circular references use direct object ID references: `obj_0` (without a `ref` keyword) diff --git a/csharp/src/Lino.Objects.Codec/Readable.cs b/csharp/src/Lino.Objects.Codec/Readable.cs index 01958c5..322e28c 100644 --- a/csharp/src/Lino.Objects.Codec/Readable.cs +++ b/csharp/src/Lino.Objects.Codec/Readable.cs @@ -70,10 +70,16 @@ public CircularReferenceException(string message, Exception innerException) /// while an empty object is written as ( and ) on two lines. /// /// -/// Only values that cannot be written as plain text are encoded: strings holding -/// control characters (including newlines and tabs, which line-based tooling and -/// CRLF normalisation would corrupt) are marked individually as -/// (base64 "…") instead of encoding the whole document. +/// Text is written as text. A string keeps every character a reader would grep +/// for, including newlines and tabs, and is quoted with a run of delimiters — +/// """say "hi"""" — when it holds the delimiter itself. Only the +/// characters a form cannot carry are escaped, and only they: the value is then +/// written as (escaped "…"), where %XX stands for one escaped +/// byte. The indented form escapes the carriage return, which CRLF normalisation +/// would otherwise rewrite, and the other control characters; the single-line +/// form escapes the newline as well, because there a record ends at the end of +/// the line. Nothing else is encoded: base64 lives in +/// , which a caller asks for by name. /// /// /// writes the same document on one line, so one record @@ -105,9 +111,19 @@ public static class Readable /// Default indentation used by . public const string DefaultIndent = " "; - /// Marker used for values that cannot be represented as plain text. + /// Marker of a string written as base64 by versions up to 0.6.0, still read. public const string Base64Marker = "base64"; + /// + /// Marker of a string whose unwritable characters are percent-escaped. + /// + /// + /// It reads as (escaped "line one%0Aline two"). Only those characters + /// change; the rest of the text is written as it is, so the value stays + /// readable and greppable. + /// + public const string EscapedMarker = "escaped"; + /// Link id naming an object in the single-line form, written as (o: …). public const string ObjectMarker = "o"; @@ -117,6 +133,19 @@ public static class Readable /// Characters that force an object key to be quoted. private static readonly char[] KeyNeedsQuotes = { '(', ')', '\'', '"', ':', '`' }; + /// Reads an escaped payload back, rejecting invalid UTF-8. + private static readonly UTF8Encoding StrictUtf8 = new(false, true); + + /// Which readable form is being written, which is what says how much has to be escaped. + private enum Form + { + /// The indented form, where a value may hold a line break of its own. + Indented, + + /// The single-line form, where a record ends at the end of the line. + Line, + } + /// /// Encode a value into the readable, indented Links Notation form. /// @@ -225,7 +254,7 @@ private static void WriteValue(object? value, string indent, int level, StringBu { output.Append('\n'); PushIndent(indent, level + 1, output); - output.Append(FormatKey(pair.Key)); + output.Append(FormatKey(pair.Key, Form.Indented)); output.Append(' '); WriteValue(pair.Value, indent, level + 1, output, path); } @@ -262,7 +291,7 @@ private static void WriteValue(object? value, string indent, int level, StringBu return; } - output.Append(FormatScalar(value)); + output.Append(FormatScalar(value, Form.Indented)); } /// @@ -285,7 +314,7 @@ private static void WriteLineValue(object? value, StringBuilder output, HashSet< output.Append('(').Append(ObjectMarker).Append(':'); foreach (var pair in dict) { - output.Append(" (").Append(FormatKey(pair.Key)).Append(' '); + output.Append(" (").Append(FormatKey(pair.Key, Form.Line)).Append(' '); WriteLineValue(pair.Value, output, path); output.Append(')'); } @@ -314,7 +343,7 @@ private static void WriteLineValue(object? value, StringBuilder output, HashSet< return; } - output.Append(FormatScalar(value)); + output.Append(FormatScalar(value, Form.Line)); } /// @@ -344,14 +373,14 @@ private static void PushIndent(string indent, int level, StringBuilder output) /// Format a scalar value. Strings are quoted, everything else stays bare so /// that its type is recoverable when reading the document back. /// - private static string FormatScalar(object? value) + private static string FormatScalar(object? value, Form form) { return value switch { null => "null", // Written in lower case in every language, so the output is identical. bool b => b ? "true" : "false", - string s => FormatString(s), + string s => FormatString(s, form), sbyte or byte or short or ushort or int or uint or long or ulong => Convert.ToString(value, CultureInfo.InvariantCulture) ?? "null", float f => FormatFloat(f), @@ -387,36 +416,95 @@ private static string FormatFloat(double value) } /// - /// Format a string value: quoted plain text, or an individually marked - /// base64 payload when the text cannot be written literally. + /// Format a string value. The text is written as text; when it holds + /// characters this form cannot carry, those characters — and only those — + /// are percent-escaped and the value is marked, so the rest of it stays + /// readable and greppable. /// - private static string FormatString(string value) + private static string FormatString(string value, Form form) { - if (NeedsEncoding(value)) - { - var payload = Convert.ToBase64String(Encoding.UTF8.GetBytes(value)); - return $"({Base64Marker} {Quote(payload)})"; - } - return Quote(value); + var escaped = EscapeUnwritable(value, form); + return escaped is null ? Quote(value) : $"({EscapedMarker} {Quote(escaped)})"; } /// - /// A value can be written as text unless it contains control characters: - /// newlines break the line structure and CRLF normalisation would rewrite them. + /// Percent-escape the characters this form cannot carry, or null when + /// the text can be written as it is. % is escaped too, so escaping is + /// reversible. /// - private static bool NeedsEncoding(string value) + private static string? EscapeUnwritable(string value, Form form) { + if (!value.Any(c => IsUnwritable(c, form))) + { + return null; + } + + var output = new StringBuilder(value.Length); + var plain = new StringBuilder(); + + void FlushPlain() + { + if (plain.Length > 0) + { + output.Append(plain); + plain.Clear(); + } + } + foreach (var c in value) { - // Unicode category Cc: the C0 and C1 control ranges. - if (c <= 0x1f || (c >= 0x7f && c <= 0x9f)) + if (c != '%' && !IsUnwritable(c, form)) { - return true; + // Kept as it is, surrogate pairs included: only the characters + // this form cannot carry turn into escapes. + plain.Append(c); + continue; + } + FlushPlain(); + foreach (var b in Encoding.UTF8.GetBytes(c.ToString())) + { + output.Append('%').Append(b.ToString("X2", CultureInfo.InvariantCulture)); } } - return false; + FlushPlain(); + + return output.ToString(); } + /// + /// Whether a character has to be escaped in this form. A tab is text a reader + /// can see, and so is a newline in the indented form, where a value may span + /// lines. A carriage return is escaped because CRLF normalisation rewrites + /// it, and the remaining control characters because they are not text at all. + /// + private static bool IsUnwritable(char c, Form form) + { + if (!IsControl(c)) + { + return false; + } + if (c == '\t') + { + return false; + } + if (c == '\n') + { + return form == Form.Line; + } + return true; + } + + /// Unicode category Cc: the C0 and C1 control ranges. + private static bool IsControl(char c) => c <= 0x1f || (c >= 0x7f && c <= 0x9f); + + /// + /// Quote a value so that both this reader and the notation's own parser read + /// it back unchanged. One delimiter is enough while the text holds none of + /// that kind; when it holds both kinds, a run of at least three opens the + /// notation's n-quote form, where the text is literal and only a run at least + /// as long closes it. A value starting with the delimiter would lengthen the + /// opening run, so the other delimiter is used for it. + /// private static string Quote(string value) { if (!value.Contains('"')) @@ -427,19 +515,37 @@ private static string Quote(string value) { return $"'{value}'"; } - // Both quote styles are present: double the double quotes, as the parser expects. - return $"\"{value.Replace("\"", "\"\"", StringComparison.Ordinal)}\""; + + var delimiter = value.StartsWith('"') ? '\'' : '"'; + // A run of two delimiters is the empty value, so the n-quote form starts + // at three; beyond that the run only has to outrun the longest one inside. + var count = Math.Max(LongestRun(value, delimiter) + 1, 3); + var run = new string(delimiter, count); + return $"{run}{value}{run}"; + } + + /// The length of the longest run of a character in a text. + private static int LongestRun(string value, char c) + { + var longest = 0; + var current = 0; + foreach (var candidate in value) + { + current = candidate == c ? current + 1 : 0; + longest = Math.Max(longest, current); + } + return longest; } /// Format an object key. Keys are bare when they read as plain identifiers. - private static string FormatKey(string key) + private static string FormatKey(string key, Form form) { var plain = key.Length > 0 && key != Base64Marker - && !NeedsEncoding(key) - && !key.Any(c => char.IsWhiteSpace(c) || KeyNeedsQuotes.Contains(c)); + && key != EscapedMarker + && !key.Any(c => char.IsWhiteSpace(c) || IsControl(c) || KeyNeedsQuotes.Contains(c)); - return plain ? key : FormatString(key); + return plain ? key : FormatString(key, form); } // === Decoding === @@ -527,8 +633,63 @@ private static List Tokenize(string text) return tokens; } - /// Read a quoted reference, where a doubled quote character means a literal one. + /// + /// Read a quoted reference. The opening run of delimiters says how it is + /// read, which is what the notation's own parser does: + /// + /// + /// + /// one delimiter — the text is literal and a doubled delimiter is one + /// literal delimiter, which is how versions up to 0.6.0 wrote such values; + /// two — the empty value; + /// three or more — the n-quote form: the text is literal, and the value + /// ends at the first run at least as long, whose last delimiters close it. A + /// longer run therefore belongs to the text, so a value may end with a + /// delimiter. + /// + /// private static (string Value, int Next) ReadQuoted(char[] chars, int start, char quoteChar) + { + var opening = RunLength(chars, start, quoteChar); + + if (opening == 2) + { + return (string.Empty, start + 2); + } + + if (opening == 1) + { + return ReadDoubledQuoted(chars, start, quoteChar); + } + + int i = start + opening; + while (i < chars.Length) + { + if (chars[i] != quoteChar) + { + i++; + continue; + } + + var run = RunLength(chars, i, quoteChar); + if (run >= opening) + { + var length = i + run - opening - (start + opening); + var value = new string(chars, start + opening, length); + return (value, i + run); + } + i += run; + } + + throw UnterminatedQuote(start); + } + + /// + /// Read the single-delimiter form, where a doubled delimiter is one literal + /// delimiter. This is how versions up to 0.6.0 wrote a value holding both + /// quote kinds, so their documents keep decoding. + /// + private static (string Value, int Next) ReadDoubledQuoted(char[] chars, int start, char quoteChar) { var value = new StringBuilder(); int i = start + 1; @@ -549,10 +710,24 @@ private static (string Value, int Next) ReadQuoted(char[] chars, int start, char i++; } - throw new FormatException( - $"unterminated quoted value starting at character {start.ToString(CultureInfo.InvariantCulture)}"); + throw UnterminatedQuote(start); } + /// The length of the run of a character that starts at an index. + private static int RunLength(char[] chars, int start, char c) + { + int i = start; + while (i < chars.Length && chars[i] == c) + { + i++; + } + return i - start; + } + + /// The error a quoted value that never closes raises. + private static FormatException UnterminatedQuote(int start) => + new($"unterminated quoted value starting at character {start.ToString(CultureInfo.InvariantCulture)}"); + /// Cursor over the token stream, turning tokens into nodes and rows. private sealed class Cursor { @@ -721,14 +896,14 @@ private bool LinkIsMultiline() } // `key value` on every line makes an object; anything else is a list of values. - var isObject = rows.All(row => row.Count == 2 && row[0].IsRef); + var isObject = rows.All(row => row.Count == 2 && NodeToKey(row[0]) is not null); if (isObject) { var result = new Dictionary(); foreach (var row in rows) { - result[row[0].Value] = NodeToValue(row[1]); + result[NodeToKey(row[0])!] = NodeToValue(row[1]); } return result; } @@ -768,23 +943,59 @@ private bool LinkIsMultiline() + $"found a link of {node.Rows.Count.ToString(CultureInfo.InvariantCulture)} lines"); } var pair = node.Rows[0]; - if (pair.Count != 2 || !pair[0].IsRef) + if (pair.Count != 2) { throw new FormatException( $"an object marked '{ObjectMarker}:' holds (key value) pairs, " + $"found a link of {pair.Count.ToString(CultureInfo.InvariantCulture)} values"); } - result[pair[0].Value] = NodeToValue(pair[1]); + var key = NodeToKey(pair[0]); + if (key is null) + { + throw new FormatException( + $"an object marked '{ObjectMarker}:' holds (key value) pairs, " + + "found a pair whose key is not text"); + } + result[key] = NodeToValue(pair[1]); } return result; } /// - /// Recognise (base64 "…"), the individual marker for values that could - /// not be written as text. A quoted base64 key is an ordinary object - /// key, not a marker. + /// The key a node in key position spells: a reference is the key itself, and + /// a marked link is the text its marker escapes, which is how a key holding a + /// character the form cannot carry stays a key instead of turning its object + /// into an array. + /// + /// The key, or null when the node is not one + private static string? NodeToKey(Node node) + { + if (node.IsRef) + { + return node.Value; + } + if (node.IsObject) + { + return null; + } + try + { + return DecodeMarkedValue(node.Rows); + } + catch (FormatException) + { + return null; + } + } + + /// + /// Recognise a marked value: (escaped "…"), whose text is written as it + /// is except for the percent-escaped characters this form cannot carry, and + /// (base64 "…"), which versions up to 0.6.0 wrote and which is still + /// read. A quoted marker is an ordinary object key, not a marker. /// + /// The decoded string, or null when the link is not a marked value private static string? DecodeMarkedValue(List> rows) { if (rows.Count != 1 || rows[0].Count != 2) @@ -795,7 +1006,7 @@ private bool LinkIsMultiline() var marker = rows[0][0]; var payload = rows[0][1]; - if (!marker.IsRef || marker.Quoted || marker.Value != Base64Marker) + if (!marker.IsRef || marker.Quoted) { return null; } @@ -804,6 +1015,16 @@ private bool LinkIsMultiline() return null; } + if (marker.Value == EscapedMarker) + { + return Unescape(payload.Value); + } + + if (marker.Value != Base64Marker) + { + return null; + } + try { return Encoding.UTF8.GetString(Convert.FromBase64String(payload.Value)); @@ -814,6 +1035,59 @@ private bool LinkIsMultiline() } } + /// + /// Undo the percent-escaping of an (escaped "…") payload. Escapes stand + /// for bytes, so a character outside ASCII is written as its UTF-8 bytes and + /// read back from them. + /// + /// If an escape is truncated, malformed or not UTF-8 + private static string Unescape(string payload) + { + var bytes = new List(payload.Length); + int i = 0; + int position = 0; + + while (i < payload.Length) + { + if (payload[i] != '%') + { + // Copied over as it is, one whole character at a time, so that a + // surrogate pair keeps standing for the character it spells. + var width = char.IsHighSurrogate(payload[i]) && i + 1 < payload.Length ? 2 : 1; + bytes.AddRange(Encoding.UTF8.GetBytes(payload.Substring(i, width))); + i += width; + position++; + continue; + } + + if (i + 2 >= payload.Length) + { + throw new FormatException( + "truncated escape at character " + + position.ToString(CultureInfo.InvariantCulture) + + " of an escaped value"); + } + + var escape = payload.Substring(i + 1, 2); + if (!byte.TryParse(escape, NumberStyles.AllowHexSpecifier, CultureInfo.InvariantCulture, out var value)) + { + throw new FormatException($"invalid escape '%{escape}' in an escaped value"); + } + bytes.Add(value); + i += 3; + position += 3; + } + + try + { + return StrictUtf8.GetString(bytes.ToArray()); + } + catch (DecoderFallbackException e) + { + throw new FormatException("invalid UTF-8 escaped value", e); + } + } + /// /// Convert a reference to a value. Quoted references are always strings; bare /// references keep the type they were written with. diff --git a/csharp/tests/Lino.Objects.Codec.Tests/PlainTextValuesTests.cs b/csharp/tests/Lino.Objects.Codec.Tests/PlainTextValuesTests.cs new file mode 100644 index 0000000..15948d9 --- /dev/null +++ b/csharp/tests/Lino.Objects.Codec.Tests/PlainTextValuesTests.cs @@ -0,0 +1,223 @@ +// Real text stays real text in both readable forms (issue #45). +// +// Before the fix a single control character turned the whole string into +// base64: one newline in a log message hid the message, the stack trace and +// every word a reader would grep for. The readable forms now write the text as +// it is, and escape only the characters the form itself cannot carry. + +using Xunit; +using Lino.Objects.Codec; + +namespace Lino.Objects.Codec.Tests; + +/// +/// Checks that the readable forms write text as text. +/// +public class PlainTextValuesTests +{ + /// A record of the shape a log line actually holds. + private static Dictionary Message(string text) => new() + { + ["message"] = text, + }; + + /// How many times a piece of text occurs in another. + private static int Occurrences(string haystack, string needle) => + haystack.Split(needle).Length - 1; + + /// Deep equality over the values these tests build. + private static bool Same(object? left, object? right) + { + switch (left, right) + { + case (string a, string b): + return a == b; + case (int a, int b): + return a == b; + case (List a, List b): + return a.Count == b.Count && a.Zip(b).All(pair => Same(pair.First, pair.Second)); + case (IDictionary a, IDictionary b): + return a.Count == b.Count + && a.Keys.SequenceEqual(b.Keys) + && a.All(entry => Same(entry.Value, b[entry.Key])); + default: + return Equals(left, right); + } + } + + /// + /// The reason for the issue: a log line holding a newline must stay greppable. + /// + [Fact] + public void AMultiLineStringKeepsItsTextInTheIndentedForm() + { + var value = Message("line one\nline two"); + var encoded = Codec.Encode(value); + + Assert.Equal("(\n message \"line one\nline two\"\n)", encoded); + Assert.DoesNotContain("base64", encoded, StringComparison.Ordinal); + Assert.Contains("line one", encoded, StringComparison.Ordinal); + Assert.Contains("line two", encoded, StringComparison.Ordinal); + Assert.True(Same(Codec.Decode(encoded), value)); + } + + /// + /// On one line the record ends at the newline, so the newline -- and nothing + /// else -- is escaped: the rest of the message stays as written. + /// + [Fact] + public void OnlyTheNewlineIsEscapedInTheSingleLineForm() + { + var value = Message("line one\nline two"); + var line = Codec.EncodeLine(value); + + Assert.Equal("(o: (message (escaped \"line one%0Aline two\")))", line); + Assert.DoesNotContain('\n', line); + Assert.DoesNotContain("base64", line, StringComparison.Ordinal); + Assert.True(Same(Codec.DecodeLine(line), value)); + } + + /// A tab is text a reader can see, so both forms keep it as it is. + [Fact] + public void ATabIsWrittenAsATabInBothForms() + { + var value = Message("a\tb"); + + Assert.Equal("(\n message \"a\tb\"\n)", Codec.Encode(value)); + Assert.Equal("(o: (message \"a\tb\"))", Codec.EncodeLine(value)); + Assert.True(Same(Codec.Decode(Codec.Encode(value)), value)); + Assert.True(Same(Codec.DecodeLine(Codec.EncodeLine(value)), value)); + } + + /// + /// A carriage return is the one whitespace character a text file rewrites on + /// its own -- CRLF normalisation would change the value -- so it is escaped. + /// + [Fact] + public void ACarriageReturnIsEscapedSoCrlfNormalisationCannotRewriteIt() + { + var value = Message("first\r\nsecond"); + var encoded = Codec.Encode(value); + + Assert.Equal("(\n message (escaped \"first%0D\nsecond\")\n)", encoded); + Assert.True(Same(Codec.Decode(encoded), value)); + } + + /// + /// The doubled-quote form desynchronises the notation's own parser, so a + /// value holding both quote kinds is written with a run of delimiters instead. + /// + [Fact] + public void AValueHoldingBothQuoteKindsUsesTheNQuoteForm() + { + var value = Message("both \"kinds\" of 'quotes'"); + var encoded = Codec.Encode(value); + + Assert.Contains( + "\"\"\"both \"kinds\" of 'quotes'\"\"\"", encoded, StringComparison.Ordinal); + Assert.DoesNotContain("\"\"kinds\"\"", encoded, StringComparison.Ordinal); + Assert.True(Same(Codec.Decode(encoded), value)); + } + + /// + /// A value that occurs twice is written twice: a shared reference would make + /// a log line depend on another line, which a line-based reader cannot resolve. + /// + [Fact] + public void ARepeatedValueIsWrittenOutEveryTime() + { + var value = new Dictionary + { + ["first"] = "same", + ["second"] = "same", + ["third"] = "same", + }; + + var encoded = Codec.Encode(value); + Assert.Equal(3, Occurrences(encoded, "\"same\"")); + Assert.True(Same(Codec.Decode(encoded), value)); + + var line = Codec.EncodeLine(value); + Assert.Equal(3, Occurrences(line, "\"same\"")); + Assert.True(Same(Codec.DecodeLine(line), value)); + } + + /// + /// A key is escaped like any other text, and stays a key rather than turning + /// the object it belongs to into an array. + /// + [Fact] + public void AKeyHoldingAControlCharacterStaysAKey() + { + var value = new Dictionary { ["a\u0000b"] = 1 }; + + Assert.True(Same(Codec.Decode(Codec.Encode(value)), value)); + Assert.True(Same(Codec.DecodeLine(Codec.EncodeLine(value)), value)); + } + + /// Documents written by earlier versions keep decoding. + [Fact] + public void ThePreviousBase64MarkerStillDecodes() + { + var decoded = Codec.Decode("(\n message (base64 \"bGluZTEKbGluZTI=\")\n)"); + Assert.True(Same(decoded, Message("line1\nline2"))); + } + + /// Every kind of text a value may hold. + public static IEnumerable EveryKindOfText() => + new[] + { + string.Empty, + "plain", + "with spaces", + "it's", + "he said \"hello\"", + "both \"kinds\" of 'quotes'", + "\"leading quote", + "trailing quote\"", + "a\"\"b", + "a\"\"\"b'c", + "'\"", + "\"'", + "line one\nline two", + "trailing newline\n", + "\ttab", + "carriage\rreturn", + "null\u0000byte", + "escape\u001b[0m", + "next\u0085line", + "unicode: 你好世界 🌍", + "percent %0A not an escape", + "(parens) and: colons", + "base64", + "escaped", + "o:", + }.Select(text => new object[] { text }); + + /// + /// Every value the readable forms write must read back unchanged, whatever + /// quotes, newlines and control characters it holds. + /// + [Theory] + [MemberData(nameof(EveryKindOfText))] + public void EveryKindOfTextRoundtripsThroughBothForms(string text) + { + var shapes = new object?[] + { + text, + Message(text), + new Dictionary { [text] = text }, + new List { text }, + }; + + foreach (var value in shapes) + { + var encoded = Codec.Encode(value); + Assert.True(Same(Codec.Decode(encoded), value), $"indented roundtrip failed: {encoded}"); + + var line = Codec.EncodeLine(value); + Assert.DoesNotContain('\n', line); + Assert.True(Same(Codec.DecodeLine(line), value), $"single-line roundtrip failed: {line}"); + } + } +} diff --git a/csharp/tests/Lino.Objects.Codec.Tests/ReadableConformanceTests.cs b/csharp/tests/Lino.Objects.Codec.Tests/ReadableConformanceTests.cs index b278b13..02c075f 100644 --- a/csharp/tests/Lino.Objects.Codec.Tests/ReadableConformanceTests.cs +++ b/csharp/tests/Lino.Objects.Codec.Tests/ReadableConformanceTests.cs @@ -39,12 +39,14 @@ private static string FindFixtures() throw new FileNotFoundException("cannot locate fixtures/readable-format/cases.json"); } - private static JsonElement Cases() + private static JsonElement Section(string key) { using var document = JsonDocument.Parse(File.ReadAllText(FixturesPath)); - return document.RootElement.GetProperty("cases").Clone(); + return document.RootElement.GetProperty(key).Clone(); } + private static JsonElement Cases() => Section("cases"); + /// /// Build a C# value from the fixtures' tagged encoding. A value is a /// single-key object naming its type, so a string "42" and the number 42 @@ -132,6 +134,15 @@ public static IEnumerable AllCases() } } + /// The documents an earlier version of this format wrote. + public static IEnumerable LegacyCases() + { + foreach (var @case in Section("legacy").EnumerateArray()) + { + yield return new object[] { @case.GetProperty("name").GetString()!, @case.Clone() }; + } + } + [Fact] public void EveryCaseIsEitherActiveOrSkippedWithAReason() { @@ -233,4 +244,32 @@ public void ThePlainDecoderReadsEachSharedLine(string name, JsonElement @case) var decoded = Codec.Decode(@case.GetProperty("line").GetString()!); Assert.True(Same(expected, decoded), $"case {name} decoded to a different value"); } + + /// + /// Documents written before this format wrote text as text keep decoding, so + /// upgrading a reader never loses a stored record. + /// + [Theory] + [MemberData(nameof(LegacyCases))] + public void DecodesTheDocumentsAnEarlierVersionWrote(string name, JsonElement @case) + { + var expected = Build(@case.GetProperty("value")); + var decoded = Codec.Decode(@case.GetProperty("text").GetString()!); + Assert.True(Same(expected, decoded), $"legacy case {name} decoded to a different value"); + } + + /// + /// The point of the change: an implementation may not reach for base64 while + /// writing a readable document, whatever the text holds. + /// + [Theory] + [MemberData(nameof(AllCases))] + public void NoSharedDocumentHidesItsTextInBase64(string name, JsonElement @case) + { + Assert.DoesNotContain( + "base64 \"", @case.GetProperty("text").GetString()!, StringComparison.Ordinal); + Assert.DoesNotContain( + "base64 \"", @case.GetProperty("line").GetString()!, StringComparison.Ordinal); + Assert.NotEqual(string.Empty, name); + } } diff --git a/csharp/tests/Lino.Objects.Codec.Tests/SingleLineFormatTests.cs b/csharp/tests/Lino.Objects.Codec.Tests/SingleLineFormatTests.cs index 5bc603a..9081348 100644 --- a/csharp/tests/Lino.Objects.Codec.Tests/SingleLineFormatTests.cs +++ b/csharp/tests/Lino.Objects.Codec.Tests/SingleLineFormatTests.cs @@ -101,8 +101,8 @@ public void AStringKeepsItsOwnCharactersOnOneLine() } /// - /// A newline inside a string would end the record, so such a string is the - /// one thing written encoded -- individually, so the rest stays readable. + /// A newline inside a string would end the record, so the newline -- and + /// nothing else -- is escaped: the rest of the text stays as it was written. /// [Fact] public void AStringHoldingANewlineStillFitsOnOneLine() @@ -114,9 +114,11 @@ public void AStringHoldingANewlineStillFitsOnOneLine() }; var line = Codec.EncodeLine(value); Assert.Equal( - "(o: (readable \"still visible\") (multiline (base64 \"bGluZTEKbGluZTI=\")))", + "(o: (readable \"still visible\") (multiline (escaped \"line1%0Aline2\")))", line); Assert.DoesNotContain('\n', line); + Assert.Contains("line1", line, StringComparison.Ordinal); + Assert.Contains("line2", line, StringComparison.Ordinal); Assert.True(Equivalent(Codec.DecodeLine(line), value)); } diff --git a/experiments/issue-45/README.md b/experiments/issue-45/README.md new file mode 100644 index 0000000..c044ae4 --- /dev/null +++ b/experiments/issue-45/README.md @@ -0,0 +1,39 @@ +# Issue #45 experiments + +## `quote-probe` + +Determines the delimiter rule `links-notation` 0.14.0 actually implements for a +quoted value, which is what the readable encoder has to write against. Run it +with `cargo run` inside `quote-probe`. + +Its output is the evidence behind `quote()` in the four readable encoders: + +- a run of **one** delimiter uses the doubled-delimiter escape, and a value that + holds the delimiter desynchronises the parser for the rest of the document; +- a run of **two** delimiters is the empty value, whatever follows it; +- a run of **three or more** delimiters carries its content literally, ends at + the first run of at least that length, and the *last* N delimiters of that run + are the closing ones -- so a value ending with the delimiter still reads back + unchanged, while a value *starting* with it would lengthen the opening run and + has to use the other delimiter instead. + +## Which parser reads the n-quote form + +The four packages pin different `links-notation` releases, and the quoting rule +changed between them, so the same document is read differently: + +| package | version | reads `"""say "hi""""` as | reads `"say ""hi"""` as | +| --- | --- | --- | --- | +| `links-notation` (Rust) | 0.14.0 | `say "hi"` | `say ` -- desynchronises | +| `Link.Foundation.Links.Notation` (C#) | 0.13.0 | `"say ` then `hi""""` | `say "hi"` | +| `links-notation` (npm) | 0.11.2 | `"""say` then `hi` then `"""` | `say ""hi""` | +| `links-notation` (PyPI) | 0.11.2 | `""say "hi"""` | `say ""hi""` | + +Every row was measured, not assumed: the Rust one by `quote-probe`, the other +three by parsing those four documents with each package directly. + +The readable format is read by this repository's own tokenizer in every +language, so a value reads back unchanged everywhere regardless. The n-quote +form is chosen because it is what the newest notation release implements, which +is the rule the issue asks the encoder to write against; the Rust suite is the +one that can prove it, in `rust/tests/plain_text_values.rs`. diff --git a/experiments/issue-45/quote-probe/.gitignore b/experiments/issue-45/quote-probe/.gitignore new file mode 100644 index 0000000..2f7896d --- /dev/null +++ b/experiments/issue-45/quote-probe/.gitignore @@ -0,0 +1 @@ +target/ diff --git a/experiments/issue-45/quote-probe/Cargo.lock b/experiments/issue-45/quote-probe/Cargo.lock new file mode 100644 index 0000000..19c2124 --- /dev/null +++ b/experiments/issue-45/quote-probe/Cargo.lock @@ -0,0 +1,81 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "links-notation" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b6e5f36d99612ea82da43dbd5efb37b0b4e9c4b8197228e073b60ef5a0e78f2" +dependencies = [ + "links-notation-macro", + "nom", +] + +[[package]] +name = "links-notation-macro" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f30ea96250240a92d69d45579dbd199a713e7a79acfa53033d24628dad23cff5" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "memchr" +version = "2.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" + +[[package]] +name = "nom" +version = "8.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df9761775871bdef83bee530e60050f7e54b1105350d6884eb0fb4f46c2f9405" +dependencies = [ + "memchr", +] + +[[package]] +name = "proc-macro2" +version = "1.0.107" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "985e7ec9bb745e6ce6535b544d84d6cd6f7ad8bd711c398938ae983b91a766d9" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.47" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fbf4db142a473a8d80c26bbf18454ed458bf8d26c8219c331daecfdbd079001" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "quote-probe" +version = "0.1.0" +dependencies = [ + "links-notation", +] + +[[package]] +name = "syn" +version = "3.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6275cddf4610d1775e6d1fe9469b2e77d0f39fd98fb7450901b821e0c53649f" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "unicode-ident" +version = "1.0.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" diff --git a/experiments/issue-45/quote-probe/Cargo.toml b/experiments/issue-45/quote-probe/Cargo.toml new file mode 100644 index 0000000..369a663 --- /dev/null +++ b/experiments/issue-45/quote-probe/Cargo.toml @@ -0,0 +1,7 @@ +[package] +name = "quote-probe" +version = "0.1.0" +edition = "2021" + +[dependencies] +links-notation = "0.14.0" diff --git a/experiments/issue-45/quote-probe/src/main.rs b/experiments/issue-45/quote-probe/src/main.rs new file mode 100644 index 0000000..eaa2b1a --- /dev/null +++ b/experiments/issue-45/quote-probe/src/main.rs @@ -0,0 +1,70 @@ +//! Determine the delimiter rule links-notation 0.14.0 actually implements for +//! quoted values, so the readable encoder can pick a delimiter that always +//! reads back unchanged. + +use links_notation::{parse_lino, LiNo}; + +/// Extract the second value of the first inner link: `(a ) …`. +fn first_value(text: &str) -> Option { + let parsed = parse_lino(text).ok()?; + fn walk(node: &LiNo, out: &mut Vec) { + match node { + LiNo::Ref(r) => out.push(r.clone()), + LiNo::Link { values, .. } => { + for v in values { + walk(v, out); + } + } + } + } + let mut refs = Vec::new(); + walk(&parsed, &mut refs); + // refs = ["a", value, …] + refs.get(1).cloned() +} + +fn quoted(value: &str, delim: char, count: usize) -> String { + let d: String = std::iter::repeat(delim).take(count).collect(); + format!("{d}{value}{d}") +} + +fn main() { + let contents = [ + "plain", + "say \"hi\"", + "\"leading", + "trailing\"", + "\"both\"", + "a\"\"b", + "a\"\"\"b", + "\"\"", + "", + "line1\nline2", + "a\tb", + "it's", + "mixed \" and '", + ]; + + for content in contents { + for count in 1..=5usize { + let body = quoted(content, '"', count); + for (label, doc) in [ + ("alone", format!("(a {body})")), + ("sibling", format!("(a {body}) (b 1)")), + ] { + let got = first_value(&doc); + let ok = got.as_deref() == Some(content); + println!( + "{:>3} {:<8} content={:<16} doc={:<32} -> {:<20} {}", + count, + label, + format!("{content:?}"), + format!("{doc:?}"), + format!("{got:?}"), + if ok { "ok" } else { "MISMATCH" } + ); + } + } + println!(); + } +} diff --git a/fixtures/readable-format/cases.json b/fixtures/readable-format/cases.json index c6465fe..e4d59ec 100644 --- a/fixtures/readable-format/cases.json +++ b/fixtures/readable-format/cases.json @@ -147,8 +147,24 @@ "value": { "str": "both \"kinds\" of 'quotes'" }, - "text": "\"both \"\"kinds\"\" of 'quotes'\"", - "line": "\"both \"\"kinds\"\" of 'quotes'\"" + "text": "\"\"\"both \"kinds\" of 'quotes'\"\"\"", + "line": "\"\"\"both \"kinds\" of 'quotes'\"\"\"" + }, + { + "name": "string_ending_with_the_quote_delimiter", + "value": { + "str": "it's \"quoted\"" + }, + "text": "\"\"\"it's \"quoted\"\"\"\"", + "line": "\"\"\"it's \"quoted\"\"\"\"" + }, + { + "name": "string_starting_with_the_quote_delimiter", + "value": { + "str": "\"quoted\" and it's" + }, + "text": "'''\"quoted\" and it's'''", + "line": "'''\"quoted\" and it's'''" }, { "name": "string_unicode", @@ -187,16 +203,40 @@ "value": { "str": "line1\nline2" }, - "text": "(base64 \"bGluZTEKbGluZTI=\")", - "line": "(base64 \"bGluZTEKbGluZTI=\")" + "text": "\"line1\nline2\"", + "line": "(escaped \"line1%0Aline2\")" }, { "name": "string_with_tab", "value": { "str": "a\tb" }, - "text": "(base64 \"YQli\")", - "line": "(base64 \"YQli\")" + "text": "\"a\tb\"", + "line": "\"a\tb\"" + }, + { + "name": "string_with_carriage_return", + "value": { + "str": "line1\r\nline2" + }, + "text": "(escaped \"line1%0D\nline2\")", + "line": "(escaped \"line1%0D%0Aline2\")" + }, + { + "name": "string_with_control_character", + "value": { + "str": "a\u0000b" + }, + "text": "(escaped \"a%00b\")", + "line": "(escaped \"a%00b\")" + }, + { + "name": "string_with_percent_sign", + "value": { + "str": "100% done, %0A is not an escape here" + }, + "text": "\"100% done, %0A is not an escape here\"", + "line": "\"100% done, %0A is not an escape here\"" }, { "name": "empty_array", @@ -484,21 +524,27 @@ } ], [ - "with:colon", + "escaped", { "int": 4 } ], [ - "with\"quote", + "with:colon", { "int": 5 } + ], + [ + "with\"quote", + { + "int": 6 + } ] ] }, - "text": "(\n \"two words\" 1\n \"\" 2\n \"base64\" 3\n \"with:colon\" 4\n 'with\"quote' 5\n)", - "line": "(o: (\"two words\" 1) (\"\" 2) (\"base64\" 3) (\"with:colon\" 4) ('with\"quote' 5))" + "text": "(\n \"two words\" 1\n \"\" 2\n \"base64\" 3\n \"escaped\" 4\n \"with:colon\" 5\n 'with\"quote' 6\n)", + "line": "(o: (\"two words\" 1) (\"\" 2) (\"base64\" 3) (\"escaped\" 4) (\"with:colon\" 5) ('with\"quote' 6))" }, { "name": "base64_key_with_plain_value_is_not_a_marker", @@ -515,6 +561,36 @@ "text": "(\n \"base64\" \"plain text\"\n)", "line": "(o: (\"base64\" \"plain text\"))" }, + { + "name": "escaped_key_with_plain_value_is_not_a_marker", + "value": { + "object": [ + [ + "escaped", + { + "str": "plain text" + } + ] + ] + }, + "text": "(\n \"escaped\" \"plain text\"\n)", + "line": "(o: (\"escaped\" \"plain text\"))" + }, + { + "name": "key_holding_a_newline_stays_a_key", + "value": { + "object": [ + [ + "a\nb", + { + "int": 1 + } + ] + ] + }, + "text": "(\n \"a\nb\" 1\n)", + "line": "(o: ((escaped \"a%0Ab\") 1))" + }, { "name": "documented_router_state", "value": { @@ -643,11 +719,71 @@ { "str": "a\tb" } + ], + [ + "returned", + { + "str": "line1\rline2" + } + ], + [ + "control", + { + "str": "a\u0000b" + } + ] + ] + }, + "text": "(\n readable \"still visible\"\n multiline \"line1\nline2\"\n tabbed \"a\tb\"\n returned (escaped \"line1%0Dline2\")\n control (escaped \"a%00b\")\n)", + "line": "(o: (readable \"still visible\") (multiline (escaped \"line1%0Aline2\")) (tabbed \"a\tb\") (returned (escaped \"line1%0Dline2\")) (control (escaped \"a%00b\")))" + }, + { + "name": "repeated_value_is_written_every_time", + "value": { + "object": [ + [ + "first", + { + "str": "same" + } + ], + [ + "second", + { + "str": "same" + } + ], + [ + "third", + { + "str": "same" + } ] ] }, - "text": "(\n readable \"still visible\"\n multiline (base64 \"bGluZTEKbGluZTI=\")\n tabbed (base64 \"YQli\")\n)", - "line": "(o: (readable \"still visible\") (multiline (base64 \"bGluZTEKbGluZTI=\")) (tabbed (base64 \"YQli\")))" + "text": "(\n first \"same\"\n second \"same\"\n third \"same\"\n)", + "line": "(o: (first \"same\") (second \"same\") (third \"same\"))" + }, + { + "name": "multi_line_log_record_stays_greppable", + "value": { + "object": [ + [ + "level", + { + "str": "error" + } + ], + [ + "message", + { + "str": "Traceback (most recent call last):\n File \"app.py\", line 42\nValueError: boom" + } + ] + ] + }, + "text": "(\n level \"error\"\n message 'Traceback (most recent call last):\n File \"app.py\", line 42\nValueError: boom'\n)", + "line": "(o: (level \"error\") (message (escaped 'Traceback (most recent call last):%0A File \"app.py\", line 42%0AValueError: boom')))" }, { "name": "deeply_nested_objects", @@ -683,5 +819,56 @@ "line": "(o: (a (o: (b (o: (c (1)))))))" } ], - "lineEncoding": "`line` is the single-line form: the same document written without any line break, one record per line. Line breaks no longer separate rows there, so an object names itself with the `o` link id (`(o: (key value) ...)`, `(o:)` when empty) and a bare link is always an array." + "lineEncoding": "`line` is the single-line form: the same document written without any line break, one record per line. Line breaks no longer separate rows there, so an object names itself with the `o` link id (`(o: (key value) ...)`, `(o:)` when empty) and a bare link is always an array.", + "legacyDecoding": "`legacy` holds documents written by versions up to 0.6.0, which every implementation must still decode: a string holding a control character was replaced by a `(base64 \"...\")` payload, and a string holding both quote kinds was written with its double quotes doubled. These are read only -- no implementation writes them any more, so they have no `line` of their own.", + "legacy": [ + { + "name": "base64_marked_string", + "value": { + "str": "line1\nline2" + }, + "text": "(base64 \"bGluZTEKbGluZTI=\")" + }, + { + "name": "base64_marked_value_in_an_object", + "value": { + "object": [ + [ + "readable", + { + "str": "still visible" + } + ], + [ + "multiline", + { + "str": "line1\nline2" + } + ] + ] + }, + "text": "(\n readable \"still visible\"\n multiline (base64 \"bGluZTEKbGluZTI=\")\n)" + }, + { + "name": "base64_marked_value_on_one_line", + "value": { + "object": [ + [ + "multiline", + { + "str": "line1\nline2" + } + ] + ] + }, + "text": "(o: (multiline (base64 \"bGluZTEKbGluZTI=\")))" + }, + { + "name": "doubled_quotes_in_a_string", + "value": { + "str": "both \"kinds\" of 'quotes'" + }, + "text": "\"both \"\"kinds\"\" of 'quotes'\"" + } + ] } diff --git a/js/.changeset/20260827_120000_issue_45_plain_text_values.md b/js/.changeset/20260827_120000_issue_45_plain_text_values.md new file mode 100644 index 0000000..a5cfca9 --- /dev/null +++ b/js/.changeset/20260827_120000_issue_45_plain_text_values.md @@ -0,0 +1,26 @@ +--- +'lino-objects-codec': minor +--- + +`encode` and `encodeLine` never reach for base64. A single control character +used to turn a whole string into base64, so a log message holding one newline +hid its own text: the message, the stack trace and every word a reader would +grep for. Both readable forms now write the text as it is and escape only what +the form itself cannot carry — the newline on a single line, the carriage return +everywhere, and the remaining control characters — in a value marked +`(escaped "line one%0Aline two")` whose payload is percent-escaped, so even the +escaped part stays readable. base64 is reachable only through `encodeCompact()` +/ `encodeObfuscated()`, which say so by name. + +A string containing the quote delimiter is written between a run of at least +three of them — `"""say "hi""""` — instead of by doubling the quote, which +desynchronises the notation's own parser. The exported `ESCAPED_MARKER` names +the new `escaped` link id. + +Fixes a key holding a control character, which used to be written as +`(base64 "…")` in key position and read back as an array element, so +`{"a\nb": "a\nb"}` decoded to `["a\nb", "a\nb"]`. `(base64 "…")` is still +decoded, so every document written up to 0.6.0 keeps reading; the shared +fixtures pin this in a `legacy` section. + +See [issue #45](https://github.com/link-foundation/lino-objects-codec/issues/45). diff --git a/js/README.md b/js/README.md index 16f238b..730f6c1 100644 --- a/js/README.md +++ b/js/README.md @@ -29,7 +29,7 @@ These tools enable easy implementation of higher-level features like: - **Readable by Default**: `encode({ obj })` writes plain, indented text that can be read and reviewed - **One Record per Line**: `encodeLine({ obj })` writes the same document on one line and `decodeLine({ notation })` reads it back exactly, so an append-only log stays greppable, tailable and countable by `wc -l` - **Object Identity**: Shared references and circular references are preserved by the compact format (`encodeCompact`) via object ids -- **Full Unicode**: Strings are written as text; only a value that cannot be written as text (one holding control characters) is base64-encoded, and it is marked individually as `(base64 "…")` +- **Full Unicode**: Strings are always written as text — a newline stays a newline and a tab stays a tab, so every word stays greppable; only the characters a form cannot carry are percent-escaped, in a value marked individually as `(escaped "…")` - **Opt-in Tracing**: Set `LINO_CODEC_DEBUG=1` to trace encoding and decoding, the same way in every language - **Compact JSON/Lino Conversion**: Convert between JSON and compact Links Notation with `jsonToLino({ json })` and `linoToJson({ lino })` - **Reference Escaping**: Properly escape strings for Links Notation format with `escapeReference({ value })` @@ -332,9 +332,16 @@ bare-value lines make an array: - Numbers, `true`, `false` and `null` are bare, so types survive a round trip - `NaN`, `Infinity` and `-Infinity` are written as such - An empty array is `()`; an empty object is `(` + newline + `)` -- A value that cannot be written as text (one containing control characters) is - base64-encoded on its own and marked as `(base64 "bGluZTEKbGluZTI=")`; - everything around it stays readable +- A string is written as text whatever it holds: a newline stays a newline and + a tab stays a tab, so every word stays greppable +- A string containing the quote delimiter is written between a run of at least + three of them — `"""say "hi""""` — rather than by doubling the quote +- Only the characters this form cannot carry — a carriage return and the + remaining control characters — are percent-escaped, in a value marked on its + own as `(escaped "first%0D")`; everything around it stays readable, and + `(base64 "…")` written by earlier versions is still decoded +- A value that occurs more than once is written out every time: a shared + reference would make one record depend on another ### Single-line format (`encodeLine`) @@ -361,7 +368,8 @@ The previous single-line form, kept for compatibility and for the object graphs the readable tree cannot express (shared and circular references): - Basic types carry a type marker: `(int 42)`, `(str aGVsbG8=)`, `(bool true)` -- Strings are base64-encoded to handle special characters and newlines +- Strings are base64-encoded here, and only here: this is the one form that + asks for it by name, and `encode()` never reaches for it - Shared / cyclic collections are defined inline with a self-reference id, e.g. `(obj_0: array (int 1) (int 2) ...)`; a self-referencing object `{ self: obj }` encodes as `(obj_0: object ((str c2VsZg==) obj_0))`. See diff --git a/js/eslint.config.js b/js/eslint.config.js index a394d0d..fa3f14e 100644 --- a/js/eslint.config.js +++ b/js/eslint.config.js @@ -22,6 +22,8 @@ export default [ __filename: 'readonly', // Node.js 18+ globals fetch: 'readonly', + TextEncoder: 'readonly', + TextDecoder: 'readonly', // Runtime-specific globals Bun: 'readonly', Deno: 'readonly', diff --git a/js/src/index.js b/js/src/index.js index d7a99bd..de35bee 100644 --- a/js/src/index.js +++ b/js/src/index.js @@ -34,6 +34,7 @@ export { export { DEFAULT_INDENT, BASE64_MARKER, + ESCAPED_MARKER, OBJECT_MARKER, CircularReferenceError, } from './readable.js'; diff --git a/js/src/readable.js b/js/src/readable.js index c5d3649..0b8d468 100644 --- a/js/src/readable.js +++ b/js/src/readable.js @@ -31,16 +31,21 @@ * | ------------------------------ | ---------------------------------------- | * | plain object | `( )` with one `key value` pair per line | * | `Array` | `( )` with one value per line | - * | `string` | quoted, never encoded | + * | `string` | quoted text, never base64 | * | `number` / `boolean` / `null` / `undefined` | bare, so the type survives the round trip | * * Empty containers keep their type: an empty array is `()` on one line, while an * empty object is written as `(` and `)` on two lines. * - * Only values that cannot be written as plain text are encoded: strings holding - * control characters (including newlines and tabs, which line-based tooling and - * CRLF normalisation would corrupt) are marked individually as - * `(base64 "…")` instead of encoding the whole document. + * Text is written as text. A string keeps every character a reader would grep + * for, including newlines and tabs, and is quoted with a run of delimiters — + * `"""say "hi""""` — when it holds the delimiter itself. Only the characters a + * form cannot carry are escaped, and only they: the value is then written as + * `(escaped "…")`, where `%XX` stands for one escaped byte. The indented form + * escapes the carriage return, which CRLF normalisation would otherwise rewrite, + * and the other control characters; the single-line form escapes the newline as + * well, because there a record ends at the end of the line. Nothing else is + * encoded: base64 lives in `encodeCompact`, which a caller asks for by name. * * # Single-line form * @@ -78,9 +83,27 @@ import { trace } from './debug.js'; /** Default indentation used by {@link encode}. */ export const DEFAULT_INDENT = ' '; -/** Marker used for values that cannot be represented as plain text. */ +/** + * Marker of a base64 payload. Written by `encodeCompact` and by versions up to + * 0.6.0 of the readable form, which is why it is still read. + */ export const BASE64_MARKER = 'base64'; +/** + * Marker of a string whose unwritable characters are percent-escaped. + * + * It reads as `(escaped "line one%0Aline two")`. Only those characters change; + * the rest of the text is written as it is, so the value stays readable and + * greppable. + */ +export const ESCAPED_MARKER = 'escaped'; + +/** The indented form, where a value may hold a line break of its own. */ +const FORM_INDENTED = 'indented'; + +/** The single-line form, where a record ends at the end of the line. */ +const FORM_LINE = 'line'; + /** Link id naming an object in the single-line form, written as `(o: …)`. */ export const OBJECT_MARKER = 'o'; @@ -99,7 +122,14 @@ const BARE_LITERALS = new Map([ const QUOTE_CHARS = ['"', "'", '`']; /** Characters that force an object key to be quoted. */ -const KEY_NEEDS_QUOTES = /[\s()':`"]/; +// eslint-disable-next-line no-control-regex +const KEY_NEEDS_QUOTES = /[\s()':`"\u0000-\u001f\u007f-\u009f]/; + +/** Encoder used to write one character as the bytes its escapes stand for. */ +const UTF8_ENCODER = new TextEncoder(); + +/** Decoder used to read an escaped payload back, rejecting invalid UTF-8. */ +const UTF8_DECODER = new TextDecoder('utf-8', { fatal: true }); /** * Raised when a value cannot be written because it refers back to itself. @@ -232,7 +262,7 @@ function writeValue(value, indent, level, out, path) { return; } writeRows(entries, indent, level, out, ([key, child]) => { - out.push(formatKey(key)); + out.push(formatKey(key, FORM_INDENTED)); out.push(' '); writeValue(child, indent, level + 1, out, path); }); @@ -240,7 +270,7 @@ function writeValue(value, indent, level, out, path) { return; } - out.push(formatScalar(value)); + out.push(formatScalar(value, FORM_INDENTED)); } /** @@ -276,7 +306,7 @@ function writeLineValue(value, out, path) { } out.push(`(${OBJECT_MARKER}:`); for (const [key, child] of entries) { - out.push(` (${formatKey(key)} `); + out.push(` (${formatKey(key, FORM_LINE)} `); writeLineValue(child, out, path); out.push(')'); } @@ -285,7 +315,7 @@ function writeLineValue(value, out, path) { return; } - out.push(formatScalar(value)); + out.push(formatScalar(value, FORM_LINE)); } /** @@ -353,9 +383,10 @@ function isPlainContainer(value) { * Format a scalar value. Strings are quoted, everything else stays bare so that * its type is recoverable when reading the document back. * @param {*} value - The scalar to format + * @param {string} form - The form being written, indented or single-line * @returns {string} The formatted scalar */ -function formatScalar(value) { +function formatScalar(value, form) { if (value === null) { return 'null'; } @@ -369,7 +400,7 @@ function formatScalar(value) { return formatNumber(value); } if (typeof value === 'string') { - return formatString(value); + return formatString(value, form); } if (typeof value === 'bigint') { return value.toString(); @@ -391,36 +422,88 @@ function formatNumber(value) { } /** - * Format a string value: quoted plain text, or an individually marked base64 - * payload when the text cannot be written literally. + * Format a string value. The text is written as text; when it holds characters + * this form cannot carry, those characters — and only those — are + * percent-escaped and the value is marked, so the rest of it stays readable and + * greppable. * @param {string} value - The string to format + * @param {string} form - The form being written, indented or single-line * @returns {string} The formatted string */ -function formatString(value) { - if (needsEncoding(value)) { - const payload = Buffer.from(value, 'utf-8').toString('base64'); - return `(${BASE64_MARKER} ${quote(payload)})`; +function formatString(value, form) { + const escaped = escapeUnwritable(value, form); + if (escaped === undefined) { + return quote(value); } - return quote(value); + return `(${ESCAPED_MARKER} ${quote(escaped)})`; } /** - * A value can be written as text unless it contains control characters: - * newlines break the line structure and CRLF normalisation would rewrite them. - * @param {string} value - The string to check - * @returns {boolean} True when the string has to be encoded + * Percent-escape the characters this form cannot carry, or `undefined` when the + * text can be written as it is. `%` is escaped too, so escaping is reversible. + * @param {string} value - The string to escape + * @param {string} form - The form being written, indented or single-line + * @returns {string|undefined} The escaped text, or undefined when none is needed */ -function needsEncoding(value) { +function escapeUnwritable(value, form) { + let unwritable = false; for (const char of value) { - const code = char.codePointAt(0); - // Unicode category Cc: the C0 and C1 control ranges. - if (code <= 0x1f || (code >= 0x7f && code <= 0x9f)) { - return true; + if (isUnwritable(char, form)) { + unwritable = true; + break; } } - return false; + if (!unwritable) { + return undefined; + } + + let out = ''; + for (const char of value) { + if (char !== '%' && !isUnwritable(char, form)) { + out += char; + continue; + } + for (const byte of UTF8_ENCODER.encode(char)) { + out += `%${byte.toString(16).toUpperCase().padStart(2, '0')}`; + } + } + return out; } +/** + * Whether a character has to be escaped in this form. A tab is text a reader can + * see, and so is a newline in the indented form, where a value may span lines. + * A carriage return is escaped because CRLF normalisation rewrites it, and the + * remaining control characters because they are not text at all. + * @param {string} char - The character to classify + * @param {string} form - The form being written, indented or single-line + * @returns {boolean} True when the character cannot be written as it is + */ +function isUnwritable(char, form) { + const code = char.codePointAt(0); + // Unicode category Cc: the C0 and C1 control ranges. + if (!(code <= 0x1f || (code >= 0x7f && code <= 0x9f))) { + return false; + } + if (char === '\t') { + return false; + } + if (char === '\n') { + return form === FORM_LINE; + } + return true; +} + +/** + * Quote a value so that both this reader and the notation's own parser read it + * back unchanged. One delimiter is enough while the text holds none of that + * kind; when it holds both kinds, a run of at least three opens the notation's + * n-quote form, where the text is literal and only a run at least as long closes + * it. A value starting with the delimiter would lengthen the opening run, so the + * other delimiter is used for it. + * @param {string} value - The text to quote + * @returns {string} The quoted text + */ function quote(value) { if (!value.includes('"')) { return `"${value}"`; @@ -428,23 +511,45 @@ function quote(value) { if (!value.includes("'")) { return `'${value}'`; } - // Both quote styles are present: double the double quotes, as the parser expects. - return `"${value.replaceAll('"', '""')}"`; + + const delimiter = value.startsWith('"') ? "'" : '"'; + // A run of two delimiters is the empty value, so the n-quote form starts at + // three; beyond that the run only has to outrun the longest one inside. + const count = Math.max(longestRun(value, delimiter) + 1, 3); + const run = delimiter.repeat(count); + return `${run}${value}${run}`; +} + +/** + * The length of the longest run of a character in a text. + * @param {string} value - The text to scan + * @param {string} char - The character to count + * @returns {number} The length of the longest run + */ +function longestRun(value, char) { + let longest = 0; + let current = 0; + for (const candidate of value) { + current = candidate === char ? current + 1 : 0; + longest = Math.max(longest, current); + } + return longest; } /** * Format an object key. Keys are bare when they read as plain identifiers. * @param {string} key - The key to format + * @param {string} form - The form being written, indented or single-line * @returns {string} The formatted key */ -function formatKey(key) { +function formatKey(key, form) { const plain = key.length > 0 && key !== BASE64_MARKER && - !needsEncoding(key) && + key !== ESCAPED_MARKER && !KEY_NEEDS_QUOTES.test(key); - return plain ? key : formatString(key); + return plain ? key : formatString(key, form); } // === Decoding === @@ -507,30 +612,87 @@ function tokenize(text) { } /** - * Read a quoted reference, where a doubled quote character means a literal one. + * Read a quoted reference. The opening run of delimiters says how it is read, + * which is what the notation's own parser does: + * + * * one delimiter — the text is literal and a doubled delimiter is one literal + * delimiter, which is how versions up to 0.6.0 wrote such values; + * * two — the empty value; + * * three or more — the n-quote form: the text is literal, and the value ends at + * the first run at least as long, whose last delimiters close it. A longer run + * therefore belongs to the text, so a value may end with a delimiter. * @param {string[]} chars - The document characters * @param {number} start - Index of the opening quote * @param {string} quoteChar - The quote character used * @returns {[string, number]} The value and the index after the closing quote */ function readQuoted(chars, start, quoteChar) { - let value = ''; - let i = start + 1; + const opening = runLength(chars, start, quoteChar); - while (i < chars.length) { - if (chars[i] === quoteChar) { - if (chars[i + 1] === quoteChar) { - value += quoteChar; - i += 2; - continue; + if (opening === 2) { + return ['', start + 2]; + } + + if (opening === 1) { + let value = ''; + let i = start + 1; + + while (i < chars.length) { + if (chars[i] === quoteChar) { + if (chars[i + 1] === quoteChar) { + value += quoteChar; + i += 2; + continue; + } + return [value, i + 1]; } - return [value, i + 1]; + value += chars[i]; + i += 1; + } + + throw unterminatedQuote(start); + } + + let i = start + opening; + while (i < chars.length) { + if (chars[i] !== quoteChar) { + i += 1; + continue; } - value += chars[i]; + + const run = runLength(chars, i, quoteChar); + if (run >= opening) { + const value = chars.slice(start + opening, i + run - opening).join(''); + return [value, i + run]; + } + i += run; + } + + throw unterminatedQuote(start); +} + +/** + * The length of the run of a character that starts at an index. + * @param {string[]} chars - The document characters + * @param {number} start - Index the run starts at + * @param {string} char - The character to count + * @returns {number} The length of the run + */ +function runLength(chars, start, char) { + let i = start; + while (i < chars.length && chars[i] === char) { i += 1; } + return i - start; +} - throw new SyntaxError( +/** + * The error a quoted value that never closes raises. + * @param {number} start - Index of the opening quote + * @returns {SyntaxError} The error to throw + */ +function unterminatedQuote(start) { + return new SyntaxError( `unterminated quoted value starting at character ${start}` ); } @@ -674,12 +836,14 @@ function rowsToValue(rows, multiline, objectMarker) { } // `key value` on every line makes an object; anything else is a list of values. - const isObject = rows.every((row) => row.length === 2 && row[0].ref); + const isObject = rows.every( + (row) => row.length === 2 && nodeToKey(row[0]) !== undefined + ); if (isObject) { const result = {}; for (const row of rows) { - result[row[0].value] = nodeToValue(row[1]); + result[nodeToKey(row[0])] = nodeToValue(row[1]); } return result; } @@ -717,21 +881,53 @@ function markedObjectToValue(rows) { ); } const [row] = node.rows; - if (row.length !== 2 || !row[0].ref) { + if (row.length !== 2) { throw new SyntaxError( `an object marked '${OBJECT_MARKER}:' holds (key value) pairs, ` + `found a link of ${row.length} values` ); } - result[row[0].value] = nodeToValue(row[1]); + const key = nodeToKey(row[0]); + if (key === undefined) { + throw new SyntaxError( + `an object marked '${OBJECT_MARKER}:' holds (key value) pairs, ` + + 'found a pair whose key is not text' + ); + } + result[key] = nodeToValue(row[1]); } return result; } /** - * Recognise `(base64 "…")`, the individual marker for values that could not be - * written as text. A quoted `base64` key is an ordinary object key, not a marker. + * The key a node in key position spells: a reference is the key itself, and a + * marked link is the text its marker escapes, which is how a key holding a + * character the form cannot carry stays a key instead of turning its object into + * an array. + * @param {object} node - The node standing in key position + * @returns {string|undefined} The key, or undefined when the node is not one + */ +function nodeToKey(node) { + if (node.ref) { + return node.value; + } + if (node.object) { + return undefined; + } + try { + const marked = decodeMarkedValue(node.rows); + return marked === undefined ? undefined : marked.value; + } catch { + return undefined; + } +} + +/** + * Recognise a marked value: `(escaped "…")`, whose text is written as it is + * except for the percent-escaped characters this form cannot carry, and + * `(base64 "…")`, which versions up to 0.6.0 wrote and which is still read. A + * quoted marker is an ordinary object key, not a marker. * @param {Array>} rows - The rows of the link being decoded * @returns {{value: string}|undefined} The decoded string, wrapped so that an * empty result is still distinguishable from "not a marker" @@ -742,15 +938,64 @@ function decodeMarkedValue(rows) { } const [marker, payload] = rows[0]; - if (!marker.ref || marker.quoted || marker.value !== BASE64_MARKER) { + if (!marker.ref || marker.quoted) { return undefined; } if (!payload.ref || !payload.quoted) { return undefined; } - const decoded = Buffer.from(payload.value, 'base64').toString('utf-8'); - return { value: decoded }; + if (marker.value === ESCAPED_MARKER) { + return { value: unescape(payload.value) }; + } + + if (marker.value === BASE64_MARKER) { + return { value: Buffer.from(payload.value, 'base64').toString('utf-8') }; + } + + return undefined; +} + +/** + * Undo the percent-escaping of an `(escaped "…")` payload. Escapes stand for + * bytes, so a character outside ASCII is written as its UTF-8 bytes and read + * back from them. + * @param {string} payload - The escaped text + * @returns {string} The text the payload stands for + * @throws {SyntaxError} If an escape is truncated, malformed or not UTF-8 + */ +function unescape(payload) { + const chars = Array.from(payload); + const bytes = []; + let i = 0; + + while (i < chars.length) { + if (chars[i] !== '%') { + for (const byte of UTF8_ENCODER.encode(chars[i])) { + bytes.push(byte); + } + i += 1; + continue; + } + + const escape = chars.slice(i + 1, i + 3).join(''); + if (escape.length !== 2) { + throw new SyntaxError( + `truncated escape at character ${i} of an escaped value` + ); + } + if (!/^[0-9a-fA-F]{2}$/.test(escape)) { + throw new SyntaxError(`invalid escape '%${escape}' in an escaped value`); + } + bytes.push(Number.parseInt(escape, 16)); + i += 3; + } + + try { + return UTF8_DECODER.decode(Uint8Array.from(bytes)); + } catch { + throw new SyntaxError('invalid UTF-8 escaped value'); + } } /** diff --git a/js/tests/test_plain_text_values.test.js b/js/tests/test_plain_text_values.test.js new file mode 100644 index 0000000..0af90df --- /dev/null +++ b/js/tests/test_plain_text_values.test.js @@ -0,0 +1,171 @@ +/** + * Real text stays real text in both readable forms. + * + * Before issue #45 a single control character turned the whole string into + * base64: one newline in a log message hid the message, the stack trace and + * every word a reader would grep for. The readable forms now write the text as + * it is, and escape only the characters the form itself cannot carry. + */ + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { encode, encodeLine, decode, decodeLine } from '../src/index.js'; + +/** + * A record of the shape a log line actually holds. + * @param {string} text - The message + * @returns {object} The record + */ +function message(text) { + return { message: text }; +} + +/** + * How many times a piece of text occurs in another. + * @param {string} haystack - The text to search + * @param {string} needle - The text to count + * @returns {number} The number of occurrences + */ +function occurrences(haystack, needle) { + return haystack.split(needle).length - 1; +} + +// The reason for the issue: a log line holding a newline must stay greppable. +test('a multi-line string keeps its text in the indented form', () => { + const value = message('line one\nline two'); + const encoded = encode({ obj: value }); + + assert.equal(encoded, '(\n message "line one\nline two"\n)'); + assert.ok(!encoded.includes('base64'), encoded); + assert.ok(encoded.includes('line one'), encoded); + assert.ok(encoded.includes('line two'), encoded); + assert.deepEqual(decode({ notation: encoded }), value); +}); + +// On one line the record ends at the newline, so the newline -- and nothing +// else -- is escaped: the rest of the message stays as written. +test('only the newline is escaped in the single-line form', () => { + const value = message('line one\nline two'); + const line = encodeLine({ obj: value }); + + assert.equal(line, '(o: (message (escaped "line one%0Aline two")))'); + assert.ok(!line.includes('\n'), line); + assert.ok(!line.includes('base64'), line); + assert.deepEqual(decodeLine({ notation: line }), value); +}); + +// A tab is text a reader can see, so both forms keep it as it is. +test('a tab is written as a tab in both forms', () => { + const value = message('a\tb'); + + assert.equal(encode({ obj: value }), '(\n message "a\tb"\n)'); + assert.equal(encodeLine({ obj: value }), '(o: (message "a\tb"))'); + assert.deepEqual(decode({ notation: encode({ obj: value }) }), value); + assert.deepEqual(decodeLine({ notation: encodeLine({ obj: value }) }), value); +}); + +// A carriage return is the one whitespace character a text file rewrites on its +// own -- CRLF normalisation would change the value -- so it is escaped. +test('a carriage return is escaped so CRLF normalisation cannot rewrite it', () => { + const value = message('first\r\nsecond'); + const encoded = encode({ obj: value }); + + assert.equal(encoded, '(\n message (escaped "first%0D\nsecond")\n)'); + assert.deepEqual(decode({ notation: encoded }), value); +}); + +// The doubled-quote form desynchronises the notation's own parser, so a value +// holding both quote kinds is written with a run of delimiters instead. +test('a value holding both quote kinds uses the n-quote form', () => { + const value = message('both "kinds" of \'quotes\''); + const encoded = encode({ obj: value }); + + assert.ok(encoded.includes('"""both "kinds" of \'quotes\'"""'), encoded); + assert.ok(!encoded.includes('""kinds""'), encoded); + assert.deepEqual(decode({ notation: encoded }), value); +}); + +// A value that occurs twice is written twice: a shared reference would make a +// log line depend on another line, which a line-based reader cannot resolve. +test('a repeated value is written out every time', () => { + const value = { first: 'same', second: 'same', third: 'same' }; + + const encoded = encode({ obj: value }); + assert.equal(occurrences(encoded, '"same"'), 3, encoded); + assert.deepEqual(decode({ notation: encoded }), value); + + const line = encodeLine({ obj: value }); + assert.equal(occurrences(line, '"same"'), 3, line); + assert.deepEqual(decodeLine({ notation: line }), value); +}); + +// A key is escaped like any other text, and stays a key rather than turning the +// object it belongs to into an array. +test('a key holding a control character stays a key', () => { + const value = { 'a\u0000b': 1 }; + + assert.deepEqual(decode({ notation: encode({ obj: value }) }), value); + assert.deepEqual(decodeLine({ notation: encodeLine({ obj: value }) }), value); +}); + +// Documents written by earlier versions keep decoding. +test('the previous base64 marker still decodes', () => { + assert.deepEqual( + decode({ notation: '(\n message (base64 "bGluZTEKbGluZTI=")\n)' }), + message('line1\nline2') + ); +}); + +// Every value the readable forms write must read back unchanged, whatever +// quotes, newlines and control characters it holds. +test('every kind of text roundtrips through both forms', () => { + const texts = [ + '', + 'plain', + 'with spaces', + "it's", + 'he said "hello"', + 'both "kinds" of \'quotes\'', + '"leading quote', + 'trailing quote"', + 'a""b', + 'a"""b\'c', + '\'"', + '"\'', + 'line one\nline two', + 'trailing newline\n', + '\ttab', + 'carriage\rreturn', + 'null\u0000byte', + 'escape\u001b[0m', + 'next\u0085line', + 'unicode: 你好世界 🌍', + 'percent %0A not an escape', + '(parens) and: colons', + 'base64', + 'escaped', + 'o:', + ]; + + for (const text of texts) { + for (const value of [text, message(text), { [text]: text }, [text]]) { + const encoded = encode({ obj: value }); + assert.deepEqual( + decode({ notation: encoded }), + value, + `indented roundtrip failed for ${JSON.stringify(text)}: ${JSON.stringify(encoded)}` + ); + + const line = encodeLine({ obj: value }); + assert.ok( + !line.includes('\n'), + `${JSON.stringify(text)} broke the line: ${JSON.stringify(line)}` + ); + assert.deepEqual( + decodeLine({ notation: line }), + value, + `single-line roundtrip failed for ${JSON.stringify(text)}: ${JSON.stringify(line)}` + ); + } + } +}); diff --git a/js/tests/test_readable_conformance.test.js b/js/tests/test_readable_conformance.test.js index 8a976a1..6b4d636 100644 --- a/js/tests/test_readable_conformance.test.js +++ b/js/tests/test_readable_conformance.test.js @@ -31,7 +31,7 @@ const SPECIAL_FLOATS = new Map([ ['-Infinity', -Infinity], ]); -const { cases } = JSON.parse(readFileSync(FIXTURES, 'utf-8')); +const { cases, legacy } = JSON.parse(readFileSync(FIXTURES, 'utf-8')); /** * Turn a fixture value specification into a JavaScript value. @@ -144,3 +144,23 @@ for (const testCase of cases) { ); }); } + +for (const testCase of legacy) { + // Documents written before this format wrote text as text keep decoding, so + // upgrading a reader never loses a stored record. + test(`decode reads the document an earlier version wrote: ${testCase.name}`, () => { + assert.ok( + same(decode({ notation: testCase.text }), build(testCase.value)), + `${JSON.stringify(decode({ notation: testCase.text }))} != ${JSON.stringify(build(testCase.value))}` + ); + }); +} + +test('no shared document hides its text in base64', () => { + // The point of the change: an implementation may not reach for base64 while + // writing a readable document, whatever the text holds. + for (const testCase of cases) { + assert.ok(!testCase.text.includes('base64 "'), testCase.name); + assert.ok(!testCase.line.includes('base64 "'), testCase.name); + } +}); diff --git a/js/tests/test_single_line_format.test.js b/js/tests/test_single_line_format.test.js index 8490c67..7e9970b 100644 --- a/js/tests/test_single_line_format.test.js +++ b/js/tests/test_single_line_format.test.js @@ -76,8 +76,11 @@ test('a string holding a newline still fits on one line', () => { const line = encodeLine({ obj: value }); assert.equal( line, - '(o: (readable "still visible") (multiline (base64 "bGluZTEKbGluZTI=")))' + '(o: (readable "still visible") (multiline (escaped "line1%0Aline2")))' ); + // The escape covers the newline, not the string: the words around it stay + // greppable, and so does the rest of the record. + assert.ok(line.includes('line1') && line.includes('line2'), line); assert.ok(!/[\n\r]/.test(line), line); assert.deepEqual(decodeLine({ notation: line }), value); }); diff --git a/python/README.md b/python/README.md index 2539018..25520da 100644 --- a/python/README.md +++ b/python/README.md @@ -17,7 +17,7 @@ A Python library to encode/decode objects to/from Links Notation format. This li - Collections: `list`, `dict` - Special float values: `NaN`, `Infinity`, `-Infinity` - **Object Identity**: Shared references and circular references are preserved by the compact format via object ids -- **Full Unicode**: Strings are written as text; only a value that cannot be written as text (one holding control characters) is base64-encoded, and it is marked individually as `(base64 "…")` +- **Full Unicode**: Strings are always written as text — a newline stays a newline and a tab stays a tab, so every word stays greppable; only the characters a form cannot carry are percent-escaped, in a value marked individually as `(escaped "…")` - **Opt-in Tracing**: Set `LINO_CODEC_DEBUG=1` to trace encoding and decoding, the same way in every language - **Simple API**: Easy-to-use `encode()` and `decode()` functions @@ -190,9 +190,16 @@ list: - Numbers, `true`, `false` and `null` are bare, so types survive a round trip - `NaN`, `Infinity` and `-Infinity` are written as such - An empty list is `()`; an empty dict is `(` + newline + `)` -- A value that cannot be written as text (one containing control characters) is - base64-encoded on its own and marked as `(base64 "bGluZTEKbGluZTI=")`; - everything around it stays readable +- A string is written as text whatever it holds: a newline stays a newline and + a tab stays a tab, so every word stays greppable +- A string containing the quote delimiter is written between a run of at least + three of them — `"""say "hi""""` — rather than by doubling the quote +- Only the characters this form cannot carry — a carriage return and the + remaining control characters — are percent-escaped, in a value marked on its + own as `(escaped "first%0D")`; everything around it stays readable, and + `(base64 "…")` written by earlier versions is still decoded +- A value that occurs more than once is written out every time: a shared + reference would make one record depend on another ### Single-line format (`encode_line`) @@ -219,7 +226,8 @@ The previous single-line form, kept for compatibility and for the object graphs the readable tree cannot express (shared and circular references): - Basic types carry a type marker: `(int 42)`, `(str aGVsbG8=)`, `(bool true)` -- Strings are base64-encoded to handle special characters and newlines +- Strings are base64-encoded here, and only here: this is the one form that + asks for it by name, and `encode()` never reaches for it - Shared / cyclic collections are defined inline with a self-reference id, e.g. `(obj_0: list (int 1) (int 2) ...)`; a self-referencing dict `{"self": obj}` encodes as `(obj_0: dict ((str c2VsZg==) obj_0))`. See diff --git a/python/changelog.d/20260827_120000_issue_45_plain_text_values.md b/python/changelog.d/20260827_120000_issue_45_plain_text_values.md new file mode 100644 index 0000000..b5fe803 --- /dev/null +++ b/python/changelog.d/20260827_120000_issue_45_plain_text_values.md @@ -0,0 +1,27 @@ +### Changed + +- `encode` and `encode_line` never reach for base64. A single control character + used to turn a whole string into base64, so a log message holding one newline + hid its own text: the message, the stack trace and every word a reader would + grep for. Both readable forms now write the text as it is and escape only what + the form itself cannot carry — the newline on a single line, the carriage + return everywhere, and the remaining control characters — in a value marked + `(escaped "line one%0Aline two")` whose payload is percent-escaped, so even the + escaped part stays readable. base64 is reachable only through + `encode_compact()` / `encode_obfuscated()`, which say so by name. See + [issue #45](https://github.com/link-foundation/lino-objects-codec/issues/45). +- A string containing the quote delimiter is written between a run of at least + three of them — `"""say "hi""""` — instead of by doubling the quote, which + desynchronises the notation's own parser. + +### Added + +- `ESCAPED_MARKER`, the `escaped` link id that marks a percent-escaped value. + +### Fixed + +- A key holding a control character stays a key. It used to be written as + `(base64 "…")` in key position and read back as a list element, so + `{"a\nb": "a\nb"}` decoded to `["a\nb", "a\nb"]`. +- `(base64 "…")` is still decoded, so every document written by an earlier + version keeps reading; the shared fixtures pin this in a `legacy` section. diff --git a/python/src/link_notation_objects_codec/__init__.py b/python/src/link_notation_objects_codec/__init__.py index 9a40c58..8b2a6a4 100644 --- a/python/src/link_notation_objects_codec/__init__.py +++ b/python/src/link_notation_objects_codec/__init__.py @@ -34,6 +34,7 @@ from .readable import ( BASE64_MARKER, DEFAULT_INDENT, + ESCAPED_MARKER, OBJECT_MARKER, CircularReferenceError, ReadableFormatError, @@ -61,6 +62,7 @@ "parse_indented", "DEFAULT_INDENT", "BASE64_MARKER", + "ESCAPED_MARKER", "OBJECT_MARKER", "ReadableFormatError", "CircularReferenceError", diff --git a/python/src/link_notation_objects_codec/readable.py b/python/src/link_notation_objects_codec/readable.py index 16f1898..3a4186c 100644 --- a/python/src/link_notation_objects_codec/readable.py +++ b/python/src/link_notation_objects_codec/readable.py @@ -31,17 +31,23 @@ ============================== ========================================== ``dict`` ``( )`` with one ``key value`` pair per line ``list`` / ``tuple`` ``( )`` with one value per line -``str`` quoted, never encoded +``str`` quoted text, never base64 ``int`` / ``float`` / ``bool`` / ``None`` bare, so the type survives the round trip ============================== ========================================== Empty containers keep their type: an empty list is ``()`` on one line, while an empty dict is written as ``(`` and ``)`` on two lines. -Only values that cannot be written as plain text are encoded: strings holding -control characters (including newlines and tabs, which line-based tooling and -CRLF normalisation would corrupt) are marked individually as ``(base64 "...")`` -instead of encoding the whole document. +Text is written as text. A string keeps every character a reader would grep +for, including newlines and tabs, and is quoted with a run of delimiters -- +``\"\"\"say \"hi\"\"\"\"`` -- when it holds the delimiter itself. Only the characters a +form cannot carry are escaped, and only they: the value is then written as +``(escaped "...")``, where ``%XX`` stands for one escaped byte. The indented form +escapes the carriage return, which CRLF normalisation would otherwise rewrite, +and the other control characters; the single-line form escapes the newline as +well, because there a record ends at the end of the line. Nothing else is +encoded: base64 lives in :func:`link_notation_objects_codec.encode_compact`, +which a caller asks for by name. Single-line form ---------------- @@ -86,9 +92,18 @@ #: Default indentation used by :func:`encode`. DEFAULT_INDENT = " " -#: Marker used for values that cannot be represented as plain text. +#: Marker of a base64 payload, written by +#: :func:`link_notation_objects_codec.encode_compact` and by versions up to 0.6.0 +#: of the readable form, which is still read back. BASE64_MARKER = "base64" +#: Marker of a string whose unwritable characters are percent-escaped. +#: +#: It reads as ``(escaped "line one%0Aline two")``. Only those characters change; +#: the rest of the text is written as it is, so the value stays readable and +#: greppable. +ESCAPED_MARKER = "escaped" + #: Link id naming a dict in the single-line form, written as ``(o: ...)``. OBJECT_MARKER = "o" @@ -96,7 +111,16 @@ _QUOTE_CHARS = ("'", '"', "`") #: Characters that force an object key to be quoted. -_KEY_NEEDS_QUOTES = re.compile(r"[\s()':`\"]") +_KEY_NEEDS_QUOTES = re.compile(r"[\s()':`\"\x00-\x1f\x7f-\x9f]") + +#: The indented form, where a value may span several lines. +_FORM_INDENTED = "indented" + +#: The single-line form, where a record ends at the end of the line. +_FORM_LINE = "line" + +#: The two hexadecimal digits of a percent escape. +_HEX_ESCAPE = re.compile(r"^[0-9a-fA-F]{2}$") _INTEGER_PATTERN = re.compile(r"^[+-]?\d+$") _FLOAT_PATTERN = re.compile(r"^[+-]?(\d+\.?\d*|\.\d+)([eE][+-]?\d+)?$") @@ -220,7 +244,7 @@ def _write_value(value: Any, indent: str, level: int, out: list[str], path: set[ def write_pair(pair: tuple[Any, Any]) -> None: key, child = pair - out.append(_format_key(key)) + out.append(_format_key(key, _FORM_INDENTED)) out.append(" ") _write_value(child, indent, level + 1, out, path) @@ -237,7 +261,7 @@ def write_item(item: Any) -> None: _write_rows(items_seq, indent, level, out, write_item) return - out.append(_format_scalar(value)) + out.append(_format_scalar(value, _FORM_INDENTED)) def _write_line_value(value: Any, out: list[str], path: set[int]) -> None: @@ -256,7 +280,7 @@ def _write_line_value(value: Any, out: list[str], path: set[int]) -> None: out.append(f"({OBJECT_MARKER}:") for key, child in items: - out.append(f" ({_format_key(key)} ") + out.append(f" ({_format_key(key, _FORM_LINE)} ") _write_line_value(child, out, path) out.append(")") out.append(")") @@ -272,7 +296,7 @@ def _write_line_value(value: Any, out: list[str], path: set[int]) -> None: out.append(")") return - out.append(_format_scalar(value)) + out.append(_format_scalar(value, _FORM_LINE)) @contextmanager @@ -321,7 +345,7 @@ def _push_indent(indent: str, level: int, out: list[str]) -> None: out.append(indent) -def _format_scalar(value: Any) -> str: +def _format_scalar(value: Any, form: str) -> str: """Format a scalar value. Strings are quoted, everything else stays bare so that its type is @@ -336,7 +360,7 @@ def _format_scalar(value: Any) -> str: if isinstance(value, float): return _format_float(value) if isinstance(value, str): - return _format_string(value) + return _format_string(value, form) if isinstance(value, (bytes, bytearray)): return f"({BASE64_MARKER} {_quote(base64.b64encode(bytes(value)).decode('ascii'))})" raise TypeError(f"Unsupported type: {type(value).__name__}") @@ -352,33 +376,90 @@ def _format_float(value: float) -> str: return repr(value) -def _format_string(value: str) -> str: - """Format a string: quoted plain text, or an individually marked payload.""" - if _needs_encoding(value): - payload = base64.b64encode(value.encode("utf-8")).decode("ascii") - return f"({BASE64_MARKER} {_quote(payload)})" - return _quote(value) +def _format_string(value: str, form: str) -> str: + """Format a string value. + + The text is written as text; when it holds characters this form cannot carry, + those characters -- and only those -- are percent-escaped and the value is + marked, so the rest of it stays readable and greppable. + """ + escaped = _escape_unwritable(value, form) + if escaped is None: + return _quote(value) + return f"({ESCAPED_MARKER} {_quote(escaped)})" + + +def _escape_unwritable(value: str, form: str) -> str | None: + """Percent-escape the characters this form cannot carry. + + Returns ``None`` when the text can be written as it is. ``%`` is escaped too, + so escaping is reversible. + """ + if not any(_is_unwritable(char, form) for char in value): + return None + + parts: list[str] = [] + for char in value: + if char == "%" or _is_unwritable(char, form): + parts.extend(f"%{byte:02X}" for byte in char.encode("utf-8")) + else: + parts.append(char) + return "".join(parts) -def _needs_encoding(value: str) -> bool: - """Whether a string has to be encoded rather than written as text. +def _is_unwritable(char: str, form: str) -> bool: + """Whether a character has to be escaped in this form. - A value can be written as text unless it contains control characters: - newlines break the line structure and CRLF normalisation would rewrite them. + A tab is text a reader can see, and so is a newline in the indented form, + where a value may span lines. A carriage return is escaped because CRLF + normalisation rewrites it, and the remaining control characters because they + are not text at all. """ - return any(unicodedata.category(char) == "Cc" for char in value) + if unicodedata.category(char) != "Cc": + return False + if char == "\t": + return False + if char == "\n": + return form == _FORM_LINE + return True def _quote(value: str) -> str: + """Quote a value so that both this reader and the notation's own parser read + it back unchanged. + + One delimiter is enough while the text holds none of that kind; when it holds + both kinds, a run of at least three opens the notation's n-quote form, where + the text is literal and only a run at least as long closes it. A value + starting with the delimiter would lengthen the opening run, so the other + delimiter is used for it. + """ if '"' not in value: return f'"{value}"' if "'" not in value: return f"'{value}'" - # Both quote styles are present: double the double quotes, as the parser expects. - return '"' + value.replace('"', '""') + '"' + + delimiter = "'" if value.startswith('"') else '"' + # A run of two delimiters is the empty value, so the n-quote form starts at + # three; beyond that the run only has to outrun the longest one inside. + run = delimiter * max(_longest_run(value, delimiter) + 1, 3) + return f"{run}{value}{run}" + + +def _longest_run(value: str, char: str) -> int: + """The length of the longest run of ``char`` in ``value``.""" + longest = 0 + current = 0 + for candidate in value: + if candidate == char: + current += 1 + longest = max(longest, current) + else: + current = 0 + return longest -def _format_key(key: Any) -> str: +def _format_key(key: Any, form: str) -> str: """Format an object key. Keys are bare when they read as plain identifiers. The readable form has string keys, like JSON: a non-string key is written as @@ -387,17 +468,17 @@ def _format_key(key: Any) -> str: if isinstance(key, str): text = key elif key is None or isinstance(key, (bool, int, float)): - text = _format_scalar(key) + text = _format_scalar(key, form) else: raise TypeError(f"Unsupported key type: {type(key).__name__}") plain = ( bool(text) and text != BASE64_MARKER - and not _needs_encoding(text) + and text != ESCAPED_MARKER and not _KEY_NEEDS_QUOTES.search(text) ) - return text if plain else _format_string(text) + return text if plain else _format_string(text, form) # === Decoding === @@ -480,7 +561,44 @@ def _tokenize(text: str) -> list[_Token]: def _read_quoted(text: str, start: int, quote_char: str) -> tuple[str, int]: - """Read a quoted reference, where a doubled quote character means a literal one.""" + """Read a quoted reference. + + The opening run of delimiters says how it is read, which is what the + notation's own parser does: + + * one delimiter -- the text is literal and a doubled delimiter is one literal + delimiter, which is how versions up to 0.6.0 wrote such values; + * two -- the empty value; + * three or more -- the n-quote form: the text is literal, and the value ends + at the first run at least as long, whose last delimiters close it. A longer + run therefore belongs to the text, so a value may end with a delimiter. + """ + opening = _run_length(text, start, quote_char) + + if opening == 2: + return "", start + 2 + + if opening == 1: + return _read_doubled_quoted(text, start, quote_char) + + length = len(text) + i = start + opening + while i < length: + if text[i] != quote_char: + i += 1 + continue + + run = _run_length(text, i, quote_char) + if run >= opening: + return text[start + opening : i + run - opening], i + run + i += run + + raise _unterminated_quote(start) + + +def _read_doubled_quoted(text: str, start: int, quote_char: str) -> tuple[str, int]: + """Read a value opened by a single delimiter, where a doubled delimiter means + one literal delimiter.""" parts: list[str] = [] i = start + 1 length = len(text) @@ -495,7 +613,19 @@ def _read_quoted(text: str, start: int, quote_char: str) -> tuple[str, int]: parts.append(text[i]) i += 1 - raise ReadableFormatError(f"unterminated quoted value starting at character {start}") + raise _unterminated_quote(start) + + +def _run_length(text: str, start: int, char: str) -> int: + """The length of the run of ``char`` that starts at ``start``.""" + i = start + while i < len(text) and text[i] == char: + i += 1 + return i - start + + +def _unterminated_quote(start: int) -> ReadableFormatError: + return ReadableFormatError(f"unterminated quoted value starting at character {start}") class _Cursor: @@ -604,12 +734,14 @@ def _rows_to_value(rows: list[list[_Node]], multiline: bool, object_marker: bool return [_node_to_value(node) for row in rows for node in row] # ``key value`` on every line makes a dict; anything else is a list of values. - is_dict = all(len(row) == 2 and row[0].is_ref for row in rows) + is_dict = all(len(row) == 2 and _node_to_key(row[0]) is not None for row in rows) if is_dict: result: dict[str, Any] = {} for row in rows: - result[row[0].value] = _node_to_value(row[1]) + key = _node_to_key(row[0]) + assert key is not None # checked by is_dict + result[key] = _node_to_value(row[1]) return result items: list[Any] = [] @@ -639,33 +771,63 @@ def _marked_object_to_value(rows: list[list[_Node]]) -> dict[str, Any]: f"found a link of {len(node.rows)} lines" ) row = node.rows[0] - if len(row) != 2 or not row[0].is_ref: + if len(row) != 2: raise ReadableFormatError( f"an object marked '{OBJECT_MARKER}:' holds (key value) pairs, " f"found a link of {len(row)} values" ) - result[row[0].value] = _node_to_value(row[1]) + key = _node_to_key(row[0]) + if key is None: + raise ReadableFormatError( + f"an object marked '{OBJECT_MARKER}:' holds (key value) pairs, " + "found a pair whose key is not text" + ) + result[key] = _node_to_value(row[1]) return result +def _node_to_key(node: _Node) -> str | None: + """The key a node in key position spells. + + A reference is the key itself, and a marked link is the text its marker + escapes, which is how a key holding a character the form cannot carry stays a + key instead of turning its dict into a list. + """ + if node.is_ref: + return node.value + if node.is_object: + return None + try: + marked = _decode_marked_value(node.rows) + except ReadableFormatError: + return None + return None if marked is None else marked[0] + + def _decode_marked_value(rows: list[list[_Node]]) -> tuple[str] | None: - """Recognise ``(base64 "...")``, the individual marker for values that could - not be written as text. + """Recognise a marked value. - A quoted ``base64`` key is an ordinary dict key, not a marker. The result is - wrapped in a tuple so that an empty string is still distinguishable from - "not a marker". + ``(escaped "...")`` holds text written as it is except for the + percent-escaped characters the form cannot carry; ``(base64 "...")`` is what + versions up to 0.6.0 wrote, and is still read. A quoted marker is an ordinary + dict key, not a marker. The result is wrapped in a tuple so that an empty + string is still distinguishable from "not a marker". """ if len(rows) != 1 or len(rows[0]) != 2: return None marker, payload = rows[0] - if not marker.is_ref or marker.quoted or marker.value != BASE64_MARKER: + if not marker.is_ref or marker.quoted: + return None + if marker.value not in (ESCAPED_MARKER, BASE64_MARKER): return None if not payload.is_ref or not payload.quoted: return None + if marker.value == ESCAPED_MARKER: + return (_unescape(payload.value),) + try: decoded = base64.b64decode(payload.value, validate=True).decode("utf-8") except Exception as error: # noqa: BLE001 - reported as a format error @@ -673,6 +835,36 @@ def _decode_marked_value(rows: list[list[_Node]]) -> tuple[str] | None: return (decoded,) +def _unescape(payload: str) -> str: + """Undo the percent-escaping of an ``(escaped "...")`` payload. + + Escapes stand for bytes, so a character outside ASCII is written as its UTF-8 + bytes and read back from them. + """ + out = bytearray() + i = 0 + length = len(payload) + + while i < length: + if payload[i] != "%": + out.extend(payload[i].encode("utf-8")) + i += 1 + continue + + escape = payload[i + 1 : i + 3] + if len(escape) != 2: + raise ReadableFormatError(f"truncated escape at character {i} of an escaped value") + if not _HEX_ESCAPE.match(escape): + raise ReadableFormatError(f"invalid escape '%{escape}' in an escaped value") + out.append(int(escape, 16)) + i += 3 + + try: + return out.decode("utf-8") + except UnicodeDecodeError as error: + raise ReadableFormatError(f"invalid UTF-8 escaped value: {error}") from error + + def _ref_to_value(value: str, quoted: bool) -> None | bool | int | float | str: """Convert a reference to a value. diff --git a/python/tests/test_plain_text_values.py b/python/tests/test_plain_text_values.py new file mode 100644 index 0000000..52f6782 --- /dev/null +++ b/python/tests/test_plain_text_values.py @@ -0,0 +1,143 @@ +"""Real text stays real text in both readable forms. + +Before issue #45 a single control character turned the whole string into base64: +one newline in a log message hid the message, the stack trace and every word a +reader would grep for. The readable forms now write the text as it is, and escape +only the characters the form itself cannot carry. +""" + +from typing import Any + +import pytest + +from link_notation_objects_codec import decode, decode_line, encode, encode_line + + +def _message(text: str) -> dict[str, Any]: + """A record of the shape a log line actually holds.""" + return {"message": text} + + +def test_a_multi_line_string_keeps_its_text_in_the_indented_form() -> None: + """The reason for the issue: a log line holding a newline must stay greppable.""" + value = _message("line one\nline two") + encoded = encode(value) + + assert encoded == '(\n message "line one\nline two"\n)' + assert "base64" not in encoded + assert "line one" in encoded + assert "line two" in encoded + assert decode(encoded) == value + + +def test_only_the_newline_is_escaped_in_the_single_line_form() -> None: + """On one line the record ends at the newline, so the newline -- and nothing + else -- is escaped: the rest of the message stays as written.""" + value = _message("line one\nline two") + line = encode_line(value) + + assert line == '(o: (message (escaped "line one%0Aline two")))' + assert "\n" not in line + assert "base64" not in line + assert decode_line(line) == value + + +def test_a_tab_is_written_as_a_tab_in_both_forms() -> None: + """A tab is text a reader can see, so both forms keep it as it is.""" + value = _message("a\tb") + + assert encode(value) == '(\n message "a\tb"\n)' + assert encode_line(value) == '(o: (message "a\tb"))' + assert decode(encode(value)) == value + assert decode_line(encode_line(value)) == value + + +def test_a_carriage_return_is_escaped_so_crlf_normalisation_cannot_rewrite_it() -> None: + """A carriage return is the one whitespace character a text file rewrites on + its own -- CRLF normalisation would change the value -- so it is escaped.""" + value = _message("first\r\nsecond") + encoded = encode(value) + + assert encoded == '(\n message (escaped "first%0D\nsecond")\n)' + assert decode(encoded) == value + + +def test_a_value_holding_both_quote_kinds_uses_the_n_quote_form() -> None: + """The doubled-quote form desynchronises the notation's own parser, so a value + holding both quote kinds is written with a run of delimiters instead.""" + value = _message("both \"kinds\" of 'quotes'") + encoded = encode(value) + + assert '"""both "kinds" of \'quotes\'"""' in encoded + assert '""kinds""' not in encoded + assert decode(encoded) == value + + +def test_a_repeated_value_is_written_out_every_time() -> None: + """A value that occurs twice is written twice: a shared reference would make a + log line depend on another line, which a line-based reader cannot resolve.""" + value = {"first": "same", "second": "same", "third": "same"} + + encoded = encode(value) + assert encoded.count('"same"') == 3 + assert decode(encoded) == value + + line = encode_line(value) + assert line.count('"same"') == 3 + assert decode_line(line) == value + + +def test_a_key_holding_a_control_character_stays_a_key() -> None: + """A key is escaped like any other text, and stays a key rather than turning + the dict it belongs to into a list.""" + value = {"a\x00b": 1} + + assert decode(encode(value)) == value + assert decode_line(encode_line(value)) == value + + +def test_the_previous_base64_marker_still_decodes() -> None: + """Documents written by earlier versions keep decoding.""" + assert decode('(\n message (base64 "bGluZTEKbGluZTI=")\n)') == _message("line1\nline2") + + +TEXTS = [ + "", + "plain", + "with spaces", + "it's", + 'he said "hello"', + "both \"kinds\" of 'quotes'", + '"leading quote', + 'trailing quote"', + 'a""b', + 'a"""b\'c', + "'\"", + "\"'", + "line one\nline two", + "trailing newline\n", + "\ttab", + "carriage\rreturn", + "null\x00byte", + "escape\x1b[0m", + "next\x85line", + "unicode: 你好世界 🌍", + "percent %0A not an escape", + "(parens) and: colons", + "base64", + "escaped", + "o:", +] + + +@pytest.mark.parametrize("text", TEXTS, ids=repr) +def test_every_kind_of_text_roundtrips_through_both_forms(text: str) -> None: + """Every value the readable forms write must read back unchanged, whatever + quotes, newlines and control characters it holds.""" + for value in (text, _message(text), {text: text}, [text]): + encoded = encode(value) + assert decode(encoded) == value, f"indented roundtrip failed: {encoded!r}" + + line = encode_line(value) + assert "\n" not in line, f"{text!r} broke the line: {line!r}" + assert decode_line(line) == value, f"single-line roundtrip failed: {line!r}" diff --git a/python/tests/test_readable_conformance.py b/python/tests/test_readable_conformance.py index 84af1ef..0e60f6c 100644 --- a/python/tests/test_readable_conformance.py +++ b/python/tests/test_readable_conformance.py @@ -23,9 +23,9 @@ _SPECIAL_FLOATS = {"NaN": math.nan, "Infinity": math.inf, "-Infinity": -math.inf} -def _load_cases() -> list[dict[str, Any]]: +def _section(key: str) -> list[dict[str, Any]]: document = json.loads(FIXTURES.read_text(encoding="utf-8")) - return document["cases"] + return document[key] def _build(spec: dict[str, Any]) -> Any: @@ -63,8 +63,9 @@ def _same(left: Any, right: Any) -> bool: return type(left) is type(right) and left == right -CASES = _load_cases() +CASES = _section("cases") ACTIVE = [case for case in CASES if LANGUAGE not in case.get("skip", {})] +LEGACY = _section("legacy") def test_every_case_is_either_active_or_skipped_with_a_reason() -> None: @@ -108,3 +109,20 @@ def test_decode_reads_the_shared_line_too(case: dict[str, Any]) -> None: expected = _build(case["value"]) decoded = decode(case["line"]) assert _same(decoded, expected), f"{decoded!r} != {expected!r}" + + +@pytest.mark.parametrize("case", LEGACY, ids=lambda case: case["name"]) +def test_decode_reads_the_document_an_earlier_version_wrote(case: dict[str, Any]) -> None: + """Documents written before this format wrote text as text keep decoding, so + upgrading a reader never loses a stored record.""" + expected = _build(case["value"]) + decoded = decode(case["text"]) + assert _same(decoded, expected), f"{decoded!r} != {expected!r}" + + +def test_no_shared_document_hides_its_text_in_base64() -> None: + """The point of the change: an implementation may not reach for base64 while + writing a readable document, whatever the text holds.""" + for case in CASES: + assert 'base64 "' not in case["text"], case["name"] + assert 'base64 "' not in case["line"], case["name"] diff --git a/python/tests/test_single_line_format.py b/python/tests/test_single_line_format.py index 02449fb..5b94dc4 100644 --- a/python/tests/test_single_line_format.py +++ b/python/tests/test_single_line_format.py @@ -82,9 +82,13 @@ def test_a_string_keeps_its_own_characters_on_one_line() -> None: def test_a_string_holding_a_newline_still_fits_on_one_line() -> None: + """A newline inside a string would end the record, so on one line -- and only + there -- it is escaped. The escape covers the newline, not the string: the + words around it stay greppable, and so does the rest of the record.""" value = {"readable": "still visible", "multiline": "line1\nline2"} line = encode_line(value) - assert line == '(o: (readable "still visible") (multiline (base64 "bGluZTEKbGluZTI=")))' + assert line == '(o: (readable "still visible") (multiline (escaped "line1%0Aline2")))' + assert "line1" in line and "line2" in line, line assert "\n" not in line, line assert decode_line(line) == value diff --git a/rust/README.md b/rust/README.md index f45abf9..fd0629b 100644 --- a/rust/README.md +++ b/rust/README.md @@ -33,7 +33,7 @@ lino-objects-codec = "0.1" - **Opt-in Tracing**: Set `LINO_CODEC_DEBUG=1` to trace encoding and decoding, the same way in every language - **Readable by Default**: `encode()` writes indented, plain-text Links Notation; keys and values stay legible and diffable - **One Record per Line**: `encode_line()` writes the same document on one line and `decode_line()` reads it back exactly, so an append-only log stays greppable, tailable and countable by `wc -l` -- **UTF-8 Support**: Full Unicode string support written as text; only values that cannot be written as text (control characters) are base64-encoded, and each is marked individually +- **UTF-8 Support**: Full Unicode string support written as text — a newline stays a newline and a tab stays a tab; only the characters a form cannot carry are percent-escaped, and each such value is marked individually - **Simple API**: Easy-to-use `encode()` and `decode()` functions ## Quick Start @@ -333,9 +333,16 @@ array: - Numbers, `true`, `false` and `null` are bare, so types survive a round trip - `NaN`, `Infinity` and `-Infinity` are written as such - An empty array is `()`; an empty object is `(` + newline + `)` -- A value that cannot be written as text (one containing control characters) is - base64-encoded on its own and marked as `(base64 "bGluZTEKbGluZTI=")`; - everything around it stays readable +- A string is written as text whatever it holds: a newline stays a newline and + a tab stays a tab, so every word stays greppable +- A string containing the quote delimiter is written between a run of at least + three of them — `"""say "hi""""` — rather than by doubling the quote +- Only the characters this form cannot carry — a carriage return and the + remaining control characters — are percent-escaped, in a value marked on its + own as `(escaped "first%0D")`; everything around it stays readable, and + `(base64 "…")` written by earlier versions is still decoded +- A value that occurs more than once is written out every time: a shared + reference would make one record depend on another Reading the format back requires `links-notation` 0.14 semantics, where a parenthesis opens a nested indentation context. @@ -346,7 +353,8 @@ The previous single-line form, kept for compatibility and for cases where size matters more than legibility: - Basic types: `(int 42)`, `(str aGVsbG8=)`, `(bool true)` -- Strings are base64-encoded to handle special characters and newlines +- Strings are base64-encoded here, and only here: this is the one form that + asks for it by name, and `encode()` never reaches for it - Arrays: `(array (int 1) (int 2) (int 3))` - Objects: `(object ((str a2V5) (int 42)) ...)` - Special floats: `(float NaN)`, `(float Infinity)`, `(float -Infinity)` diff --git a/rust/changelog.d/20260827_120000_issue_45_plain_text_values.md b/rust/changelog.d/20260827_120000_issue_45_plain_text_values.md new file mode 100644 index 0000000..853a7af --- /dev/null +++ b/rust/changelog.d/20260827_120000_issue_45_plain_text_values.md @@ -0,0 +1,34 @@ +--- +bump: minor +--- + +### Changed + +- `encode` and `encode_line` never reach for base64. A single control character + used to turn a whole string into base64, so a log message holding one newline + hid its own text: the message, the stack trace and every word a reader would + grep for. Both readable forms now write the text as it is and escape only what + the form itself cannot carry — the newline on a single line, the carriage + return everywhere, and the remaining control characters — in a value marked + `(escaped "line one%0Aline two")` whose payload is percent-escaped, so even the + escaped part stays readable. base64 is reachable only through + `encode_compact()` / `encode_obfuscated()`, which say so by name. See + [issue #45](https://github.com/link-foundation/lino-objects-codec/issues/45). +- A string containing the quote delimiter is written between a run of at least + three of them — `"""say "hi""""` — instead of by doubling the quote. The + doubled form desynchronises `links-notation`'s own parser; the run form reads + back unchanged, which `tests/plain_text_values.rs` checks with + `parse_lino_to_links`. + +### Added + +- `readable::ESCAPED_MARKER`, the `escaped` link id that marks a + percent-escaped value. + +### Fixed + +- A key holding a control character stays a key. It used to be written as + `(base64 "…")` in key position and read back as an array element, so + `{"a\nb": "a\nb"}` decoded to `["a\nb", "a\nb"]`. +- `(base64 "…")` is still decoded, so every document written up to 0.5.0 keeps + reading; the shared fixtures pin this in a `legacy` section. diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 4aec31b..5ab735a 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -12,9 +12,10 @@ //! - **Special Float Values**: Support for NaN, Infinity, -Infinity (which are not valid JSON) //! - **Circular References**: Detect and preserve circular references (via object IDs) //! - **Object Identity**: Maintain object identity for shared references -//! - **UTF-8 Support**: Full Unicode string support, written as text; only values that cannot be -//! represented as text (strings holding control characters) are base64-encoded, and they are -//! marked individually as `(base64 "…")` +//! - **UTF-8 Support**: Full Unicode string support, always written as text -- a string is never +//! base64-encoded, so every word of it stays greppable. A character the chosen form cannot hold +//! (a carriage return in either form, a newline on a single line) is percent-escaped, and only +//! that one value is marked, as `(escaped "line one%0Aline two")` //! - **Simple API**: Easy-to-use `encode()` and `decode()` functions //! //! # Example @@ -55,7 +56,7 @@ use std::fmt; pub mod debug; pub mod readable; -pub use readable::{BASE64_MARKER, DEFAULT_INDENT, OBJECT_MARKER}; +pub use readable::{BASE64_MARKER, DEFAULT_INDENT, ESCAPED_MARKER, OBJECT_MARKER}; /// Type identifiers used in the compact (base64) Links Notation format mod type_ids { diff --git a/rust/src/readable.rs b/rust/src/readable.rs index 8519756..3e607ff 100644 --- a/rust/src/readable.rs +++ b/rust/src/readable.rs @@ -36,10 +36,16 @@ //! Empty containers keep their type: an empty array is `()` on one line, while an //! empty object is written as `(` and `)` on two lines. //! -//! Only values that cannot be written as plain text are encoded: strings holding -//! control characters (including newlines and tabs, which line-based tooling and -//! CRLF normalisation would corrupt) are marked individually as -//! `(base64 "…")` instead of encoding the whole document. +//! Text is written as text. A string keeps every character a reader would grep +//! for, including newlines and tabs, and is quoted with a run of delimiters — +//! `"""say "hi""""` — when it holds the delimiter itself. Only the characters a +//! form cannot carry are escaped, and only they: the value is then written as +//! `(escaped "…")`, where `%XX` stands for one escaped byte. The indented form +//! escapes the carriage return, which CRLF normalisation would otherwise rewrite, +//! and the other control characters; the single-line form escapes the newline as +//! well, because there a record ends at the end of the line. Nothing else is +//! encoded: base64 lives in [`crate::encode_compact`], which a caller asks for by +//! name. //! //! # Single-line form //! @@ -76,9 +82,17 @@ use base64::{engine::general_purpose::STANDARD as BASE64, Engine}; /// Default indentation used by [`encode`]. pub const DEFAULT_INDENT: &str = " "; -/// Marker used for values that cannot be represented as plain text. +/// Marker of a base64 payload. Written by [`crate::encode_compact`] and by +/// versions up to 0.6.0 of the readable form, which is why it is still read. pub const BASE64_MARKER: &str = "base64"; +/// Marker of a string whose unwritable characters are percent-escaped. +/// +/// It reads as `(escaped "line one%0Aline two")`. Only those characters change; +/// the rest of the text is written as it is, so the value stays readable and +/// greppable. +pub const ESCAPED_MARKER: &str = "escaped"; + /// Link id naming an object in the single-line form, written as `(o: …)`. pub const OBJECT_MARKER: &str = "o"; @@ -137,6 +151,16 @@ pub fn decode(text: &str) -> Result { // === Encoding === +/// The line structure of the text being written, which is what decides whether a +/// newline can be written literally: in the indented form a value ends at its +/// closing quote, so it may span lines; in the single-line form a record ends at +/// the end of the line, so a newline inside a value would end the record. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Form { + Indented, + Line, +} + fn write_value(value: &LinoValue, indent: &str, level: usize, out: &mut String) { match value { LinoValue::Object(pairs) => { @@ -152,7 +176,7 @@ fn write_value(value: &LinoValue, indent: &str, level: usize, out: &mut String) for (key, child) in pairs { out.push('\n'); push_indent(indent, level + 1, out); - out.push_str(&format_key(key)); + out.push_str(&format_key(key, Form::Indented)); out.push(' '); write_value(child, indent, level + 1, out); } @@ -178,7 +202,7 @@ fn write_value(value: &LinoValue, indent: &str, level: usize, out: &mut String) out.push(')'); } - scalar => out.push_str(&format_scalar(scalar)), + scalar => out.push_str(&format_scalar(scalar, Form::Indented)), } } @@ -200,7 +224,7 @@ fn write_line_value(value: &LinoValue, out: &mut String) { out.push(':'); for (key, child) in pairs { out.push_str(" ("); - out.push_str(&format_key(key)); + out.push_str(&format_key(key, Form::Line)); out.push(' '); write_line_value(child, out); out.push(')'); @@ -219,7 +243,7 @@ fn write_line_value(value: &LinoValue, out: &mut String) { out.push(')'); } - scalar => out.push_str(&format_scalar(scalar)), + scalar => out.push_str(&format_scalar(scalar, Form::Line)), } } @@ -231,13 +255,13 @@ fn push_indent(indent: &str, level: usize, out: &mut String) { /// Format a scalar value. Strings are quoted, everything else stays bare so that /// its type is recoverable when reading the document back. -fn format_scalar(value: &LinoValue) -> String { +fn format_scalar(value: &LinoValue, form: Form) -> String { match value { LinoValue::Null => "null".to_string(), LinoValue::Bool(b) => b.to_string(), LinoValue::Int(i) => i.to_string(), LinoValue::Float(f) => format_float(*f), - LinoValue::String(s) => format_string(s), + LinoValue::String(s) => format_string(s, form), // Containers are handled by write_value. LinoValue::Array(_) | LinoValue::Object(_) => String::new(), } @@ -259,52 +283,109 @@ fn format_float(f: f64) -> String { } } -/// Format a string value: quoted plain text, or an individually marked -/// base64 payload when the text cannot be written literally. -fn format_string(value: &str) -> String { - if needs_encoding(value) { - return format!( - "({} {})", - BASE64_MARKER, - quote(&BASE64.encode(value.as_bytes())) - ); +/// Format a string value. The text is written as text; when it holds characters +/// this form cannot carry, those characters — and only those — are percent-escaped +/// and the value is marked, so the rest of it stays readable and greppable. +fn format_string(value: &str, form: Form) -> String { + match escape_unwritable(value, form) { + Some(escaped) => format!("({} {})", ESCAPED_MARKER, quote(&escaped)), + None => quote(value), + } +} + +/// Percent-escape the characters this form cannot carry, or `None` when the text +/// can be written as it is. `%` is escaped too, so escaping is reversible. +fn escape_unwritable(value: &str, form: Form) -> Option { + if !value.chars().any(|c| is_unwritable(c, form)) { + return None; + } + + let mut out = String::with_capacity(value.len()); + let mut buffer = [0u8; 4]; + for c in value.chars() { + if c == '%' || is_unwritable(c, form) { + for byte in c.encode_utf8(&mut buffer).as_bytes() { + out.push('%'); + out.push(hex_digit(byte >> 4)); + out.push(hex_digit(byte & 0xf)); + } + } else { + out.push(c); + } } - quote(value) + Some(out) } -/// A value can be written as text unless it contains control characters: -/// newlines break the line structure and CRLF normalisation would rewrite them. -fn needs_encoding(value: &str) -> bool { - value.chars().any(char::is_control) +/// One upper-case hexadecimal digit of a percent escape. +fn hex_digit(value: u8) -> char { + char::from_digit(u32::from(value), 16).map_or('0', |digit| digit.to_ascii_uppercase()) } -fn quote(value: &str) -> String { - let has_double = value.contains('"'); - let has_single = value.contains('\''); +/// Whether a character has to be escaped in this form. A tab is text a reader can +/// see, and so is a newline in the indented form, where a value may span lines. +/// A carriage return is escaped because CRLF normalisation rewrites it, and the +/// remaining control characters because they are not text at all. +fn is_unwritable(c: char, form: Form) -> bool { + if !c.is_control() { + return false; + } + match c { + '\t' => false, + '\n' => form == Form::Line, + _ => true, + } +} - if !has_double { +/// Quote a value so that both this reader and the notation's own parser read it +/// back unchanged. One delimiter is enough while the text holds none of that +/// kind; when it holds both kinds, a run of at least three opens the notation's +/// n-quote form, where the text is literal and only a run at least as long closes +/// it. A value starting with the delimiter would lengthen the opening run, so the +/// other delimiter is used for it. +fn quote(value: &str) -> String { + if !value.contains('"') { return format!("\"{}\"", value); } - if !has_single { + if !value.contains('\'') { return format!("'{}'", value); } - // Both quote styles are present: double the double quotes, as the parser expects. - format!("\"{}\"", value.replace('"', "\"\"")) + + let delimiter = if value.starts_with('"') { '\'' } else { '"' }; + // A run of two delimiters is the empty value, so the n-quote form starts at + // three; beyond that the run only has to outrun the longest one inside. + let count = (longest_run(value, delimiter) + 1).max(3); + let run: String = std::iter::repeat(delimiter).take(count).collect(); + format!("{run}{value}{run}") +} + +/// The length of the longest run of `c` in `value`. +fn longest_run(value: &str, c: char) -> usize { + let mut longest = 0; + let mut current = 0; + for candidate in value.chars() { + if candidate == c { + current += 1; + longest = longest.max(current); + } else { + current = 0; + } + } + longest } /// Format an object key. Keys are bare when they read as plain identifiers. -fn format_key(key: &str) -> String { +fn format_key(key: &str, form: Form) -> String { let plain = !key.is_empty() && key != BASE64_MARKER - && !needs_encoding(key) - && !key - .chars() - .any(|c| c.is_whitespace() || matches!(c, '(' | ')' | '\'' | '"' | ':' | '`')); + && key != ESCAPED_MARKER + && !key.chars().any(|c| { + c.is_whitespace() || c.is_control() || matches!(c, '(' | ')' | '\'' | '"' | ':' | '`') + }); if plain { key.to_string() } else { - format_string(key) + format_string(key, form) } } @@ -378,32 +459,74 @@ fn tokenize(text: &str) -> Result, CodecError> { Ok(tokens) } -/// Read a quoted reference, where a doubled quote character means a literal one. +/// Read a quoted reference. The opening run of delimiters says how it is read, +/// which is what the notation's own parser does: +/// +/// * one delimiter — the text is literal and a doubled delimiter is one literal +/// delimiter, which is how versions up to 0.6.0 wrote such values; +/// * two — the empty value; +/// * three or more — the n-quote form: the text is literal, and the value ends at +/// the first run at least as long, whose last delimiters close it. A longer run +/// therefore belongs to the text, so a value may end with a delimiter. fn read_quoted( chars: &[char], start: usize, quote_char: char, ) -> Result<(String, usize), CodecError> { - let mut value = String::new(); - let mut i = start + 1; + let opening = run_length(chars, start, quote_char); - while i < chars.len() { - if chars[i] == quote_char { - if chars.get(i + 1) == Some("e_char) { - value.push(quote_char); - i += 2; - continue; + if opening == 2 { + return Ok((String::new(), start + 2)); + } + + if opening == 1 { + let mut value = String::new(); + let mut i = start + 1; + + while i < chars.len() { + if chars[i] == quote_char { + if chars.get(i + 1) == Some("e_char) { + value.push(quote_char); + i += 2; + continue; + } + return Ok((value, i + 1)); } - return Ok((value, i + 1)); + value.push(chars[i]); + i += 1; + } + + return Err(unterminated_quote(start)); + } + + let mut i = start + opening; + while i < chars.len() { + if chars[i] != quote_char { + i += 1; + continue; } - value.push(chars[i]); - i += 1; + + let run = run_length(chars, i, quote_char); + if run >= opening { + let value = chars[start + opening..i + run - opening].iter().collect(); + return Ok((value, i + run)); + } + i += run; } - Err(CodecError::ParseError(format!( + Err(unterminated_quote(start)) +} + +/// The length of the run of `c` that starts at `start`. +fn run_length(chars: &[char], start: usize, c: char) -> usize { + chars[start..].iter().take_while(|&&x| x == c).count() +} + +fn unterminated_quote(start: usize) -> CodecError { + CodecError::ParseError(format!( "unterminated quoted value starting at character {}", start - ))) + )) } struct Cursor { @@ -546,15 +669,15 @@ fn rows_to_value( // `key value` on every line makes an object; anything else is a list of values. let is_object = rows .iter() - .all(|row| row.len() == 2 && matches!(row[0], Node::Ref { .. })); + .all(|row| row.len() == 2 && node_to_key(&row[0]).is_some()); if is_object { let mut pairs = Vec::with_capacity(rows.len()); for row in rows { - let Node::Ref { value: key, .. } = &row[0] else { + let Some(key) = node_to_key(&row[0]) else { unreachable!("checked by is_object") }; - pairs.push((key.clone(), node_to_value(&row[1])?)); + pairs.push((key, node_to_value(&row[1])?)); } return Ok(LinoValue::Object(pairs)); } @@ -594,7 +717,7 @@ fn marked_object_to_value(rows: &[Vec]) -> Result { ))); }; - let [Node::Ref { value: key, .. }, value] = row.as_slice() else { + let [key_node, value] = row.as_slice() else { return Err(CodecError::ParseError(format!( "an object marked '{}:' holds (key value) pairs, found a link of {} values", OBJECT_MARKER, @@ -602,14 +725,42 @@ fn marked_object_to_value(rows: &[Vec]) -> Result { ))); }; - pairs.push((key.clone(), node_to_value(value)?)); + let Some(key) = node_to_key(key_node) else { + return Err(CodecError::ParseError(format!( + "an object marked '{}:' holds (key value) pairs, found a pair whose key is not text", + OBJECT_MARKER + ))); + }; + + pairs.push((key, node_to_value(value)?)); } Ok(LinoValue::Object(pairs)) } -/// Recognise `(base64 "…")`, the individual marker for values that could not be -/// written as text. A quoted `base64` key is an ordinary object key, not a marker. +/// The key a node in key position spells: a reference is the key itself, and a +/// marked link is the text its marker escapes, which is how a key holding a +/// character the form cannot carry stays a key instead of turning its object into +/// an array. +fn node_to_key(node: &Node) -> Option { + match node { + Node::Ref { value, .. } => Some(value.clone()), + Node::Link { + rows, + object: false, + .. + } => match decode_marked_value(rows)? { + Ok(LinoValue::String(key)) => Some(key), + _ => None, + }, + Node::Link { .. } => None, + } +} + +/// Recognise a marked value: `(escaped "…")`, whose text is written as it is +/// except for the percent-escaped characters this form cannot carry, and +/// `(base64 "…")`, which versions up to 0.6.0 wrote and which is still read. A +/// quoted marker is an ordinary object key, not a marker. fn decode_marked_value(rows: &[Vec]) -> Option> { if rows.len() != 1 || rows[0].len() != 2 { return None; @@ -622,9 +773,6 @@ fn decode_marked_value(rows: &[Vec]) -> Option]) -> Option Result { + let bytes = payload.as_bytes(); + let mut out: Vec = Vec::with_capacity(bytes.len()); + let mut i = 0; + + while i < bytes.len() { + if bytes[i] != b'%' { + out.push(bytes[i]); + i += 1; + continue; + } + + let escape = payload.get(i + 1..i + 3).ok_or_else(|| { + CodecError::DecodeError(format!( + "truncated escape at character {} of an escaped value", + i + )) + })?; + let byte = u8::from_str_radix(escape, 16).map_err(|_| { + CodecError::DecodeError(format!("invalid escape '%{}' in an escaped value", escape)) + })?; + out.push(byte); + i += 3; + } + + String::from_utf8(out) + .map_err(|e| CodecError::DecodeError(format!("invalid UTF-8 escaped value: {}", e))) } /// Convert a reference to a value. Quoted references are always strings; bare @@ -733,21 +921,98 @@ mod tests { } #[test] - fn control_characters_are_marked_individually() { + fn text_is_written_as_text_and_only_the_rest_is_escaped() { let value = LinoValue::object([ ("plain", LinoValue::String("visible".to_string())), ("raw", LinoValue::String("line1\nline2".to_string())), + ("returned", LinoValue::String("line1\r\nline2".to_string())), ]); let text = encode(&value, DEFAULT_INDENT); assert!(text.contains("plain \"visible\""), "{}", text); + assert!(text.contains("raw \"line1\nline2\""), "{}", text); assert!( - text.contains("raw (base64 \"bGluZTEKbGluZTI=\")"), + text.contains("returned (escaped \"line1%0D\nline2\")"), "{}", text ); + assert!(!text.contains("base64"), "{}", text); assert_eq!(roundtrip(&value), value); } + /// The readable form is Links Notation, so the notation's own parser has to + /// read every document it writes -- quotes, newlines and escapes included. + #[test] + fn every_written_document_parses_as_links_notation() { + let texts = [ + "plain", + "it's", + "he said \"hello\"", + "both \"kinds\" of 'quotes'", + "a\"\"b'c", + "a\"\"\"b'c", + "trailing quote\"'", + "'\"", + "line one\nline two", + "a\tb", + "null\u{0}byte", + "unicode: 你好世界 🌍", + ]; + + for text in texts { + let value = LinoValue::object([ + ("message", LinoValue::String(text.to_string())), + ("level", LinoValue::String("info".to_string())), + ]); + + for document in [encode(&value, DEFAULT_INDENT), encode_line(&value)] { + assert!( + links_notation::parse_lino(&document).is_ok(), + "links-notation rejected {:?}", + document + ); + } + } + } + + /// The value the notation's parser reads back has to be the value written, + /// not merely something that parses. + #[test] + fn links_notation_reads_back_the_text_that_was_written() { + for text in [ + "he said \"hello\"", + "both \"kinds\" of 'quotes'", + "a\"\"b'c", + "line one\nline two", + ] { + let document = encode( + &LinoValue::object([("message", LinoValue::String(text.to_string()))]), + DEFAULT_INDENT, + ); + let parsed = links_notation::parse_lino(&document) + .unwrap_or_else(|e| panic!("links-notation rejected {:?}: {}", document, e)); + + let mut refs = Vec::new(); + collect_refs(&parsed, &mut refs); + assert!( + refs.contains(&text.to_string()), + "links-notation read {:?} out of {:?}", + refs, + document + ); + } + } + + fn collect_refs(node: &links_notation::LiNo, out: &mut Vec) { + match node { + links_notation::LiNo::Ref(value) => out.push(value.clone()), + links_notation::LiNo::Link { values, .. } => { + for value in values { + collect_refs(value, out); + } + } + } + } + #[test] fn custom_indent_is_used() { let value = LinoValue::object([("a", LinoValue::Int(1))]); @@ -869,6 +1134,25 @@ mod tests { fn unterminated_input_is_an_error() { assert!(decode("(\n a 1\n").is_err()); assert!(decode("(\n a \"unterminated\n").is_err()); + assert!(decode("(\n a \"\"\"unterminated\n").is_err()); assert!(decode("a 1)").is_err()); } + + /// The three ways a run of delimiters reads, which is what keeps documents + /// written by earlier versions decoding as they did. + #[test] + fn a_run_of_delimiters_says_how_the_value_is_read() { + // One delimiter: a doubled delimiter is one literal delimiter. + assert_eq!( + decode("\"both \"\"kinds\"\" of 'quotes'\"").unwrap(), + LinoValue::String("both \"kinds\" of 'quotes'".to_string()) + ); + // Two: the empty value. + assert_eq!(decode("\"\"").unwrap(), LinoValue::String(String::new())); + // Three or more: the text is literal, and the last delimiters close it. + assert_eq!( + decode("\"\"\"say \"hi\"\"\"\"").unwrap(), + LinoValue::String("say \"hi\"".to_string()) + ); + } } diff --git a/rust/tests/documented_examples.rs b/rust/tests/documented_examples.rs index 30d1e04..c5daf00 100644 --- a/rust/tests/documented_examples.rs +++ b/rust/tests/documented_examples.rs @@ -1,6 +1,6 @@ //! Checks that the snippets shown in `README.md` and the crate docs stay true. -use lino_objects_codec::{decode, encode, encode_compact, LinoValue}; +use lino_objects_codec::{decode, encode, encode_compact, encode_line, LinoValue}; #[test] fn scalars_are_written_as_documented() { @@ -34,11 +34,10 @@ fn empty_containers_are_written_as_documented() { } #[test] -fn unrepresentable_values_use_the_documented_marker() { - assert_eq!( - encode(&LinoValue::String("line1\nline2".into())), - "(base64 \"bGluZTEKbGluZTI=\")" - ); +fn a_newline_stays_a_newline_and_only_a_line_escapes_it() { + let value = LinoValue::String("line1\nline2".into()); + assert_eq!(encode(&value), "\"line1\nline2\""); + assert_eq!(encode_line(&value), "(escaped \"line1%0Aline2\")"); } #[test] diff --git a/rust/tests/plain_text_values.rs b/rust/tests/plain_text_values.rs new file mode 100644 index 0000000..ee0a91a --- /dev/null +++ b/rust/tests/plain_text_values.rs @@ -0,0 +1,209 @@ +//! Real text stays real text in both readable forms. +//! +//! Before issue #45 a single control character turned the whole string into +//! base64: one newline in a log message hid the message, the stack trace and +//! every word a reader would grep for. The readable forms now write the text as +//! it is, and escape only the characters the form itself cannot carry. + +use lino_objects_codec::{decode, decode_line, encode, encode_line, LinoValue}; + +fn message(text: &str) -> LinoValue { + LinoValue::object([("message", LinoValue::String(text.to_string()))]) +} + +/// The reason for the issue: a log line holding a newline must stay greppable. +#[test] +fn a_multi_line_string_keeps_its_text_in_the_indented_form() { + let value = message("line one\nline two"); + let encoded = encode(&value); + + assert_eq!(encoded, "(\n message \"line one\nline two\"\n)"); + assert!(!encoded.contains("base64"), "{encoded}"); + assert!(encoded.contains("line one"), "{encoded}"); + assert!(encoded.contains("line two"), "{encoded}"); + assert_eq!(decode(&encoded).unwrap(), value); +} + +/// On one line the record ends at the newline, so the newline -- and nothing +/// else -- is escaped: the rest of the message stays as written. +#[test] +fn only_the_newline_is_escaped_in_the_single_line_form() { + let value = message("line one\nline two"); + let line = encode_line(&value); + + assert_eq!(line, r#"(o: (message (escaped "line one%0Aline two")))"#); + assert!(!line.contains('\n'), "{line}"); + assert!(!line.contains("base64"), "{line}"); + assert_eq!(decode_line(&line).unwrap(), value); +} + +/// A tab is text a reader can see, so both forms keep it as it is. +#[test] +fn a_tab_is_written_as_a_tab_in_both_forms() { + let value = message("a\tb"); + + assert_eq!(encode(&value), "(\n message \"a\tb\"\n)"); + assert_eq!(encode_line(&value), "(o: (message \"a\tb\"))"); + assert_eq!(decode(&encode(&value)).unwrap(), value); + assert_eq!(decode_line(&encode_line(&value)).unwrap(), value); +} + +/// A carriage return is the one whitespace character a text file rewrites on its +/// own -- CRLF normalisation would change the value -- so it is escaped. +#[test] +fn a_carriage_return_is_escaped_so_crlf_normalisation_cannot_rewrite_it() { + let value = message("first\r\nsecond"); + let encoded = encode(&value); + + assert_eq!(encoded, "(\n message (escaped \"first%0D\nsecond\")\n)"); + assert_eq!(decode(&encoded).unwrap(), value); +} + +/// The doubled-quote form desynchronises the notation's own parser, so a value +/// holding both quote kinds is written with a run of delimiters instead. +#[test] +fn a_value_holding_both_quote_kinds_uses_the_n_quote_form() { + let value = message("both \"kinds\" of 'quotes'"); + let encoded = encode(&value); + + assert!( + encoded.contains("\"\"\"both \"kinds\" of 'quotes'\"\"\""), + "{encoded}" + ); + assert!(!encoded.contains("\"\"kinds\"\""), "{encoded}"); + assert_eq!(decode(&encoded).unwrap(), value); +} + +/// A value that occurs twice is written twice: a shared reference would make a +/// log line depend on another line, which a line-based reader cannot resolve. +#[test] +fn a_repeated_value_is_written_out_every_time() { + let repeated = LinoValue::String("same".to_string()); + let value = LinoValue::object([ + ("first", repeated.clone()), + ("second", repeated.clone()), + ("third", repeated), + ]); + + let encoded = encode(&value); + assert_eq!(encoded.matches("\"same\"").count(), 3, "{encoded}"); + assert_eq!(decode(&encoded).unwrap(), value); + + let line = encode_line(&value); + assert_eq!(line.matches("\"same\"").count(), 3, "{line}"); + assert_eq!(decode_line(&line).unwrap(), value); +} + +/// A key is escaped like any other text, and stays a key rather than turning the +/// object it belongs to into an array. +#[test] +fn a_key_holding_a_control_character_stays_a_key() { + let value = LinoValue::object([("a\u{0}b", LinoValue::Int(1))]); + + assert_eq!(decode(&encode(&value)).unwrap(), value); + assert_eq!(decode_line(&encode_line(&value)).unwrap(), value); +} + +/// Documents written by earlier versions keep decoding. +#[test] +fn the_previous_base64_marker_still_decodes() { + assert_eq!( + decode("(\n message (base64 \"bGluZTEKbGluZTI=\")\n)").unwrap(), + message("line1\nline2") + ); +} + +/// Every value the readable forms write must read back unchanged, whatever +/// quotes, newlines and control characters it holds. +#[test] +fn every_kind_of_text_roundtrips_through_both_forms() { + let texts = [ + "", + "plain", + "with spaces", + "it's", + "he said \"hello\"", + "both \"kinds\" of 'quotes'", + "\"leading quote", + "trailing quote\"", + "a\"\"b", + "a\"\"\"b'c", + "'\"", + "\"'", + "line one\nline two", + "trailing newline\n", + "\ttab", + "carriage\rreturn", + "null\u{0}byte", + "escape\u{1b}[0m", + "next\u{85}line", + "unicode: 你好世界 🌍", + "percent %0A not an escape", + "(parens) and: colons", + "base64", + "escaped", + "o:", + ]; + + for text in texts { + for value in [ + LinoValue::String(text.to_string()), + message(text), + LinoValue::object([(text, LinoValue::String(text.to_string()))]), + LinoValue::array([LinoValue::String(text.to_string())]), + ] { + let encoded = encode(&value); + assert_eq!( + decode(&encoded).unwrap(), + value, + "indented roundtrip failed for {text:?}: {encoded:?}" + ); + + let line = encode_line(&value); + assert!(!line.contains('\n'), "{text:?} broke the line: {line:?}"); + assert_eq!( + decode_line(&line).unwrap(), + value, + "single-line roundtrip failed for {text:?}: {line:?}" + ); + } + } +} + +/// The evidence the issue reports: `parse_lino` desynchronises on a doubled +/// quote, so 241 of 6,000 fuzzed values came back as something else. Whatever a +/// value holds, the notation's own parser must now read the encoder's output +/// back as exactly that value. +#[test] +fn the_notation_s_own_parser_reads_every_written_value_back() { + let texts = [ + "plain", + "with spaces", + "it's", + "he said \"hello\"", + "both \"kinds\" of 'quotes'", + "\"leading quote", + "trailing quote\"", + "a\"\"b", + "a\"\"\"b'c", + "'\"", + "\"'", + "unicode: 你好世界 🌍", + ]; + + for text in texts { + let encoded = encode(&LinoValue::String(text.to_string())); + let links = links_notation::parse_lino_to_links(&encoded) + .unwrap_or_else(|e| panic!("links-notation rejected {encoded:?}: {e:?}")); + + assert_eq!( + links.len(), + 1, + "links-notation read {encoded:?} as {links:?} instead of one value" + ); + let links_notation::LiNo::Ref(read) = &links[0] else { + panic!("links-notation read {encoded:?} as a link: {links:?}"); + }; + assert_eq!(read, text, "links-notation lost the text of {encoded:?}"); + } +} diff --git a/rust/tests/readable_conformance.rs b/rust/tests/readable_conformance.rs index 992c485..23e0f13 100644 --- a/rust/tests/readable_conformance.rs +++ b/rust/tests/readable_conformance.rs @@ -24,9 +24,18 @@ fn fixtures() -> Json { } fn cases() -> Vec { - match fixtures()["cases"].take() { + section("cases") +} + +/// Documents earlier versions wrote, which are read but never written again. +fn legacy() -> Vec { + section("legacy") +} + +fn section(key: &str) -> Vec { + match fixtures()[key].take() { Json::Array(cases) => cases, - other => panic!("`cases` must be an array, got {other}"), + other => panic!("`{key}` must be an array, got {other}"), } } @@ -295,3 +304,43 @@ fn the_plain_decoder_reads_every_shared_line() { failures.join("\n") ); } + +/// Documents written before this format wrote text as text keep decoding, so +/// upgrading a reader never loses a stored record. +#[test] +fn decodes_every_document_earlier_versions_wrote() { + let mut failures = Vec::new(); + let legacy = legacy(); + assert!(!legacy.is_empty(), "the fixtures must contain legacy cases"); + for case in legacy { + let expected = build(&case["value"]); + match decode(text(&case)) { + Ok(decoded) if same(&decoded, &expected) => {} + Ok(decoded) => failures.push(format!( + "{}: expected {:?}, got {:?}", + name(&case), + expected, + decoded + )), + Err(error) => failures.push(format!("{}: {error}", name(&case))), + } + } + assert!( + failures.is_empty(), + "legacy decoding mismatches:\n{}", + failures.join("\n") + ); +} + +/// The point of the change: an implementation may not reach for base64 while +/// writing a readable document, whatever the text holds. +#[test] +fn no_shared_document_hides_its_text_in_base64() { + for case in cases() { + assert!( + !text(&case).contains("base64 \"") && !line(&case).contains("base64 \""), + "case {} still marks a value with base64", + name(&case) + ); + } +} diff --git a/rust/tests/readable_format.rs b/rust/tests/readable_format.rs index 4a2feed..2a6445c 100644 --- a/rust/tests/readable_format.rs +++ b/rust/tests/readable_format.rs @@ -258,14 +258,22 @@ fn values_that_cannot_be_written_as_text_are_marked_individually() { ("readable", LinoValue::String("still visible".to_string())), ("multiline", LinoValue::String("line1\nline2".to_string())), ("tabbed", LinoValue::String("a\tb".to_string())), + ("returned", LinoValue::String("line1\rline2".to_string())), ]); let encoded = encode(&value); - // Only the values that need it are encoded; the rest stays readable. + // An indented document holds line breaks and tabs of its own, so only the + // carriage return -- which a line ending would rewrite -- is escaped, and + // only that one value is marked. assert!(encoded.contains("readable \"still visible\""), "{encoded}"); - assert!(encoded.contains("multiline (base64 \""), "{encoded}"); - assert!(encoded.contains("tabbed (base64 \""), "{encoded}"); + assert!(encoded.contains("multiline \"line1\nline2\""), "{encoded}"); + assert!(encoded.contains("tabbed \"a\tb\""), "{encoded}"); + assert!( + encoded.contains("returned (escaped \"line1%0Dline2\")"), + "{encoded}" + ); + assert!(!encoded.contains("base64"), "{encoded}"); assert_eq!(decode(&encoded).unwrap(), value); } diff --git a/rust/tests/single_line_format.rs b/rust/tests/single_line_format.rs index 489bdb3..5b171a1 100644 --- a/rust/tests/single_line_format.rs +++ b/rust/tests/single_line_format.rs @@ -90,9 +90,9 @@ fn a_string_keeps_its_own_characters_on_one_line() { assert_eq!(decode_line(&line), Ok(value)); } -/// A newline inside a string would end the record, so such a string is the one -/// thing written encoded -- individually, so the rest of the record stays -/// readable. +/// A newline inside a string would end the record, so on one line -- and only +/// there -- it is escaped. The escape covers the newline, not the string: the +/// words around it stay greppable, and so does the rest of the record. #[test] fn a_string_holding_a_newline_still_fits_on_one_line() { let value = LinoValue::object([ @@ -102,8 +102,9 @@ fn a_string_holding_a_newline_still_fits_on_one_line() { let line = encode_line(&value); assert_eq!( line, - r#"(o: (readable "still visible") (multiline (base64 "bGluZTEKbGluZTI=")))"# + r#"(o: (readable "still visible") (multiline (escaped "line1%0Aline2")))"# ); + assert!(line.contains("line1") && line.contains("line2"), "{line:?}"); assert!( !line.contains('\n'), "a record must stay on one line: {line:?}"