golang filter: fetch body data as []byte (envoyproxy#29775)

* golang filter: fetch body data as []byte Most of the Go libraries consider using []byte is more efficient. Therefore, they only provide api with []byte. By returning []byte instead of string, we can avoid an extra `string -> []byte` copy. This change also align the behavior with the proxy-wasm-go-sdk, make it easier to port Wasm plugin to Go plugin. Signed-off-by: spacewander <[email protected]> * for write op Signed-off-by: spacewander <[email protected]> --------- Signed-off-by: spacewander <[email protected]>
Winbobob · Sep 26, 2023 · b7a4d93 · b7a4d93
1 parent 34ea49f
commit b7a4d93
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 14 deletions.
diff --git a/contrib/golang/common/go/api/capi.go b/contrib/golang/common/go/api/capi.go
@@ -32,9 +32,10 @@ type HttpCAPI interface {
 	HttpSetHeader(r unsafe.Pointer, key *string, value *string, add bool)
 	HttpRemoveHeader(r unsafe.Pointer, key *string)
 
-	HttpGetBuffer(r unsafe.Pointer, bufferPtr uint64, value *string, length uint64)
+	HttpGetBuffer(r unsafe.Pointer, bufferPtr uint64, length uint64) []byte
 	HttpDrainBuffer(r unsafe.Pointer, bufferPtr uint64, length uint64)
 	HttpSetBufferHelper(r unsafe.Pointer, bufferPtr uint64, value string, action BufferAction)
+	HttpSetBytesBufferHelper(r unsafe.Pointer, bufferPtr uint64, value []byte, action BufferAction)
 
 	HttpCopyTrailers(r unsafe.Pointer, num uint64, bytes uint64) map[string][]string
 	HttpSetTrailer(r unsafe.Pointer, key *string, value *string, add bool)

diff --git a/contrib/golang/filters/http/source/go/pkg/http/capi_impl.go b/contrib/golang/filters/http/source/go/pkg/http/capi_impl.go
@@ -183,14 +183,12 @@ func (c *httpCApiImpl) HttpRemoveHeader(r unsafe.Pointer, key *string) {
 	handleCApiStatus(res)
 }
 
-func (c *httpCApiImpl) HttpGetBuffer(r unsafe.Pointer, bufferPtr uint64, value *string, length uint64) {
+func (c *httpCApiImpl) HttpGetBuffer(r unsafe.Pointer, bufferPtr uint64, length uint64) []byte {
 	buf := make([]byte, length)
 	bHeader := (*reflect.SliceHeader)(unsafe.Pointer(&buf))
-	sHeader := (*reflect.StringHeader)(unsafe.Pointer(value))
-	sHeader.Data = bHeader.Data
-	sHeader.Len = int(length)
 	res := C.envoyGoFilterHttpGetBuffer(r, C.ulonglong(bufferPtr), unsafe.Pointer(bHeader.Data))
 	handleCApiStatus(res)
+	return buf
 }
 
 func (c *httpCApiImpl) HttpDrainBuffer(r unsafe.Pointer, bufferPtr uint64, length uint64) {
@@ -200,6 +198,15 @@ func (c *httpCApiImpl) HttpDrainBuffer(r unsafe.Pointer, bufferPtr uint64, lengt
 
 func (c *httpCApiImpl) HttpSetBufferHelper(r unsafe.Pointer, bufferPtr uint64, value string, action api.BufferAction) {
 	sHeader := (*reflect.StringHeader)(unsafe.Pointer(&value))
+	c.httpSetBufferHelper(r, bufferPtr, unsafe.Pointer(sHeader.Data), C.int(sHeader.Len), action)
+}
+
+func (c *httpCApiImpl) HttpSetBytesBufferHelper(r unsafe.Pointer, bufferPtr uint64, value []byte, action api.BufferAction) {
+	bHeader := (*reflect.SliceHeader)(unsafe.Pointer(&value))
+	c.httpSetBufferHelper(r, bufferPtr, unsafe.Pointer(bHeader.Data), C.int(bHeader.Len), action)
+}
+
+func (c *httpCApiImpl) httpSetBufferHelper(r unsafe.Pointer, bufferPtr uint64, data unsafe.Pointer, length C.int, action api.BufferAction) {
 	var act C.bufferAction
 	switch action {
 	case api.SetBuffer:
@@ -209,7 +216,7 @@ func (c *httpCApiImpl) HttpSetBufferHelper(r unsafe.Pointer, bufferPtr uint64, v
 	case api.PrependBuffer:
 		act = C.Prepend
 	}
-	res := C.envoyGoFilterHttpSetBufferHelper(r, C.ulonglong(bufferPtr), unsafe.Pointer(sHeader.Data), C.int(sHeader.Len), act)
+	res := C.envoyGoFilterHttpSetBufferHelper(r, C.ulonglong(bufferPtr), data, length, act)
 	handleCApiStatus(res)
 }
 

diff --git a/contrib/golang/filters/http/source/go/pkg/http/type.go b/contrib/golang/filters/http/source/go/pkg/http/type.go
@@ -326,13 +326,16 @@ type httpBuffer struct {
 	request             *httpRequest
 	envoyBufferInstance uint64
 	length              uint64
-	value               string
+	value               []byte
 }
 
 var _ api.BufferInstance = (*httpBuffer)(nil)
 
 func (b *httpBuffer) Write(p []byte) (n int, err error) {
-	return b.WriteString(string(p))
+	cAPI.HttpSetBytesBufferHelper(unsafe.Pointer(b.request.req), b.envoyBufferInstance, p, api.AppendBuffer)
+	n = len(p)
+	b.length += uint64(n)
+	return n, nil
 }
 
 func (b *httpBuffer) WriteString(s string) (n int, err error) {
@@ -370,8 +373,8 @@ func (b *httpBuffer) Bytes() []byte {
 	if b.length == 0 {
 		return nil
 	}
-	cAPI.HttpGetBuffer(unsafe.Pointer(b.request.req), b.envoyBufferInstance, &b.value, b.length)
-	return []byte(b.value)
+	b.value = cAPI.HttpGetBuffer(unsafe.Pointer(b.request.req), b.envoyBufferInstance, b.length)
+	return b.value
 }
 
 func (b *httpBuffer) Drain(offset int) {
@@ -401,8 +404,8 @@ func (b *httpBuffer) String() string {
 	if b.length == 0 {
 		return ""
 	}
-	cAPI.HttpGetBuffer(unsafe.Pointer(b.request.req), b.envoyBufferInstance, &b.value, b.length)
-	return b.value
+	b.value = cAPI.HttpGetBuffer(unsafe.Pointer(b.request.req), b.envoyBufferInstance, b.length)
+	return string(b.value)
 }
 
 func (b *httpBuffer) Append(data []byte) error {
@@ -411,7 +414,7 @@ func (b *httpBuffer) Append(data []byte) error {
 }
 
 func (b *httpBuffer) Prepend(data []byte) error {
-	cAPI.HttpSetBufferHelper(unsafe.Pointer(b.request.req), b.envoyBufferInstance, string(data), api.PrependBuffer)
+	cAPI.HttpSetBytesBufferHelper(unsafe.Pointer(b.request.req), b.envoyBufferInstance, data, api.PrependBuffer)
 	b.length += uint64(len(data))
 	return nil
 }
@@ -428,7 +431,7 @@ func (b *httpBuffer) PrependString(s string) error {
 }
 
 func (b *httpBuffer) Set(data []byte) error {
-	cAPI.HttpSetBufferHelper(unsafe.Pointer(b.request.req), b.envoyBufferInstance, string(data), api.SetBuffer)
+	cAPI.HttpSetBytesBufferHelper(unsafe.Pointer(b.request.req), b.envoyBufferInstance, data, api.SetBuffer)
 	b.length = uint64(len(data))
 	return nil
 }

diff --git a/contrib/golang/filters/http/test/test_data/basic/filter.go b/contrib/golang/filters/http/test/test_data/basic/filter.go
@@ -217,6 +217,11 @@ func (f *filter) decodeData(buffer api.BufferInstance, endStream bool) api.Statu
 	f.req_body_length += uint64(buffer.Len())
 	if buffer.Len() != 0 {
 		data := buffer.String()
+		if string(buffer.Bytes()) != data {
+			return f.sendLocalReply(fmt.Sprintf("data in bytes: %s vs data in string: %s",
+				string(buffer.Bytes()), data))
+		}
+
 		buffer.SetString(strings.ToUpper(data))
 		buffer.AppendString("_append")
 		buffer.PrependString("prepend_")