Skip to content

Commit 59f4b79

Browse files
olshmishamyte
andauthored
Use relaxed JSON escaping for log bodies (#340)
* Use relaxed JSON escaping for log bodies The body formatter and batch serializer build Utf8JsonWriters with no options, so they use the default HTML-safe JavaScriptEncoder, which unicode-escapes every quote, ', <, >, & and all non-ASCII characters. Because Serilog quotes string property values by default, this makes most stored Loki lines unreadable (a regression from v8.x, which formatted bodies with Serilog's JsonValueFormatter and standard escaping). Construct the writers with JsonWriterOptions(Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping). Output stays valid JSON and UTF-8, so consumers (including the json parser) are unaffected. Add a unit test covering markup and non-ASCII characters. * Harden escaping test: assert quotes render as \" not " Addresses CodeRabbit review on #340. Assert at the decoded body level (the raw HTTP payload embeds the body as a JSON string, double-escaping its quotes), so a regression back to " for quotes is caught. * Centralize relaxed JSON writer; add Format-path and label escaping tests The relaxed-encoder JsonWriterOptions was defined twice (formatter and serializer) with drifting comments, and each Utf8JsonWriter opted in per construction site: a future writer that forgot the options argument would silently revert to HTML-safe \uXXXX escaping with no compiler warning. Extract a single Infrastructure.JsonWriterDefaults.createWriter as the one way the sink builds a writer, so escaping cannot drift between sites. The shared comment also corrects the description: the relaxed encoder still escapes U+2028/U+2029 and DEL, so it is not "only what JSON mandates". Add the two missing wire-format guards: - the public LokiJsonTextFormatter.Format entry point (the sink fast path bypasses it, so its writer site had no test coverage); - promoted label values (the envelope writer escapes label keys/values too). The escaping tests sample neutral, multi-script non-ASCII (accented Latin plus CJK) rather than a single script. --------- Co-authored-by: Mykhailo Shevchuk <myte@ukr.net>
1 parent fa376ad commit 59f4b79

5 files changed

Lines changed: 93 additions & 4 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright 2020-2026 Mykhailo Shevchuk & Contributors
2+
//
3+
// Licensed under the MIT license;
4+
// you may not use this file except in compliance with the License.
5+
//
6+
// Unless required by applicable law or agreed to in writing, software
7+
// distributed under the License is distributed on an "AS IS" BASIS,
8+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
// See LICENSE file in the project root for full license information.
10+
11+
namespace Serilog.Sinks.Grafana.Loki.Infrastructure
12+
13+
open System.Buffers
14+
open System.Text.Encodings.Web
15+
open System.Text.Json
16+
17+
// Single source of truth for every Utf8JsonWriter the sink creates.
18+
//
19+
// The default Utf8JsonWriter encoder is HTML-safe and renders every '"', '<', '>', '&', '\'' and
20+
// all non-ASCII as \uXXXX, which makes the stored Loki log line unreadable. The relaxed encoder
21+
// escapes the JSON-mandatory set (quote, backslash, control chars) plus the line/paragraph
22+
// separators U+2028/U+2029 and DEL, and emits printable non-ASCII verbatim. Output stays valid
23+
// JSON (and valid UTF-8), so consumers are unaffected.
24+
//
25+
// Centralised behind createWriter so escaping cannot drift between writer sites: a writer built
26+
// the default way (`new Utf8JsonWriter(buffer)`) would silently re-introduce \uXXXX over part of
27+
// the payload, and no compiler warning catches the missing options argument.
28+
[<RequireQualifiedAccess>]
29+
module internal JsonWriterDefaults =
30+
31+
let relaxedOptions =
32+
JsonWriterOptions(Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping)
33+
34+
/// Creates a Utf8JsonWriter over the buffer using the relaxed (readable) encoder.
35+
let createWriter (buffer: PooledByteBufferWriter) =
36+
new Utf8JsonWriter(buffer :> IBufferWriter<byte>, relaxedOptions)

src/Serilog.Sinks.Grafana.Loki/LokiJsonTextFormatter.fs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
namespace Serilog.Sinks.Grafana.Loki
1212

1313
open System
14-
open System.Buffers
1514
open System.Text.Json
1615
open Serilog.Events
1716
open Serilog.Formatting
@@ -173,7 +172,7 @@ type LokiJsonTextFormatter(exceptionFormatter: ILokiExceptionFormatter, enrichTr
173172
use buffer = new PooledByteBufferWriter(256)
174173
// Throwaway writer + scratch for the message render. The internal sink path passes reused
175174
// instances instead; the public path stays allocation-light but fully thread-safe.
176-
use jsonWriter = new Utf8JsonWriter(buffer :> IBufferWriter<byte>)
175+
use jsonWriter = JsonWriterDefaults.createWriter buffer
177176
use messageBuffer = new PooledByteBufferWriter(128)
178177
use messageWriter = new Utf8TextWriter(messageBuffer)
179178
self.FormatToBuffer(logEvent, jsonWriter, messageWriter)

src/Serilog.Sinks.Grafana.Loki/Serialization.fs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ type internal SerializationBuffers() =
3030
let main = new PooledByteBufferWriter(4096)
3131
let body = new PooledByteBufferWriter(256)
3232
let message = new PooledByteBufferWriter(256)
33-
let bodyWriter = new Utf8JsonWriter(body :> IBufferWriter<byte>)
33+
34+
let bodyWriter = JsonWriterDefaults.createWriter body
3435
let messageWriter = new Utf8TextWriter(message)
3536

3637
/// Envelope buffer holding the full push payload; read by LokiPushContent after serialize.
@@ -87,7 +88,7 @@ module internal Serialization =
8788
| :? LokiJsonTextFormatter as fmt when fmt.GetType() = typeof<LokiJsonTextFormatter> -> fmt
8889
| _ -> Unchecked.defaultof<LokiJsonTextFormatter>
8990

90-
use jsonWriter = new Utf8JsonWriter(buffers.Main :> IBufferWriter<byte>)
91+
use jsonWriter = JsonWriterDefaults.createWriter buffers.Main
9192

9293
// Reused stack scratch for the per-event Unix-nanosecond timestamp. Hoisted out of
9394
// the event loop so the localloc happens once per batch, not once per event. An

src/Serilog.Sinks.Grafana.Loki/Serilog.Sinks.Grafana.Loki.fsproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
<!-- Infrastructure (no Serilog dependency) -->
4949
<Compile Include="Infrastructure\PooledByteBufferWriter.fs"/>
5050
<Compile Include="Infrastructure\Utf8TextWriter.fs"/>
51+
<Compile Include="Infrastructure\JsonWriterDefaults.fs"/>
5152

5253
<!-- Public contract types -->
5354
<Compile Include="LokiLabel.fs"/>

tests/Serilog.Sinks.Grafana.Loki.UnitTests/WireFormatTests.fs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,58 @@ let ``body: contains Message and MessageTemplate`` () : Task =
252252
test <@ hasMsg && hasTpl @>
253253
}
254254

255+
[<Fact>]
256+
let ``body: quotes, markup and non-ASCII are not unicode-escaped`` () : Task =
257+
task {
258+
let handler, sink = makeSink id
259+
use _ = sink
260+
do! flush sink [ mkInfo [ "Note", box "a>b <c> & \"q\" café 日本語" ] ]
261+
use doc = handler.LastBodyJson
262+
let body = bodyStringOf (streamAt 0 doc) 0
263+
// Relaxed escaping: '<' '>' '&' and non-ASCII stay verbatim and quotes use the JSON-standard
264+
// \", never \uXXXX. The default HTML-safe encoder would render all of these as \uXXXX.
265+
test <@ body.Contains("a>b <c> &") @>
266+
test <@ body.Contains("café 日本語") @>
267+
test <@ body.Contains("\\\"q\\\"") @>
268+
test <@ not (body.Contains("\\u0022")) @>
269+
}
270+
271+
[<Fact>]
272+
let ``body: public Format path keeps quotes, markup and non-ASCII readable`` () =
273+
// The sink fast path calls FormatToBuffer directly; this is the only coverage of the public
274+
// ITextFormatter.Format entry point, whose Utf8JsonWriter is a separate construction site.
275+
let formatter = LokiJsonTextFormatter() :> ITextFormatter
276+
let event = mkInfo [ "Note", box "a>b <c> & \"q\" café 日本語" ]
277+
use output = new System.IO.StringWriter()
278+
formatter.Format(event, output)
279+
let body = output.ToString()
280+
use _ = JsonDocument.Parse(body) // output must stay valid JSON
281+
test <@ body.Contains("a>b <c> &") @>
282+
test <@ body.Contains("café 日本語") @>
283+
test <@ body.Contains("\\\"q\\\"") @>
284+
test <@ not (body.Contains("\\u")) @>
285+
286+
[<Fact>]
287+
let ``labels: promoted label value keeps markup and non-ASCII verbatim`` () : Task =
288+
task {
289+
let handler, sink =
290+
makeSink (fun o ->
291+
{ o with
292+
PropertiesAsLabels = [| "app" |]
293+
})
294+
295+
use _ = sink
296+
do! flush sink [ mkInfo [ "app", box "<svc> café 日本語" ] ]
297+
use doc = handler.LastBodyJson
298+
let raw = handler.LastBodyText
299+
// The envelope writer escapes label keys/values too. The round-trip proves the label
300+
// decodes back to the original; the raw scan proves it ships verbatim, not \uXXXX (a
301+
// label-only regression would still decode correctly but leak escaped < or non-ASCII bytes).
302+
test <@ labelOf "app" (streamAt 0 doc) = Some "<svc> café 日本語" @>
303+
test <@ raw.Contains("<svc> café 日本語") @>
304+
test <@ not (raw.Contains("\\u")) @>
305+
}
306+
255307
[<Fact>]
256308
let ``body: reserved property "Message" sanitized to "_Message"`` () : Task =
257309
task {

0 commit comments

Comments
 (0)