Repository: amzn/ion-go
Pull request #130 (2020-08-12 16:38:44)

Fix malformed clobs
This is to fix various cases where a ```clob``` should be considered invalid:
- Check for ```\u``` and ```\U``` in [this commit](402a469).
- Check for non 7-bit ASCII characters in [this commit](d946a09)
- Check for comments inside a clob in [this commit](a4cda0f)




Patch:
diff --git a/ion/skipper.go b/ion/skipper.go
index 3e91f74..deeef9b 100644
--- a/ion/skipper.go
+++ b/ion/skipper.go
@@ -804,6 +804,12 @@ func stopForCommentsHandler() (bool, error) {
 	return false, nil
 }
 
+// ensureNoCommentsHandler is a commentHandler that returns an
+// error if any comments are found, else no error is returned.
+func (t *tokenizer) ensureNoCommentsHandler() (bool, error) {
+	return false, &UnexpectedTokenError{"comments are not allowed within a clob", t.Pos() - 1}
+}
+
 // SkipCommentsHandler is a commentHandler that skips over any
 // comments it finds.
 func (t *tokenizer) skipCommentsHandler() (bool, error) {
diff --git a/ion/tokenizer.go b/ion/tokenizer.go
index 5a7305b..0362b64 100644
--- a/ion/tokenizer.go
+++ b/ion/tokenizer.go
@@ -59,6 +59,9 @@ const (
 	tokenCloseDoubleBrace // }}
 )
 
+const clobText = true
+const nonClobText = false
+
 func (t token) String() string {
 	switch t {
 	case tokenError:
@@ -360,9 +363,9 @@ func (t *tokenizer) ReadValue(tok token) (string, error) {
 	case tokenSymbolOperator, tokenDot:
 		str, err = t.readOperator()
 	case tokenString:
-		str, err = t.readString()
+		str, err = t.readString(nonClobText)
 	case tokenLongString:
-		str, err = t.readLongString()
+		str, err = t.readLongString(nonClobText)
 	case tokenBinary:
 		str, err = t.readBinary()
 	case tokenHex:
@@ -542,7 +545,7 @@ func (t *tokenizer) readQuotedSymbol() (string, error) {
 				continue
 			}
 
-			r, err := t.readEscapedChar(false)
+			r, err := t.readEscapedChar(nonClobText)
 			if err != nil {
 				return "", err
 			}
@@ -578,7 +581,7 @@ func (t *tokenizer) readOperator() (string, error) {
 }
 
 // ReadString reads a quoted string.
-func (t *tokenizer) readString() (string, error) {
+func (t *tokenizer) readString(isClob bool) (string, error) {
 	ret := strings.Builder{}
 
 	for {
@@ -587,7 +590,7 @@ func (t *tokenizer) readString() (string, error) {
 			return "", err
 		}
 		// -1 denotes EOF, and new lines are not allowed in short string
-		if c == -1 || c == '\n' || isProhibitedControlChar(c) {
+		if c == -1 || c == '\n' || isProhibitedControlChar(c) || (isClob && !isASCII(c)) {
 			return "", t.invalidChar(c)
 		}
 
@@ -609,7 +612,7 @@ func (t *tokenizer) readString() (string, error) {
 				continue
 			}
 
-			r, err := t.readEscapedChar(false)
+			r, err := t.readEscapedChar(isClob)
 			if err != nil {
 				return "", err
 			}
@@ -622,7 +625,7 @@ func (t *tokenizer) readString() (string, error) {
 }
 
 // ReadLongString reads a triple-quoted string.
-func (t *tokenizer) readLongString() (string, error) {
+func (t *tokenizer) readLongString(isClob bool) (string, error) {
 	ret := strings.Builder{}
 
 	for {
@@ -631,14 +634,15 @@ func (t *tokenizer) readLongString() (string, error) {
 			return "", err
 		}
 		// -1 denotes EOF
-		if c == -1 || isProhibitedControlChar(c) {
+		if c == -1 || isProhibitedControlChar(c) || (isClob && !isASCII(c)) {
 			return "", t.invalidChar(c)
 		}
 
 		switch c {
 		case '\'':
 			startPosition := t.pos
-			ok, err := t.skipEndOfLongString(t.skipCommentsHandler)
+			handler := getLongStringCommentsHandler(t, isClob)
+			ok, err := t.skipEndOfLongString(handler)
 			if err != nil {
 				return "", err
 			}
@@ -663,7 +667,7 @@ func (t *tokenizer) readLongString() (string, error) {
 				continue
 			}
 
-			r, err := t.readEscapedChar(false)
+			r, err := t.readEscapedChar(isClob)
 			if err != nil {
 				return "", err
 			}
@@ -676,7 +680,7 @@ func (t *tokenizer) readLongString() (string, error) {
 }
 
 // ReadEscapedChar reads an escaped character.
-func (t *tokenizer) readEscapedChar(clob bool) (rune, error) {
+func (t *tokenizer) readEscapedChar(isClob bool) (rune, error) {
 	// We just read the '\', grab the next char.
 	c, err := t.read()
 	if err != nil {
@@ -711,11 +715,14 @@ func (t *tokenizer) readEscapedChar(clob bool) (rune, error) {
 	case '\\':
 		return '\\', nil
 	case 'U':
-		if clob {
+		if isClob {
 			return 0, t.invalidChar('U')
 		}
 		return t.readHexEscapeSeq(8)
 	case 'u':
+		if isClob {
+			return 0, t.invalidChar('u')
+		}
 		return t.readHexEscapeSeq(4)
 	case 'x':
 		return t.readHexEscapeSeq(2)
@@ -1058,7 +1065,7 @@ func (t *tokenizer) ReadBlob() (string, error) {
 }
 
 func (t *tokenizer) ReadShortClob() (string, error) {
-	str, err := t.readString()
+	str, err := t.readString(clobText)
 	if err != nil {
 		return "", err
 	}
@@ -1083,7 +1090,7 @@ func (t *tokenizer) ReadShortClob() (string, error) {
 }
 
 func (t *tokenizer) ReadLongClob() (string, error) {
-	str, err := t.readLongString()
+	str, err := t.readLongString(clobText)
 	if err != nil {
 		return "", err
 	}
@@ -1383,3 +1390,15 @@ func isNewLineChar(c int) bool {
 	return c == 0x0A || //new line
 		c == 0x0D //carriage return
 }
+
+// isASCII returns true if c is a 7-bit ASCII character.
+func isASCII(c int) bool {
+	return c < 0x80
+}
+
+func getLongStringCommentsHandler(t *tokenizer, isClob bool) commentHandler {
+	if isClob {
+		return t.ensureNoCommentsHandler
+	}
+	return t.skipCommentsHandler
+}


Test patch:
diff --git a/ion/integration_test.go b/ion/integration_test.go
index 8633c9c..73d24ff 100644
--- a/ion/integration_test.go
+++ b/ion/integration_test.go
@@ -109,18 +109,6 @@ var malformedIonsSkipList = []string{
 	"annotationNested.10n",
 	"annotationSymbolIDUnmapped.10n",
 	"annotationSymbolIDUnmapped.ion",
-	"clobWithLongLiteralBlockCommentAtEnd.ion",
-	"clobWithLongLiteralCommentsInMiddle.ion",
-	"clobWithNonAsciiCharacter.ion",
-	"clobWithNonAsciiCharacterMultiline.ion",
-	"clobWithValidUtf8ButNonAsciiCharacter.ion",
-	"clob_2.ion",
-	"clob_U0000003F.ion",
-	"clob_U00000080.ion",
-	"clob_U0000013F.ion",
-	"clob_u0020.ion",
-	"clob_u00FF.ion",
-	"clob_u01FF.ion",
 	"emptyAnnotatedInt.10n",
 	"fieldNameSymbolIDUnmapped.10n",
 	"fieldNameSymbolIDUnmapped.ion",