Skip to content

Commit 412d718

Browse files
committed
refactor(hash): extract jsonMD5Raw helper and fix DeterministicUUID to use raw bytes
Extract jsonMD5Raw as an internal helper function to compute MD5 digest without hex encoding. This allows DeterministicUUID to use the raw 16-byte digest directly instead of incorrectly parsing hex-encoded strings as UUID bytes, fixing a regression where UUID bytes were ASCII codes of hex digits. Also add comprehensive test coverage for JSONMD5Hash and DeterministicUUID, including passthrough behavior for UUID values, pointers, byte arrays, and strings. Update CI workflow to use gavel for testing and linting with PR comment support. Fixes incorrect UUID generation from non-UUID seeds.
1 parent 9f147ce commit 412d718

5 files changed

Lines changed: 268 additions & 11 deletions

File tree

.github/workflows/test.yml

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ jobs:
1313
go-version:
1414
- 1.25.x
1515
runs-on: ${{ matrix.platform }}
16+
permissions:
17+
contents: read
18+
pull-requests: write # required when comment=true so the action can post/update the sticky PR comment
1619
steps:
1720
- name: Harden the runner (Audit all outbound calls)
1821
uses: step-security/harden-runner@f4a75cfd619ee5ce8d5b864b0d183aff3c69b55a # v2.13.1
@@ -25,5 +28,15 @@ jobs:
2528
go-version: ${{ matrix.go-version }}
2629
- name: Checkout code
2730
uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6
28-
- name: Test
29-
run: go test ./...
31+
32+
- uses: flanksource/gavel@main
33+
with:
34+
args: test --lint
35+
version: latest
36+
json-file: gavel-results.json
37+
html-file: gavel-results.html
38+
summary-file: gavel-summary.md
39+
artifact-name: gavel-results
40+
comment: "true"
41+
comment-header: gavel
42+
fail-on-error: "true"

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,4 @@ y
2020
cliff.toml
2121
.todos/
2222
cmd/hx/fixtures/hx
23-
.ginkgo
23+
.ginkgo/

har/pretty.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func headersToDescriptionList(headers []Header) api.DescriptionList {
5959
func (e Entry) Columns() []api.ColumnDef {
6060
return []api.ColumnDef{
6161
api.Column("method").Label("Method").Style("font-bold text-green-500 uppercase").Build(),
62-
api.Column("url").Label("URL").MaxWidth(80).Build(),
62+
api.Column("url").Label("URL").MaxWidth(100).Build(),
6363
api.Column("status").Label("Status").Build(),
6464
api.Column("duration").Label("Duration").Build(),
6565
api.Column("size").Label("Size").Build(),

hash/hash.go

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,17 +64,22 @@ import (
6464
// hash, err := JSONMD5Hash(config)
6565
// // hash will be consistent for the same config values
6666
func JSONMD5Hash(obj any) (string, error) {
67-
data, err := json.Marshal(obj)
67+
raw, err := jsonMD5Raw(obj)
6868
if err != nil {
6969
return "", err
7070
}
71+
return hex.EncodeToString(raw[:]), nil
72+
}
7173

72-
hash := md5.Sum(data)
74+
// jsonMD5Raw marshals the object into JSON and returns the raw 16-byte MD5
75+
// digest. Internal helper shared by JSONMD5Hash (which hex-encodes it) and
76+
// DeterministicUUID (which uses the raw bytes as UUID bytes).
77+
func jsonMD5Raw(obj any) ([16]byte, error) {
78+
data, err := json.Marshal(obj)
7379
if err != nil {
74-
return "", err
80+
return [16]byte{}, err
7581
}
76-
77-
return hex.EncodeToString(hash[:]), nil
82+
return md5.Sum(data), nil
7883
}
7984

8085
// Sha256Hex computes the SHA256 hash of the input string and returns it
@@ -113,15 +118,56 @@ func Sha256Hex(in string) string {
113118
// // Generate UUID from string
114119
// id2, err := DeterministicUUID("unique-resource-name")
115120
func DeterministicUUID(seed any) (uuid.UUID, error) {
116-
byteHash, err := JSONMD5Hash(seed)
121+
// If the seed is already a UUID (value, pointer, 16 bytes, or parseable
122+
// string — uuid.Nil included), return it verbatim. Re-hashing a UUID would
123+
// produce a different UUID, defeating the caller's intent.
124+
if id, ok := asUUID(seed); ok {
125+
return id, nil
126+
}
127+
128+
raw, err := jsonMD5Raw(seed)
117129
if err != nil {
118130
return uuid.Nil, err
119131
}
120132

121-
id, err := uuid.FromBytes([]byte(byteHash[0:16]))
133+
// md5.Sum returns exactly 16 bytes, which is the size of a UUID. Use the
134+
// raw digest directly — NOT the hex-encoded representation.
135+
id, err := uuid.FromBytes(raw[:])
122136
if err != nil {
123137
return uuid.Nil, err
124138
}
125139

126140
return id, nil
127141
}
142+
143+
// asUUID reports whether seed is already a UUID in one of its common forms:
144+
// uuid.UUID, *uuid.UUID, [16]byte, []byte of length 16, or a string that
145+
// parses as a UUID. uuid.Nil counts as a valid UUID.
146+
//
147+
// Composite seeds ([]string, pq.StringArray, structs, etc.) are not unwrapped
148+
// — a single-element slice containing a UUID is still a composite and should
149+
// be hashed.
150+
func asUUID(seed any) (uuid.UUID, bool) {
151+
switch v := seed.(type) {
152+
case uuid.UUID:
153+
return v, true
154+
case *uuid.UUID:
155+
if v == nil {
156+
return uuid.Nil, false
157+
}
158+
return *v, true
159+
case [16]byte:
160+
return uuid.UUID(v), true
161+
case []byte:
162+
if len(v) == 16 {
163+
var id uuid.UUID
164+
copy(id[:], v)
165+
return id, true
166+
}
167+
case string:
168+
if id, err := uuid.Parse(v); err == nil {
169+
return id, true
170+
}
171+
}
172+
return uuid.Nil, false
173+
}

hash/hash_test.go

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
package hash
2+
3+
import (
4+
"bytes"
5+
"crypto/md5"
6+
"encoding/hex"
7+
"encoding/json"
8+
"testing"
9+
10+
"github.com/google/uuid"
11+
)
12+
13+
func TestJSONMD5Hash_ReturnsHexEncoded(t *testing.T) {
14+
// Hex encoding of an MD5 digest is always 32 characters.
15+
h, err := JSONMD5Hash("hello")
16+
if err != nil {
17+
t.Fatalf("unexpected error: %v", err)
18+
}
19+
if len(h) != 32 {
20+
t.Fatalf("expected 32-char hex hash, got %d chars: %q", len(h), h)
21+
}
22+
if _, err := hex.DecodeString(h); err != nil {
23+
t.Fatalf("hash is not valid hex: %v", err)
24+
}
25+
}
26+
27+
func TestJSONMD5Hash_Deterministic(t *testing.T) {
28+
a, _ := JSONMD5Hash(map[string]string{"k": "v"})
29+
b, _ := JSONMD5Hash(map[string]string{"k": "v"})
30+
if a != b {
31+
t.Fatalf("expected deterministic hash, got %q vs %q", a, b)
32+
}
33+
}
34+
35+
func TestDeterministicUUID_UsesRawBytes(t *testing.T) {
36+
// Regression for a bug where DeterministicUUID fed the *hex-encoded*
37+
// JSONMD5Hash string into uuid.FromBytes, producing UUIDs whose bytes
38+
// were the ASCII codes of hex digits (e.g. 30663964-3638-3061-... where
39+
// 0x30='0', 0x66='f', 0x39='9', 0x64='d'). The correct behavior is to
40+
// use the raw 16-byte md5 digest as the UUID bytes.
41+
42+
seed := "test-seed"
43+
got, err := DeterministicUUID(seed)
44+
if err != nil {
45+
t.Fatalf("unexpected error: %v", err)
46+
}
47+
48+
data, _ := json.Marshal(seed)
49+
sum := md5.Sum(data)
50+
// The UUID bytes must equal the raw md5 bytes.
51+
for i := 0; i < 16; i++ {
52+
if got[i] != sum[i] {
53+
t.Fatalf("UUID byte %d: want %#x, got %#x", i, sum[i], got[i])
54+
}
55+
}
56+
57+
// Also assert the UUID is NOT the ASCII-hex-encoded variant of the hex
58+
// representation of the md5, which is what the old buggy code produced.
59+
hexStr := hex.EncodeToString(sum[:])
60+
var bogus [16]byte
61+
copy(bogus[:], hexStr[:16])
62+
if got == bogus {
63+
t.Fatal("DeterministicUUID regressed: still uses ASCII hex bytes as UUID bytes")
64+
}
65+
}
66+
67+
func TestDeterministicUUID_Deterministic(t *testing.T) {
68+
a, _ := DeterministicUUID("same-seed")
69+
b, _ := DeterministicUUID("same-seed")
70+
if a != b {
71+
t.Fatalf("expected deterministic UUID, got %q vs %q", a, b)
72+
}
73+
}
74+
75+
func TestDeterministicUUID_DifferentSeedsProduceDifferentUUIDs(t *testing.T) {
76+
a, _ := DeterministicUUID("seed-one")
77+
b, _ := DeterministicUUID("seed-two")
78+
if a == b {
79+
t.Fatalf("expected distinct UUIDs for distinct seeds, got %q twice", a)
80+
}
81+
}
82+
83+
const passthroughUUIDStr = "550e8400-e29b-41d4-a716-446655440000"
84+
85+
func TestDeterministicUUID_PassesThroughValidUUIDString(t *testing.T) {
86+
got, err := DeterministicUUID(passthroughUUIDStr)
87+
if err != nil {
88+
t.Fatalf("unexpected error: %v", err)
89+
}
90+
if got.String() != passthroughUUIDStr {
91+
t.Fatalf("expected passthrough %q, got %q", passthroughUUIDStr, got.String())
92+
}
93+
}
94+
95+
func TestDeterministicUUID_PassesThroughUUIDValue(t *testing.T) {
96+
in := uuid.MustParse(passthroughUUIDStr)
97+
got, err := DeterministicUUID(in)
98+
if err != nil {
99+
t.Fatalf("unexpected error: %v", err)
100+
}
101+
if got != in {
102+
t.Fatalf("expected passthrough %q, got %q", in, got)
103+
}
104+
}
105+
106+
func TestDeterministicUUID_PassesThroughUUIDPointer(t *testing.T) {
107+
in := uuid.MustParse(passthroughUUIDStr)
108+
got, err := DeterministicUUID(&in)
109+
if err != nil {
110+
t.Fatalf("unexpected error: %v", err)
111+
}
112+
if got != in {
113+
t.Fatalf("expected passthrough %q, got %q", in, got)
114+
}
115+
}
116+
117+
func TestDeterministicUUID_PassesThroughUUIDBytes(t *testing.T) {
118+
in := uuid.MustParse(passthroughUUIDStr)
119+
120+
gotArr, err := DeterministicUUID([16]byte(in))
121+
if err != nil {
122+
t.Fatalf("unexpected error ([16]byte): %v", err)
123+
}
124+
if gotArr != in {
125+
t.Fatalf("[16]byte passthrough: want %q, got %q", in, gotArr)
126+
}
127+
128+
gotSlice, err := DeterministicUUID(in[:])
129+
if err != nil {
130+
t.Fatalf("unexpected error ([]byte): %v", err)
131+
}
132+
if gotSlice != in {
133+
t.Fatalf("[]byte passthrough: want %q, got %q", in, gotSlice)
134+
}
135+
}
136+
137+
func TestDeterministicUUID_PassesThroughNilUUID(t *testing.T) {
138+
const nilStr = "00000000-0000-0000-0000-000000000000"
139+
140+
cases := []struct {
141+
name string
142+
in any
143+
}{
144+
{"uuid.Nil value", uuid.Nil},
145+
{"nil string", nilStr},
146+
{"zero [16]byte", [16]byte{}},
147+
}
148+
for _, tc := range cases {
149+
got, err := DeterministicUUID(tc.in)
150+
if err != nil {
151+
t.Fatalf("%s: unexpected error: %v", tc.name, err)
152+
}
153+
if got != uuid.Nil {
154+
t.Fatalf("%s: expected uuid.Nil passthrough, got %q", tc.name, got)
155+
}
156+
}
157+
}
158+
159+
func TestDeterministicUUID_SingleElementSliceIsHashed(t *testing.T) {
160+
in := []string{passthroughUUIDStr}
161+
got, err := DeterministicUUID(in)
162+
if err != nil {
163+
t.Fatalf("unexpected error: %v", err)
164+
}
165+
if got.String() == passthroughUUIDStr {
166+
t.Fatal("single-element slice must be treated as a composite and hashed, not unwrapped")
167+
}
168+
if got == uuid.Nil {
169+
t.Fatal("hashed composite should not be uuid.Nil")
170+
}
171+
}
172+
173+
func TestDeterministicUUID_NonUUIDStringStillHashes(t *testing.T) {
174+
got, err := DeterministicUUID("test-seed")
175+
if err != nil {
176+
t.Fatalf("unexpected error: %v", err)
177+
}
178+
179+
data, _ := json.Marshal("test-seed")
180+
sum := md5.Sum(data)
181+
if !bytes.Equal(got[:], sum[:]) {
182+
t.Fatalf("non-UUID strings must still hash; want %x, got %x", sum, got[:])
183+
}
184+
}
185+
186+
func TestDeterministicUUID_ShortByteSliceStillHashes(t *testing.T) {
187+
in := []byte{1, 2, 3}
188+
got, err := DeterministicUUID(in)
189+
if err != nil {
190+
t.Fatalf("unexpected error: %v", err)
191+
}
192+
193+
data, _ := json.Marshal(in)
194+
sum := md5.Sum(data)
195+
if !bytes.Equal(got[:], sum[:]) {
196+
t.Fatalf("len != 16 []byte must hash via JSON-MD5; want %x, got %x", sum, got[:])
197+
}
198+
}

0 commit comments

Comments
 (0)