opentelemetry-go icon indicating copy to clipboard operation
opentelemetry-go copied to clipboard

Fix issue when percent encoded octet sequences don't match the UTF-8 encoding schema

Open santileira opened this issue 8 months ago • 6 comments

Goal

Replace the percent encoded octet sequence with the replacement code point (U+FFFD) when it doesn't match the UTF-8 encoding schema.

Issue: https://github.com/open-telemetry/opentelemetry-go/issues/5519

Current behavior:

package main

import (
	"fmt"
	"log"
	"unicode/utf8"

	"go.opentelemetry.io/otel/baggage"
)

func main() {
	kv := "k=aa%ffcc"

	b, err := baggage.Parse(kv)
	if err != nil {
		log.Fatal(err)
	}
	val := b.Members()[0].Value()

	fmt.Println(len(val)) # 5
	fmt.Println(utf8.ValidString(val)) # false
}

Expected behavior:

package main

import (
	"fmt"
	"log"
	"unicode/utf8"

	"go.opentelemetry.io/otel/baggage"
)

func main() {
	kv := "k=aa%ffcc"

	b, err := baggage.Parse(kv)
	if err != nil {
		log.Fatal(err)
	}
	val := b.Members()[0].Value()

	fmt.Println(len(val)) # 7
	fmt.Println(utf8.ValidString(val)) # true
}

Benchmark

  • go test -bench=BenchmarkParse -count 20 > old.txt
goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/baggage
BenchmarkParse-10    	 2037052	       582.9 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2050328	       584.1 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2052172	       590.2 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2034764	       595.2 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2058732	       581.8 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2067124	       674.3 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2073754	       596.1 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1957274	       590.6 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1871066	       599.4 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1902244	       612.4 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1933255	       604.3 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2040190	       587.5 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2057910	       585.5 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2050994	       593.9 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2000907	       614.2 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2052204	       627.0 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1908308	       653.6 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1834033	       635.4 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1939261	       693.5 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1901704	       597.4 ns/op	     832 B/op	       7 allocs/op
PASS
ok  	go.opentelemetry.io/otel/baggage	37.026s

  • go test -bench=BenchmarkParse -count 20 > new.txt
goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/baggage
BenchmarkParse-10    	 1317759	       793.5 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1859682	       618.8 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2039910	       587.6 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2011011	       589.7 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2038920	       588.5 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2020298	       609.1 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2009492	       590.8 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2028278	       633.5 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2047069	       586.6 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2024991	       591.0 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2039985	       595.4 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2033997	       594.1 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1814720	       597.2 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2025032	       592.6 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2018917	       595.0 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1991096	       598.9 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2028741	       594.4 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 2007601	       605.2 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1997462	       592.8 ns/op	     832 B/op	       7 allocs/op
BenchmarkParse-10    	 1989120	       642.0 ns/op	     832 B/op	       7 allocs/op
PASS
ok  	go.opentelemetry.io/otel/baggage	36.960s
  • benchstat old.txt new.txt
goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/baggage
         │   old.txt   │            new.txt            │
         │   sec/op    │   sec/op     vs base          │
Parse-10   596.8n ± 3%   594.7n ± 2%  ~ (p=0.904 n=20)

         │  old.txt   │            new.txt             │
         │    B/op    │    B/op     vs base            │
Parse-10   832.0 ± 0%   832.0 ± 0%  ~ (p=1.000 n=20) ¹
¹ all samples are equal

         │  old.txt   │            new.txt             │
         │ allocs/op  │ allocs/op   vs base            │
Parse-10   7.000 ± 0%   7.000 ± 0%  ~ (p=1.000 n=20) ¹
¹ all samples are equal

santileira avatar Jun 19 '24 20:06 santileira