Support odd length strings and error on value too long (#605)

* Support odd length strings and error on value too long Signed-off-by: Joe Elliott <[email protected]> * changelog Signed-off-by: Joe Elliott <[email protected]>
grafana · Mar 22, 2021 · 7539d0f · 7539d0f
1 parent 8e79e79
commit 7539d0f
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@
 * [BUGFIX] Fixes where ingester may leave file open [#570](https://github.com/grafana/tempo/pull/570)
 * [BUGFIX] Fixes a bug where some blocks were not searched due to query sharding and randomness in blocklist poll. [#583](https://github.com/grafana/tempo/pull/583)
 * [BUGFIX] Fixes issue where wal was deleted before successful flush and adds exponential backoff for flush errors [#593](https://github.com/grafana/tempo/pull/593)
+* [BUGFIX] Fixes issue where Tempo would not parse odd length trace ids [#605](https://github.com/grafana/tempo/pull/605)
 
 ## v0.6.0
 

diff --git a/pkg/util/hash.go b/pkg/util/hash.go
@@ -1,7 +1,6 @@
 package util
 
 import (
-	"encoding/hex"
 	"hash/fnv"
 )
 
@@ -19,17 +18,3 @@ func TokenForTraceID(b []byte) uint32 {
 	_, _ = h.Write(b)
 	return h.Sum32()
 }
-
-func HexStringToTraceID(id string) ([]byte, error) {
-	byteID, err := hex.DecodeString(id)
-	if err != nil {
-		return nil, err
-	}
-
-	size := len(byteID)
-	if size < 16 {
-		byteID = append(make([]byte, 16-size), byteID...)
-	}
-
-	return byteID, nil
-}
diff --git a/pkg/util/http.go b/pkg/util/http.go
@@ -1,6 +1,8 @@
 package util
 
 import (
+	"encoding/hex"
+	"errors"
 	"fmt"
 	"net/http"
 
@@ -21,10 +23,33 @@ func ParseTraceID(r *http.Request) ([]byte, error) {
 		return nil, fmt.Errorf("please provide a traceID")
 	}
 
-	byteID, err := HexStringToTraceID(traceID)
+	byteID, err := hexStringToTraceID(traceID)
 	if err != nil {
 		return nil, err
 	}
 
 	return byteID, nil
 }
+
+func hexStringToTraceID(id string) ([]byte, error) {
+	// the encoding/hex package does not like odd length strings.
+	// just append a bit here
+	if len(id)%2 == 1 {
+		id = "0" + id
+	}
+
+	byteID, err := hex.DecodeString(id)
+	if err != nil {
+		return nil, err
+	}
+
+	size := len(byteID)
+	if size > 16 {
+		return nil, errors.New("trace ids can't be larger than 128 bits")
+	}
+	if size < 16 {
+		byteID = append(make([]byte, 16-size), byteID...)
+	}
+
+	return byteID, nil
+}
diff --git a/pkg/util/http_test.go b/pkg/util/http_test.go
@@ -0,0 +1,53 @@
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestHexStringToTraceID(t *testing.T) {
+
+	tc := []struct {
+		id          string
+		expected    []byte
+		expectError bool
+	}{
+		{
+			id:       "12",
+			expected: []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12},
+		},
+		{
+			id:       "1234567890abcdef", // 64 bit
+			expected: []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef},
+		},
+		{
+			id:       "1234567890abcdef1234567890abcdef", // 128 bit
+			expected: []byte{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef},
+		},
+		{
+			id:          "121234567890abcdef1234567890abcdef", // value too long
+			expected:    []byte{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef},
+			expectError: true,
+		},
+		{
+			id:       "234567890abcdef", // odd length
+			expected: []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef},
+		},
+	}
+
+	for _, tt := range tc {
+		t.Run(tt.id, func(t *testing.T) {
+			actual, err := hexStringToTraceID(tt.id)
+
+			if tt.expectError {
+				assert.Error(t, err)
+				assert.Nil(t, actual)
+				return
+			}
+
+			assert.NoError(t, err)
+			assert.Equal(t, tt.expected, actual)
+		})
+	}
+}