From a209843d8ea9db383ce1a1976e43c8e95f831c61 Mon Sep 17 00:00:00 2001
From: Erik Dubbelboer <erik@dubbelboer.com>
Date: Mon, 4 Apr 2022 10:41:36 +0200
Subject: [PATCH] Fix bytesToStr (#358)

The current implementation of bytesToStr uses an unsafe
reflect.StringHeader value. Change the implementation of this function
to a safe and simpler version.

To explain what could go wrong here is some example code:

  var d []byte
  d = someFunctionThatReturnsBytes()

  s := bytesToStr(d)

  doSomethingWith(s)

When this code gets compiled bytesToStr would get inlined and the
code would be like the following. I have included in comments at which
point things could go wrong:

  var d []byte
  d = someFunctionThatReturnsBytes()

  h := (*reflect.SliceHeader)(unsafe.Pointer(&d))
  shdr := reflect.StringHeader{Data: h.Data, Len: h.Len}

  // At this point in time d and d.Data have nothing referencing them anymore
  // shdr.Data is an uintptr so it will be ignored by the GC.
  // This means d and d.Data can be garbage collected here.
  // Internally strings don't use a uintptr for the data, but since this is
  // just a reflect.StringHeader and not a real string yet that doesn't apply
  // here.
  // This is why https://pkg.go.dev/unsafe#Pointer says:
  //   In general, reflect.SliceHeader and reflect.StringHeader should be
  //   used only as *reflect.SliceHeader and *reflect.StringHeader pointing
  //   at actual slices or strings, never as plain structs.

  s := *(*string)(unsafe.Pointer(&shdr))

  // Only at this point s.Data points to d.Data again and the backing storage
  // of d won't be garbage collected anymore.

  doSomethingWith(s)

The chance of this going wrong is probably so small that nobody ever
noticed it happening, but it is there.
---
 jlexer/bytestostr.go | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/jlexer/bytestostr.go b/jlexer/bytestostr.go
index ff7b27c..e68108f 100644
--- a/jlexer/bytestostr.go
+++ b/jlexer/bytestostr.go
@@ -8,7 +8,6 @@
 package jlexer
 
 import (
-	"reflect"
 	"unsafe"
 )
 
@@ -18,7 +17,5 @@ import (
 // chunk may be either blocked from being freed by GC because of a single string or the buffer.Data
 // may be garbage-collected even when the string exists.
 func bytesToStr(data []byte) string {
-	h := (*reflect.SliceHeader)(unsafe.Pointer(&data))
-	shdr := reflect.StringHeader{Data: h.Data, Len: h.Len}
-	return *(*string)(unsafe.Pointer(&shdr))
+	return *(*string)(unsafe.Pointer(&data))
 }