encoding/prototext: Support URL chars in type URLs in text-format.

Change the text-format parser to allow for more characters and formats
in the type URL prefixes of expanded Any protos. This follows a recent
change to the text-format spec [1,2] and the Go implementation is now
almost exactly implementing this spec (with some exceptions to ensure
backwards compatibility).

Note that we are also making the whitespace handling between [ and ]
more lenient compared to the current implementation. This is also
in line with the new spec.

Refs:
- [1] https://protobuf.dev/reference/protobuf/textformat-spec/#characters
- [2] https://protobuf.dev/reference/protobuf/textformat-spec/#field-names
- go/protobuf-improved-type-url-support

Change-Id: I55055becec0248c1d161e776f1937eaaa4af2066
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/726780
Reviewed-by: Michael Stapelberg <stapelberg@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>

Author: ferstlf

encoding/prototext/decode_test.go

@@ -1554,6 +1554,13 @@ value: "some bytes"
 		wantMessage: &anypb.Any{
 			TypeUrl: "foo.com/pb2.Nested",
 		},
+	}, {
+		desc:         "Any expanded with URL chars in type URL prefix",
+		inputMessage: &anypb.Any{},
+		inputText:    `[foo.com/bar//=*+./pb2.Nested]: {}`,
+		wantMessage: &anypb.Any{
+			TypeUrl: "foo.com/bar//=*+./pb2.Nested",
+		},
 	}, {
 		desc:         "Any expanded with missing required",
 		inputMessage: &anypb.Any{},

internal/encoding/text/decode.go

@@ -424,67 +424,94 @@ func (d *Decoder) parseFieldName() (tok Token, err error) {
 	return Token{}, d.newSyntaxError("invalid field name: %s", errId(d.in))
 }
 
-// parseTypeName parses Any type URL or extension field name. The name is
-// enclosed in [ and ] characters. The C++ parser does not handle many legal URL
-// strings. This implementation is more liberal and allows for the pattern
-// ^[-_a-zA-Z0-9]+([./][-_a-zA-Z0-9]+)*`). Whitespaces and comments are allowed
-// in between [ ], '.', '/' and the sub names.
+// parseTypeName parses an Any type URL or an extension field name. The name is
+// enclosed in [ and ] characters. We allow almost arbitrary type URL prefixes,
+// closely following the text-format spec [1,2]. We implement "ExtensionName |
+// AnyName" as follows (with some exceptions for backwards compatibility):
+//
+// char      = [-_a-zA-Z0-9]
+// url_char  = char | [.~!$&'()*+,;=] | "%", hex, hex
+//
+// Ident         = char, { char }
+// TypeName      = Ident, { ".", Ident } ;
+// UrlPrefix     = url_char, { url_char | "/" } ;
+// ExtensionName = "[", TypeName, "]" ;
+// AnyName       = "[", UrlPrefix, "/", TypeName, "]" ;
+//
+// Additionally, we allow arbitrary whitespace and comments between [ and ].
+//
+// [1] https://protobuf.dev/reference/protobuf/textformat-spec/#characters
+// [2] https://protobuf.dev/reference/protobuf/textformat-spec/#field-names
 func (d *Decoder) parseTypeName() (Token, error) {
-	startPos := len(d.orig) - len(d.in)
 	// Use alias s to advance first in order to use d.in for error handling.
-	// Caller already checks for [ as first character.
+	// Caller already checks for [ as first character (d.in[0] == '[').
 	s := consume(d.in[1:], 0)
 	if len(s) == 0 {
 		return Token{}, ErrUnexpectedEOF
 	}
 
+	// Collect everything between [ and ] in name.
 	var name []byte
-	for len(s) > 0 && isTypeNameChar(s[0]) {
-		name = append(name, s[0])
-		s = s[1:]
-	}
-	s = consume(s, 0)
-
 	var closed bool
 	for len(s) > 0 && !closed {
 		switch {
 		case s[0] == ']':
 			s = s[1:]
 			closed = true
 
-		case s[0] == '/', s[0] == '.':
-			if len(name) > 0 && (name[len(name)-1] == '/' || name[len(name)-1] == '.') {
-				return Token{}, d.newSyntaxError("invalid type URL/extension field name: %s",
-					d.orig[startPos:len(d.orig)-len(s)+1])
-			}
+		case s[0] == '/' || isTypeNameChar(s[0]) || isUrlExtraChar(s[0]):
 			name = append(name, s[0])
-			s = s[1:]
-			s = consume(s, 0)
-			for len(s) > 0 && isTypeNameChar(s[0]) {
-				name = append(name, s[0])
-				s = s[1:]
+			s = consume(s[1:], 0)
+
+		// URL percent-encoded chars
+		case s[0] == '%':
+			if len(s) < 3 || !isHexChar(s[1]) || !isHexChar(s[2]) {
+				return Token{}, d.parseTypeNameError(s, 3)
 			}
-			s = consume(s, 0)
+			name = append(name, s[0], s[1], s[2])
+			s = consume(s[3:], 0)
 
 		default:
-			return Token{}, d.newSyntaxError(
-				"invalid type URL/extension field name: %s", d.orig[startPos:len(d.orig)-len(s)+1])
+			return Token{}, d.parseTypeNameError(s, 1)
 		}
 	}
 
 	if !closed {
 		return Token{}, ErrUnexpectedEOF
 	}
 
-	// First character cannot be '.'. Last character cannot be '.' or '/'.
-	size := len(name)
-	if size == 0 || name[0] == '.' || name[size-1] == '.' || name[size-1] == '/' {
-		return Token{}, d.newSyntaxError("invalid type URL/extension field name: %s",
-			d.orig[startPos:len(d.orig)-len(s)])
+	// Split collected name on last '/' into urlPrefix and typeName (if '/' is
+	// present).
+	typeName := name
+	if i := bytes.LastIndexByte(name, '/'); i != -1 {
+		urlPrefix := name[:i]
+		typeName = name[i+1:]
+
+		// urlPrefix may be empty (for backwards compatibility).
+		// If non-empty, it must not start with '/'.
+		if len(urlPrefix) > 0 && urlPrefix[0] == '/' {
+			return Token{}, d.parseTypeNameError(s, 0)
+		}
 	}
 
+	// typeName must not be empty (note: "" splits to [""]) and all identifier
+	// parts must not be empty.
+	for _, ident := range bytes.Split(typeName, []byte{'.'}) {
+		if len(ident) == 0 {
+			return Token{}, d.parseTypeNameError(s, 0)
+		}
+	}
+
+	// typeName must not contain any percent-encoded or special URL chars.
+	for _, b := range typeName {
+		if b == '%' || (b != '.' && isUrlExtraChar(b)) {
+			return Token{}, d.parseTypeNameError(s, 0)
+		}
+	}
+
+	startPos := len(d.orig) - len(d.in)
+	endPos := len(d.orig) - len(s)
 	d.in = s
-	endPos := len(d.orig) - len(d.in)
 	d.consume(0)
 
 	return Token{
@@ -496,16 +523,32 @@ func (d *Decoder) parseTypeName() (Token, error) {
 	}, nil
 }
 
+func (d *Decoder) parseTypeNameError(s []byte, numUnconsumedChars int) error {
+	return d.newSyntaxError(
+		"invalid type URL/extension field name: %s",
+		d.in[:len(d.in)-len(s)+min(numUnconsumedChars, len(s))],
+	)
+}
+
+func isHexChar(b byte) bool {
+	return ('0' <= b && b <= '9') ||
+		('a' <= b && b <= 'f') ||
+		('A' <= b && b <= 'F')
+}
+
 func isTypeNameChar(b byte) bool {
-	return (b == '-' || b == '_' ||
+	return b == '-' || b == '_' ||
 		('0' <= b && b <= '9') ||
 		('a' <= b && b <= 'z') ||
-		('A' <= b && b <= 'Z'))
+		('A' <= b && b <= 'Z')
 }
 
-func isWhiteSpace(b byte) bool {
+// isUrlExtraChar complements isTypeNameChar with extra characters that we allow
+// in URLs but not in type names. Note that '/' is not included so that it can
+// be treated specially.
+func isUrlExtraChar(b byte) bool {
 	switch b {
-	case ' ', '\n', '\r', '\t':
+	case '.', '~', '!', '$', '&', '(', ')', '*', '+', ',', ';', '=':
 		return true
 	default:
 		return false

internal/encoding/text/decode_test.go

@@ -408,6 +408,14 @@ func TestDecoder(t *testing.T) {
 				{K: text.Name, T: NT{K: text.TypeName, S: "type"}, RS: "[type]"},
 			},
 		},
+		{
+			in:   "[",
+			want: []R{{E: text.ErrUnexpectedEOF.Error()}},
+		},
+		{
+			in:   "[]",
+			want: []R{{E: "invalid type URL/extension field name: []"}},
+		},
 		{
 			// V1 allows this syntax. C++ does not, however, C++ also fails if
 			// field is Any and does not contain '/'.
@@ -416,6 +424,10 @@ func TestDecoder(t *testing.T) {
 				{K: text.Name, T: NT{K: text.TypeName, S: "/type"}},
 			},
 		},
+		{
+			in:   "[/domain.com/type]",
+			want: []R{{E: "invalid type URL/extension field name: [/domain.com/type]"}},
+		},
 		{
 			in:   "[.type]",
 			want: []R{{E: "invalid type URL/extension field name: [.type]"}},
@@ -451,6 +463,25 @@ func TestDecoder(t *testing.T) {
 				},
 			},
 		},
+		{
+			// V3 allows more valid URL chars
+			in: "[url.special.chars/.~!$&()*+,;=/type]",
+			want: []R{
+				{
+					K: text.Name,
+					T: NT{K: text.TypeName, S: "url.special.chars/.~!$&()*+,;=/type"},
+				},
+			},
+		},
+		{
+			// V3 allows multiple slashes and empty path segments in URL prefix.
+			in: `message_field{[val/id//type]`,
+			want: []R{
+				{K: text.Name},
+				{K: text.MessageOpen},
+				{K: text.Name, T: NT{K: text.TypeName, S: "val/id//type"}},
+			},
+		},
 		{
 			// V2 no longer allows a quoted string for the Any type URL.
 			in:   `["domain.com/pkg.type"]`,
@@ -466,14 +497,18 @@ func TestDecoder(t *testing.T) {
 			want: []R{{E: "invalid type URL/extension field name: [pkg.Foo.extension_field:"}},
 		},
 		{
-			// V2 no longer allows whitespace within identifier "word".
-			in:   "[proto.packa ge.field]",
-			want: []R{{E: "invalid type URL/extension field name: [proto.packa g"}},
+			// V3 allows whitespace anywhere between [ and ].
+			in: "[ do\tmain.\ncom/pro\rto.packa ge.field ]",
+			want: []R{
+				{K: text.Name, T: NT{K: text.TypeName, S: "domain.com/proto.package.field"}},
+			},
 		},
 		{
-			// V2 no longer allows comments within identifier "word".
-			in:   "[proto.packa # comment\n ge.field]",
-			want: []R{{E: "invalid type URL/extension field name: [proto.packa # comment\n g"}},
+			// V3 allows comments anywhere between [ and ].
+			in: "[# comment\n domain# comment\n .com/proto.packa # comment\n ge.field# comment\n]",
+			want: []R{
+				{K: text.Name, T: NT{K: text.TypeName, S: "domain.com/proto.package.field"}},
+			},
 		},
 		{
 			in:   "[proto.package.]",
@@ -492,12 +527,48 @@ func TestDecoder(t *testing.T) {
 			},
 		},
 		{
-			in: `message_field{[invalid//type]`,
-			want: []R{
-				{K: text.Name},
-				{K: text.MessageOpen},
-				{E: `invalid type URL/extension field name: [invalid//`},
-			},
+			// V3 allows URL percent-encoded chars
+			in: "[percent.encoded/%0a%1B%2c%3D%4e%F5%A6%b7%C8%f9/type]",
+			want: []R{{
+				K: text.Name,
+				T: NT{K: text.TypeName, S: "percent.encoded/%0a%1B%2c%3D%4e%F5%A6%b7%C8%f9/type"},
+			}},
+		},
+		{
+			in:   `[percent.encode.incomplete%/type]`,
+			want: []R{{E: `invalid type URL/extension field name: [percent.encode.incomplete%`}},
+		},
+		{
+			in:   `[percent.encode.nohex%Z/type]`,
+			want: []R{{E: `invalid type URL/extension field name: [percent.encode.nohex%Z`}},
+		},
+		{
+			in:   `[percent.encode.nohex%aZ/type]`,
+			want: []R{{E: `invalid type URL/extension field name: [percent.encode.nohex%aZ`}},
+		},
+		{
+			in:   `[percent.encode.eof%`,
+			want: []R{{E: `invalid type URL/extension field name: [percent.encode.eof%`}},
+		},
+		{
+			in:   `[percent.encode.eof%a`,
+			want: []R{{E: `invalid type URL/extension field name: [percent.encode.eof%a`}},
+		},
+		{
+			in:   `[invalid.type.char/type!]`,
+			want: []R{{E: `invalid type URL/extension field name: [invalid.type.char/type!`}},
+		},
+		{
+			in:   `[invalid.type.char/type=]`,
+			want: []R{{E: `invalid type URL/extension field name: [invalid.type.char/type=`}},
+		},
+		{
+			in:   `[invalid.type.char/type+]`,
+			want: []R{{E: `invalid type URL/extension field name: [invalid.type.char/type+`}},
+		},
+		{
+			in:   `[invalid.type.char/type%2b]`,
+			want: []R{{E: `invalid type URL/extension field name: [invalid.type.char/type%`}},
 		},
 		{
 			in: `message_field{[proto.package.]`,