Fix panics from symbolParserV2 (#338)

The added test case contains an escaped multi-byte string that's valid UTF8 that caused a panic before the fix. The fix applied is to advance the entire multi-byte sequence instead of capping it to a single byte.

This caused a few other test failures, namely because we were validating unescaped identifiers were valid UTF8 where elsewhere we assume they're ASCII. The follow-up fix applied was to treat these in a uniform way.

Author: efritz

bindings/go/scip/symbol_parser.go

@@ -3,7 +3,6 @@ package scip
 import (
 	"fmt"
 	"strings"
-	"unicode"
 
 	"github.com/cockroachdb/errors"
 	"github.com/sourcegraph/beaut"
@@ -469,10 +468,6 @@ func (e unrecognizedDescriptorError) Error() string {
 	return fmt.Sprintf("unrecognized descriptor %q", e.value)
 }
 
-func isIdentifierCharacter(r rune) bool {
-	return unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' || r == '+' || r == '$' || r == '_'
-}
-
 func (z *symbolParserV2) advanceOneByte(b byte) {
 	assert(z.currentRune == rune(b), "passed in byte does not match current rune")
 	nextRune, nextRuneByteLength := z.peekNext()
@@ -484,7 +479,7 @@ func (z *symbolParserV2) advanceOneByte(b byte) {
 
 func (z *symbolParserV2) advanceRune() {
 	nextRune, nextRuneByteLength := z.peekNext()
-	z.advance(nextRune, min(nextRuneByteLength, 1))
+	z.advance(nextRune, nextRuneByteLength)
 }
 
 func (z *symbolParserV2) acceptOneByte(b byte, what parseCtx) error {
@@ -503,7 +498,7 @@ func (z *symbolParserV2) acceptIdentifier(what parseCtx, sw *stringWriter) error
 	start := z.byteIndex
 	slen := len(z.SymbolString)
 	for z.byteIndex < slen {
-		if !isIdentifierCharacter(z.currentRune) {
+		if !shared.IsSimpleIdentifierCharacter(z.currentRune) {
 			break
 		}
 		z.advanceRune()

bindings/go/scip/symbol_test.go

@@ -108,6 +108,19 @@ func TestParseSymbol(t *testing.T) {
 				},
 			},
 		},
+		{
+			Symbol: "a b c d `F⃗`.", Expected: &Symbol{
+				Scheme: "a",
+				Package: &Package{
+					Manager: "b",
+					Name:    "c",
+					Version: "d",
+				},
+				Descriptors: []*Descriptor{{
+					Name: "F⃗", Suffix: Descriptor_Term,
+				}},
+			},
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.Symbol, func(t *testing.T) {
@@ -127,6 +140,8 @@ func TestParseSymbolError(t *testing.T) {
 		"lsif-java maven package 1.0.0 java/io/File#Entry.trailingstring",
 		"lsif-java maven package 1.0.0 java/io/File#Entry.unrecognizedSuffix@",
 		"lsif-java maven package 1.0.0 java/io/File#Entry.nonSimpλeIdentifier.",
+		"lsif-java maven package 1.0.0 java/io/File#Entry.`unterminatedEscapedIdentifier",
+		"lsif-java maven package 1.0.0 java/io/File#Entry.[UnterminatedDescriptorSuffix",
 		"local 🧠",
 		"local ",
 		"local &&&",