Repository: goadesign/goa
Pull request #2754 (2021-01-03 11:08:02)

Fix validation of UUID
Currently the UUID format accepts string like `abcdefgh-ijkl-mnop-qrst-uvqxyz012345` as valid although it contains characters that are not hex digits.

This PR makes is to verify that UUID strings contain hex digits.

raphael: Thank you for the PR! This looks good. Could I ask you to back-port this change to v2 as well? thanks!

Patch:
diff --git a/go.mod b/go.mod
index 307359f3..b201d73d 100644
--- a/go.mod
+++ b/go.mod
@@ -9,6 +9,7 @@ require (
 	github.com/go-openapi/loads v0.19.7
 	github.com/golang/protobuf v1.4.3
 	github.com/google/gxui v0.0.0-20151028112939-f85e0a97b3a4 // indirect
+	github.com/google/uuid v1.1.2
 	github.com/gorilla/websocket v1.4.2
 	github.com/manveru/faker v0.0.0-20171103152722-9fbc68a78c4d
 	github.com/manveru/gobdd v0.0.0-20131210092515-f1a17fdd710b // indirect
diff --git a/pkg/validation.go b/pkg/validation.go
index 7491933f..e29442c5 100644
--- a/pkg/validation.go
+++ b/pkg/validation.go
@@ -1,7 +1,6 @@
 package goa
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
 	"net"
@@ -10,6 +9,8 @@ import (
 	"regexp"
 	"sync"
 	"time"
+
+	googleuuid "github.com/google/uuid"
 )
 
 // Format defines a validation format.
@@ -60,10 +61,8 @@ const (
 )
 
 var (
-	hostnameRegex  = regexp.MustCompile(`^[[:alnum:]][[:alnum:]\-]{0,61}[[:alnum:]]|[[:alpha:]]$`)
-	ipv4Regex      = regexp.MustCompile(`^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$`)
-	uuidURNPrefix  = []byte("urn:uuid:")
-	uuidByteGroups = []int{8, 4, 4, 4, 12}
+	hostnameRegex = regexp.MustCompile(`^[[:alnum:]][[:alnum:]\-]{0,61}[[:alnum:]]|[[:alpha:]]$`)
+	ipv4Regex     = regexp.MustCompile(`^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$`)
 )
 
 // ValidateFormat validates val against f. It returns nil if the string conforms
@@ -166,35 +165,17 @@ func ValidatePattern(name, val, p string) error {
 
 // The following formats are supported:
 // "6ba7b810-9dad-11d1-80b4-00c04fd430c8",
+// "6ba7b8109dad11d180b400c04fd430c8",
 // "{6ba7b810-9dad-11d1-80b4-00c04fd430c8}",
 // "urn:uuid:6ba7b810-9dad-11d1-80b4-00c04fd430c8"
 func validateUUID(uuid string) error {
-	if len(uuid) < 32 {
-		return fmt.Errorf("uuid: UUID string too short: %s", uuid)
-	}
-	t := []byte(uuid)
-	braced := false
-	if bytes.Equal(t[:9], uuidURNPrefix) {
-		t = t[9:]
-	} else if t[0] == '{' {
-		t = t[1:]
-		braced = true
+	u, err := googleuuid.Parse(uuid)
+	if err != nil {
+		return fmt.Errorf("uuid: %s: %v", uuid, err)
 	}
-	for i, byteGroup := range uuidByteGroups {
-		if i > 0 {
-			if t[0] != '-' {
-				return fmt.Errorf("uuid: invalid string format")
-			}
-			t = t[1:]
-		}
-		if len(t) < byteGroup {
-			return fmt.Errorf("uuid: UUID string too short: %s", uuid)
-		}
-		if i == 4 && len(t) > byteGroup &&
-			((braced && t[byteGroup] != '}') || len(t[byteGroup:]) > 1 || !braced) {
-			return fmt.Errorf("uuid: UUID string too long: %s", uuid)
-		}
-		t = t[byteGroup:]
+
+	if u.Variant() != googleuuid.RFC4122 {
+		return fmt.Errorf("uuid: expected RFC4122 format, but got %s", u.Variant().String())
 	}
 
 	return nil


Test patch:
diff --git a/pkg/validation_test.go b/pkg/validation_test.go
index 60fab7b1..5f125242 100644
--- a/pkg/validation_test.go
+++ b/pkg/validation_test.go
@@ -12,13 +12,17 @@ import (
 
 func TestValidateFormat(t *testing.T) {
 	var (
-		validDate       = "2015-10-26"
-		invalidDate     = "201510-26"
-		validDateTime   = "2015-10-26T08:31:23Z"
-		invalidDateTime = "201510-26T08:31:23Z"
-		validUUID       = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"
-		invalidUUID     = "96054a62-a9e45ed26688389b"
-		validEmail      = "raphael@goa.design"
+		validDate              = "2015-10-26"
+		invalidDate            = "201510-26"
+		validDateTime          = "2015-10-26T08:31:23Z"
+		invalidDateTime        = "201510-26T08:31:23Z"
+		validUUID              = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"
+		validUUIDWithBrace     = "{6ba7b810-9dad-11d1-80b4-00c04fd430c8}"
+		validUUIDRaw           = "6ba7b8109dad11d180b400c04fd430c8"
+		validUUIDWithURNPrefix = "urn:uuid:6ba7b810-9dad-11d1-80b4-00c04fd430c8"
+		invalidUUID            = "96054a62-a9e45ed26688389b"
+		invalidUUIDNonHex      = "abcdefgh-ijkl-mnop-qrst-uvqxyz012345" // UUID with characters other than hex digit
+		validEmail             = "raphael@goa.design"
 
 		// Re-enable once CircleCI uses Go 1.13
 		// invalidEmail    = "foo"
@@ -48,13 +52,18 @@ func TestValidateFormat(t *testing.T) {
 		format   Format
 		expected error
 	}{
-		"valid date":        {"validDate", validDate, FormatDate, nil},
-		"invalid date":      {"invalidDate", invalidDate, FormatDate, InvalidFormatError("invalidDate", invalidDate, FormatDate, &time.ParseError{Layout: "2006-01-02", Value: invalidDate, LayoutElem: "-", ValueElem: invalidDate[4:]})},
-		"valid date-time":   {"validDateTime", validDateTime, FormatDateTime, nil},
-		"invalid date-time": {"invalidDateTime", invalidDateTime, FormatDateTime, InvalidFormatError("invalidDateTime", invalidDateTime, FormatDateTime, &time.ParseError{Layout: time.RFC3339, Value: invalidDateTime, LayoutElem: "-", ValueElem: invalidDateTime[4:]})},
-		"valid uuid":        {"validUUID", validUUID, FormatUUID, nil},
-		"invalid uuid":      {"invalidUUID", invalidUUID, FormatUUID, InvalidFormatError("invalidUUID", invalidUUID, FormatUUID, fmt.Errorf("uuid: UUID string too short: %s", invalidUUID))},
-		"valid email":       {"validEmail", validEmail, FormatEmail, nil},
+		"valid date":                 {"validDate", validDate, FormatDate, nil},
+		"invalid date":               {"invalidDate", invalidDate, FormatDate, InvalidFormatError("invalidDate", invalidDate, FormatDate, &time.ParseError{Layout: "2006-01-02", Value: invalidDate, LayoutElem: "-", ValueElem: invalidDate[4:]})},
+		"valid date-time":            {"validDateTime", validDateTime, FormatDateTime, nil},
+		"invalid date-time":          {"invalidDateTime", invalidDateTime, FormatDateTime, InvalidFormatError("invalidDateTime", invalidDateTime, FormatDateTime, &time.ParseError{Layout: time.RFC3339, Value: invalidDateTime, LayoutElem: "-", ValueElem: invalidDateTime[4:]})},
+		"valid uuid":                 {"validUUID", validUUID, FormatUUID, nil},
+		"valid uuid with brace":      {"validUUIDWithBrace", validUUIDWithBrace, FormatUUID, nil},
+		"valid uuid with no dash":    {"validUUIDRaw", validUUIDRaw, FormatUUID, nil},
+		"valid uuid with urn prefix": {"validUUIDWithURNPrefix", validUUIDWithURNPrefix, FormatUUID, nil},
+		"invalid uuid":               {"invalidUUID", invalidUUID, FormatUUID, InvalidFormatError("invalidUUID", invalidUUID, FormatUUID, fmt.Errorf("uuid: %s: invalid UUID length: 25", invalidUUID))},
+		"invalid uuid non hex":       {"invalidUUIDNonHex", invalidUUIDNonHex, FormatUUID, InvalidFormatError("invalidUUIDNonHex", invalidUUIDNonHex, FormatUUID, fmt.Errorf("uuid: %s: invalid UUID format", invalidUUIDNonHex))},
+
+		"valid email": {"validEmail", validEmail, FormatEmail, nil},
 
 		// Re-enable once CircleCI uses Go 1.13
 		// "invalid email":      {"invalidEmail", invalidEmail, FormatEmail, InvalidFormatError("invalidEmail", invalidEmail, FormatEmail, errors.New("mail: missing '@' or angle-addr"))},
@@ -89,7 +98,7 @@ func TestValidateFormat(t *testing.T) {
 		actual := ValidateFormat(tc.name, tc.val, tc.format)
 		if actual != tc.expected {
 			// Compare only the messages because the error has always a new error ID.
-			if actual.Error() != tc.expected.Error() {
+			if actual == nil || tc.expected == nil || actual.Error() != tc.expected.Error() {
 				t.Errorf("%s: got %#v, expected %#v", k, actual, tc.expected)
 			}
 		}