From 973d211083a79c69e21e010582219a30f7795128 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 11 Jun 2025 15:52:23 +0200 Subject: [PATCH] graph/db: fix DecodeHexColor Ensure that it does a sanity check on the length of its input and also only call it if the nullable SQL string is not null. Expand an existing unit tests to cover the DecodeHexColor such that it would have caught the bug. --- graph/db/notifications.go | 5 +++++ graph/db/sql_store.go | 9 ++++++--- graph/notifications_test.go | 36 ++++++++++++++++++++++++++++-------- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/graph/db/notifications.go b/graph/db/notifications.go index 6c0acba88..4af953381 100644 --- a/graph/db/notifications.go +++ b/graph/db/notifications.go @@ -473,6 +473,11 @@ func EncodeHexColor(color color.RGBA) string { // DecodeHexColor takes a hex color string like "#rrggbb" and returns a // color.RGBA. func DecodeHexColor(hex string) (color.RGBA, error) { + if len(hex) != 7 || hex[0] != '#' { + return color.RGBA{}, fmt.Errorf("invalid hex color string: %s", + hex) + } + r, err := strconv.ParseUint(hex[1:3], 16, 8) if err != nil { return color.RGBA{}, fmt.Errorf("invalid red component: %w", diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go index e8cb36dfd..ca8569aee 100644 --- a/graph/db/sql_store.go +++ b/graph/db/sql_store.go @@ -934,9 +934,12 @@ func buildNode(ctx context.Context, db SQLQueries, dbNode *sqlc.Node) ( node.LastUpdate = time.Unix(dbNode.LastUpdate.Int64, 0) var err error - node.Color, err = DecodeHexColor(dbNode.Color.String) - if err != nil { - return nil, fmt.Errorf("unable to decode color: %w", err) + if dbNode.Color.Valid { + node.Color, err = DecodeHexColor(dbNode.Color.String) + if err != nil { + return nil, fmt.Errorf("unable to decode color: %w", + err) + } } // Fetch the node's features. diff --git a/graph/notifications_test.go b/graph/notifications_test.go index 099c2244b..c9096efc1 100644 --- a/graph/notifications_test.go +++ b/graph/notifications_test.go @@ -991,7 +991,9 @@ func TestChannelCloseNotification(t *testing.T) { // TestEncodeHexColor tests that the string used to represent a node color is // correctly encoded. func TestEncodeHexColor(t *testing.T) { - var colorTestCases = []struct { + t.Parallel() + + var tests = []struct { R uint8 G uint8 B uint8 @@ -1006,14 +1008,32 @@ func TestEncodeHexColor(t *testing.T) { {1, 2, 3, "#", false}, } - for _, tc := range colorTestCases { - encoded := graphdb.EncodeHexColor( - color.RGBA{tc.R, tc.G, tc.B, 0}, + for _, test := range tests { + t.Run(fmt.Sprintf("R:%d,G:%d,B:%d", test.R, test.G, test.B), + func(t *testing.T) { + expColor := color.RGBA{ + R: test.R, + G: test.G, + B: test.B, + } + + encoded := graphdb.EncodeHexColor(expColor) + + if test.isValid { + require.Equal(t, test.encoded, encoded) + } + + decoded, err := graphdb.DecodeHexColor( + test.encoded, + ) + if test.isValid { + require.NoError(t, err) + require.Equal(t, expColor, decoded) + } else { + require.Error(t, err) + } + }, ) - if (encoded == tc.encoded) != tc.isValid { - t.Fatalf("incorrect color encoding, "+ - "want: %v, got: %v", tc.encoded, encoded) - } } }