Skip to content

Commit eac2a9f

Browse files
asadijabarclaude
asadijabar
and
claude
committed
fix: report malformed min/max tags even on early-return paths
Move min/max tag parseability checks before required/zero-value early returns in validateFieldWithPresence, so malformed constraints are always surfaced. The validateXxxMinMax functions now only do value comparison (parse errors are caught upfront by validateMinMaxParseable). Addresses code review feedback from PR #53. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 39d0ca8 commit eac2a9f

2 files changed

Lines changed: 101 additions & 56 deletions

File tree

‎validate.go‎

Lines changed: 76 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ func validateFieldWithPresence(fieldValue reflect.Value, fieldPath string, tags
2929
})
3030
}
3131

32+
// Validate min/max tag parseability upfront so malformed constraints
33+
// are reported even when value validation is skipped (e.g., zero/missing values).
34+
errors = append(errors, validateMinMaxParseable(fieldValue, fieldPath, tags)...)
35+
3236
// Check required constraint
3337
if tags.required {
3438
if presentFields != nil {
@@ -196,20 +200,78 @@ func isZeroValue(v reflect.Value) bool {
196200
}
197201
}
198202

203+
// validateMinMaxParseable checks that min/max tag values can be parsed for the given field kind.
204+
// Called before early returns so malformed constraints are always reported.
205+
func validateMinMaxParseable(fieldValue reflect.Value, fieldPath string, tags tagConfig) []FieldError {
206+
var errors []FieldError
207+
208+
kind := fieldValue.Kind()
209+
switch kind {
210+
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
211+
if tags.min != "" {
212+
if _, err := strconv.ParseInt(tags.min, 10, 64); err != nil {
213+
errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag,
214+
Message: fmt.Sprintf("invalid min constraint %q: must be a valid integer", tags.min)})
215+
}
216+
}
217+
if tags.max != "" {
218+
if _, err := strconv.ParseInt(tags.max, 10, 64); err != nil {
219+
errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag,
220+
Message: fmt.Sprintf("invalid max constraint %q: must be a valid integer", tags.max)})
221+
}
222+
}
223+
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
224+
if tags.min != "" {
225+
if _, err := strconv.ParseUint(tags.min, 10, 64); err != nil {
226+
errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag,
227+
Message: fmt.Sprintf("invalid min constraint %q: must be a valid unsigned integer", tags.min)})
228+
}
229+
}
230+
if tags.max != "" {
231+
if _, err := strconv.ParseUint(tags.max, 10, 64); err != nil {
232+
errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag,
233+
Message: fmt.Sprintf("invalid max constraint %q: must be a valid unsigned integer", tags.max)})
234+
}
235+
}
236+
case reflect.Float32, reflect.Float64:
237+
if tags.min != "" {
238+
if _, err := strconv.ParseFloat(tags.min, 64); err != nil {
239+
errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag,
240+
Message: fmt.Sprintf("invalid min constraint %q: must be a valid number", tags.min)})
241+
}
242+
}
243+
if tags.max != "" {
244+
if _, err := strconv.ParseFloat(tags.max, 64); err != nil {
245+
errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag,
246+
Message: fmt.Sprintf("invalid max constraint %q: must be a valid number", tags.max)})
247+
}
248+
}
249+
case reflect.String:
250+
if tags.min != "" {
251+
if _, err := strconv.Atoi(tags.min); err != nil {
252+
errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag,
253+
Message: fmt.Sprintf("invalid min constraint %q: must be a valid integer", tags.min)})
254+
}
255+
}
256+
if tags.max != "" {
257+
if _, err := strconv.Atoi(tags.max); err != nil {
258+
errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag,
259+
Message: fmt.Sprintf("invalid max constraint %q: must be a valid integer", tags.max)})
260+
}
261+
}
262+
}
263+
264+
return errors
265+
}
266+
199267
// validateIntMinMax validates min/max constraints for signed integer types.
200268
func validateIntMinMax(fieldValue reflect.Value, fieldPath string, tags tagConfig) []FieldError {
201269
var errors []FieldError
202270
value := fieldValue.Int()
203271

204272
if tags.min != "" {
205273
minVal, err := strconv.ParseInt(tags.min, 10, 64)
206-
if err != nil {
207-
errors = append(errors, FieldError{
208-
FieldPath: fieldPath,
209-
Code: ErrCodeInvalidTag,
210-
Message: fmt.Sprintf("invalid min constraint %q: must be a valid integer", tags.min),
211-
})
212-
} else if value < minVal {
274+
if err == nil && value < minVal {
213275
errors = append(errors, FieldError{
214276
FieldPath: fieldPath,
215277
Code: ErrCodeMin,
@@ -220,13 +282,7 @@ func validateIntMinMax(fieldValue reflect.Value, fieldPath string, tags tagConfi
220282

221283
if tags.max != "" {
222284
maxVal, err := strconv.ParseInt(tags.max, 10, 64)
223-
if err != nil {
224-
errors = append(errors, FieldError{
225-
FieldPath: fieldPath,
226-
Code: ErrCodeInvalidTag,
227-
Message: fmt.Sprintf("invalid max constraint %q: must be a valid integer", tags.max),
228-
})
229-
} else if value > maxVal {
285+
if err == nil && value > maxVal {
230286
errors = append(errors, FieldError{
231287
FieldPath: fieldPath,
232288
Code: ErrCodeMax,
@@ -245,13 +301,7 @@ func validateUintMinMax(fieldValue reflect.Value, fieldPath string, tags tagConf
245301

246302
if tags.min != "" {
247303
minVal, err := strconv.ParseUint(tags.min, 10, 64)
248-
if err != nil {
249-
errors = append(errors, FieldError{
250-
FieldPath: fieldPath,
251-
Code: ErrCodeInvalidTag,
252-
Message: fmt.Sprintf("invalid min constraint %q: must be a valid unsigned integer", tags.min),
253-
})
254-
} else if value < minVal {
304+
if err == nil && value < minVal {
255305
errors = append(errors, FieldError{
256306
FieldPath: fieldPath,
257307
Code: ErrCodeMin,
@@ -262,13 +312,7 @@ func validateUintMinMax(fieldValue reflect.Value, fieldPath string, tags tagConf
262312

263313
if tags.max != "" {
264314
maxVal, err := strconv.ParseUint(tags.max, 10, 64)
265-
if err != nil {
266-
errors = append(errors, FieldError{
267-
FieldPath: fieldPath,
268-
Code: ErrCodeInvalidTag,
269-
Message: fmt.Sprintf("invalid max constraint %q: must be a valid unsigned integer", tags.max),
270-
})
271-
} else if value > maxVal {
315+
if err == nil && value > maxVal {
272316
errors = append(errors, FieldError{
273317
FieldPath: fieldPath,
274318
Code: ErrCodeMax,
@@ -287,13 +331,7 @@ func validateFloatMinMax(fieldValue reflect.Value, fieldPath string, tags tagCon
287331

288332
if tags.min != "" {
289333
minVal, err := strconv.ParseFloat(tags.min, 64)
290-
if err != nil {
291-
errors = append(errors, FieldError{
292-
FieldPath: fieldPath,
293-
Code: ErrCodeInvalidTag,
294-
Message: fmt.Sprintf("invalid min constraint %q: must be a valid number", tags.min),
295-
})
296-
} else if value < minVal {
334+
if err == nil && value < minVal {
297335
errors = append(errors, FieldError{
298336
FieldPath: fieldPath,
299337
Code: ErrCodeMin,
@@ -304,13 +342,7 @@ func validateFloatMinMax(fieldValue reflect.Value, fieldPath string, tags tagCon
304342

305343
if tags.max != "" {
306344
maxVal, err := strconv.ParseFloat(tags.max, 64)
307-
if err != nil {
308-
errors = append(errors, FieldError{
309-
FieldPath: fieldPath,
310-
Code: ErrCodeInvalidTag,
311-
Message: fmt.Sprintf("invalid max constraint %q: must be a valid number", tags.max),
312-
})
313-
} else if value > maxVal {
345+
if err == nil && value > maxVal {
314346
errors = append(errors, FieldError{
315347
FieldPath: fieldPath,
316348
Code: ErrCodeMax,
@@ -330,13 +362,7 @@ func validateStringMinMax(fieldValue reflect.Value, fieldPath string, tags tagCo
330362

331363
if tags.min != "" {
332364
minLen, err := strconv.Atoi(tags.min)
333-
if err != nil {
334-
errors = append(errors, FieldError{
335-
FieldPath: fieldPath,
336-
Code: ErrCodeInvalidTag,
337-
Message: fmt.Sprintf("invalid min constraint %q: must be a valid integer", tags.min),
338-
})
339-
} else if length < minLen {
365+
if err == nil && length < minLen {
340366
errors = append(errors, FieldError{
341367
FieldPath: fieldPath,
342368
Code: ErrCodeMin,
@@ -347,13 +373,7 @@ func validateStringMinMax(fieldValue reflect.Value, fieldPath string, tags tagCo
347373

348374
if tags.max != "" {
349375
maxLen, err := strconv.Atoi(tags.max)
350-
if err != nil {
351-
errors = append(errors, FieldError{
352-
FieldPath: fieldPath,
353-
Code: ErrCodeInvalidTag,
354-
Message: fmt.Sprintf("invalid max constraint %q: must be a valid integer", tags.max),
355-
})
356-
} else if length > maxLen {
376+
if err == nil && length > maxLen {
357377
errors = append(errors, FieldError{
358378
FieldPath: fieldPath,
359379
Code: ErrCodeMax,

‎validate_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,3 +572,28 @@ func TestValidateField_ParseErrors(t *testing.T) {
572572
t.Errorf("unexpected message: %s", errors[0].Message)
573573
}
574574
}
575+
576+
func TestValidateField_InvalidMinMax_EarlyReturnPath(t *testing.T) {
577+
// Malformed min/max must be reported even when value validation is skipped
578+
// (e.g., required field with zero value triggers early return).
579+
tags := tagConfig{required: true, min: "abc"}
580+
fieldValue := reflect.ValueOf(0)
581+
errors := validateFieldWithPresence(fieldValue, "TestField", tags, nil)
582+
583+
hasRequired := false
584+
hasInvalidTag := false
585+
for _, e := range errors {
586+
if e.Code == ErrCodeRequired {
587+
hasRequired = true
588+
}
589+
if e.Code == ErrCodeInvalidTag {
590+
hasInvalidTag = true
591+
}
592+
}
593+
if !hasRequired {
594+
t.Error("expected required error")
595+
}
596+
if !hasInvalidTag {
597+
t.Error("expected invalid_tag error for malformed min, even on early return")
598+
}
599+
}

0 commit comments

Comments
 (0)