omissis / go-jsonschema

A tool to generate Go data types from JSON Schema definitions.
MIT License
563 stars 90 forks source link

Incorrect string length validation for some strings. #183

Open ofw opened 8 months ago

ofw commented 8 months ago

The code uses the len check if min_lenth/max_length specified in schema. It should use utf8.RuneCountInString from unicode/utf8 instead because len("ó") == 2 while utf8.RuneCountInString("ó") == 1 which is right.

I tried to fix it but the tests fail for wrong indents in compared files. You can fix it either yourself or tell me how to make the generated files well formatted. Thanks in advance!

Here is my fix:

--- a/pkg/generator/schema_generator.go
+++ b/pkg/generator/schema_generator.go
@@ -266,6 +266,13 @@ func (g *schemaGenerator) generateDeclaredType(
                }
            }

+           for _, v := range validators {
+               if sv, ok := v.(*stringValidator); ok && (sv.minLength != 0 || sv.maxLength != 0) {
+                   g.output.file.Package.AddImport("unicode/utf8", "")
+                   break
+               }
+           }
+
            for _, formatter := range g.formatters {
                formatter := formatter

diff --git a/pkg/generator/validator.go b/pkg/generator/validator.go
index bd8d1fb..ff93599 100644
--- a/pkg/generator/validator.go
+++ b/pkg/generator/validator.go
@@ -236,7 +236,7 @@ func (v *stringValidator) generate(out *codegen.Emitter) {
    }

    if v.minLength != 0 {
-       out.Printlnf(`if %slen(%s%s) < %d {`, checkPointer, pointerPrefix, value, v.minLength)
+       out.Printlnf(`if %sutf8.RuneCountInString(%s%s) < %d {`, checkPointer, pointerPrefix, value, v.minLength)
        out.Indent(1)
        out.Printlnf(`return fmt.Errorf("field %%s length: must be >= %%d", "%s", %d)`, fieldName, v.minLength)
        out.Indent(-1)
@@ -244,7 +244,7 @@ func (v *stringValidator) generate(out *codegen.Emitter) {
    }

    if v.maxLength != 0 {
-       out.Printlnf(`if %slen(%s%s) > %d {`, checkPointer, pointerPrefix, value, v.maxLength)
+       out.Printlnf(`if %sutf8.RuneCountInString(%s%s) > %d {`, checkPointer, pointerPrefix, value, v.maxLength)
        out.Indent(1)
        out.Printlnf(`return fmt.Errorf("field %%s length: must be <= %%d", "%s", %d)`, fieldName, v.maxLength)
        out.Indent(-1)
diff --git a/tests/data/validation/maxLength/maxLength.go b/tests/data/validation/maxLength/maxLength.go
index 92cf124..1649e72 100644
--- a/tests/data/validation/maxLength/maxLength.go
+++ b/tests/data/validation/maxLength/maxLength.go
@@ -4,6 +4,7 @@ package test

 import "encoding/json"
 import "fmt"
+import "unicode/utf8"

 type MaxLength struct {
    // MyNullableString corresponds to the JSON schema field "myNullableString".
@@ -30,7 +31,7 @@ func (j *MaxLength) UnmarshalJSON(b []byte) error {
    if plain.MyNullableString != nil && len(*plain.MyNullableString) > 10 {
        return fmt.Errorf("field %s length: must be <= %d", "myNullableString", 10)
    }
-   if len(plain.MyString) > 5 {
+   if utf8.RuneCountInString(plain.MyString) > 5 {
        return fmt.Errorf("field %s length: must be <= %d", "myString", 5)
    }
    *j = MaxLength(plain)
diff --git a/tests/data/validation/minLength/minLength.go b/tests/data/validation/minLength/minLength.go
index 2502034..94b5c16 100644
--- a/tests/data/validation/minLength/minLength.go
+++ b/tests/data/validation/minLength/minLength.go
@@ -4,6 +4,7 @@ package test

 import "encoding/json"
 import "fmt"
+import "unicode/utf8"

 type MinLength struct {
    // MyNullableString corresponds to the JSON schema field "myNullableString".
@@ -30,7 +31,7 @@ func (j *MinLength) UnmarshalJSON(b []byte) error {
    if plain.MyNullableString != nil && len(*plain.MyNullableString) < 10 {
        return fmt.Errorf("field %s length: must be >= %d", "myNullableString", 10)
    }
-   if len(plain.MyString) < 5 {
+   if utf8.RuneCountInString(plain.MyString) < 5 {
        return fmt.Errorf("field %s length: must be >= %d", "myString", 5)
    }
    *j = MinLength(plain)
omissis commented 8 months ago

Hi @ofw , thanks for the heads up! I will look at this as soon as possible. 🙏