From 5957a8183adae8423aaf817d6e9e5dc247aaf445 Mon Sep 17 00:00:00 2001
From: Antonin Bas <abas@vmware.com>
Date: Thu, 28 Sep 2023 17:07:24 -0700
Subject: [PATCH] Improve handling of string IEs

* Use the copy built-in to copy string bytes to buffer, instead of
  copying bytes one-by-one with a for loop. This is more efficient
  (confirmed with new Benchmark functions) and more correct (in case
  the string includes UTF-8 characters which use more than 1 byte).
* Increase max string length from 65,534 to 65,535.

Signed-off-by: Antonin Bas <abas@vmware.com>
---
 pkg/entities/ie.go      | 28 +++++++++++++---------------
 pkg/entities/ie_test.go | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/pkg/entities/ie.go b/pkg/entities/ie.go
index de72b331..7de28d59 100644
--- a/pkg/entities/ie.go
+++ b/pkg/entities/ie.go
@@ -485,16 +485,14 @@ func EncodeToIEDataType(dataType IEDataType, val interface{}) ([]byte, error) {
 		if len(v) < 255 {
 			encodedBytes = make([]byte, len(v)+1)
 			encodedBytes[0] = uint8(len(v))
-			for i, b := range v {
-				encodedBytes[i+1] = byte(b)
-			}
-		} else if len(v) < 65535 {
+			copy(encodedBytes[1:], v)
+		} else if len(v) <= math.MaxUint16 {
 			encodedBytes = make([]byte, len(v)+3)
 			encodedBytes[0] = byte(255)
 			binary.BigEndian.PutUint16(encodedBytes[1:3], uint16(len(v)))
-			for i, b := range v {
-				encodedBytes[i+3] = byte(b)
-			}
+			copy(encodedBytes[3:], v)
+		} else {
+			return fmt.Errorf("provided String value is too long and cannot be encoded: len=%d, maxlen=%d", len(v), math.MaxUint16)
 		}
 		return encodedBytes, nil
 	}
@@ -560,15 +558,15 @@ func encodeInfoElementValueToBuff(element InfoElementWithValue, buffer []byte, i
 		v := element.GetStringValue()
 		if len(v) < 255 {
 			buffer[index] = uint8(len(v))
-			for i, b := range v {
-				buffer[i+index+1] = byte(b)
-			}
-		} else if len(v) < 65535 {
-			buffer[index] = byte(255)
+			// See https://pkg.go.dev/builtin#copy
+			// As a special case, it also will copy bytes from a string to a slice of bytes.
+			copy(buffer[index+1:], v)
+		} else if len(v) <= math.MaxUint16 {
+			buffer[index] = byte(255) // marker byte for long strings
 			binary.BigEndian.PutUint16(buffer[index+1:index+3], uint16(len(v)))
-			for i, b := range v {
-				buffer[i+index+3] = byte(b)
-			}
+			copy(buffer[index+3:], v)
+		} else {
+			return fmt.Errorf("provided String value is too long and cannot be encoded: len=%d, maxlen=%d", len(v), math.MaxUint16)
 		}
 	default:
 		return fmt.Errorf("API supports only valid information elements with datatypes given in RFC7011")
diff --git a/pkg/entities/ie_test.go b/pkg/entities/ie_test.go
index 9f95355e..9bcb9e3e 100644
--- a/pkg/entities/ie_test.go
+++ b/pkg/entities/ie_test.go
@@ -2,6 +2,7 @@ package entities
 
 import (
 	"net"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -72,3 +73,37 @@ func TestNewInfoElementWithValue(t *testing.T) {
 	assert.Equal(t, element.GetInfoElement().Name, "sourceIPv4Address")
 	assert.Equal(t, element.GetIPAddressValue(), ip)
 }
+
+func BenchmarkEncodeInfoElementValueToBuffShortString(b *testing.B) {
+	// a short string has a max length of 254
+	str := strings.Repeat("x", 128)
+	element := NewStringInfoElement(NewInfoElement("interfaceDescription", 83, 13, 0, 65535), str)
+	const numCopies = 1000
+	length := element.GetLength()
+	buffer := make([]byte, numCopies*length)
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		index := 0
+		for j := 0; j < numCopies; j++ {
+			require.NoError(b, encodeInfoElementValueToBuff(element, buffer, index))
+			index += length
+		}
+	}
+}
+
+func BenchmarkEncodeInfoElementValueToBuffLongString(b *testing.B) {
+	// a long string has a max length of 65534
+	str := strings.Repeat("x", 10000)
+	element := NewStringInfoElement(NewInfoElement("interfaceDescription", 83, 13, 0, 65535), str)
+	const numCopies = 1000
+	length := element.GetLength()
+	buffer := make([]byte, numCopies*length)
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		index := 0
+		for j := 0; j < numCopies; j++ {
+			require.NoError(b, encodeInfoElementValueToBuff(element, buffer, index))
+			index += length
+		}
+	}
+}