Browse Source

Fix JsonParser error behavior (#1038)

Co-authored-by: Jonathan Resnick <[email protected]>
Jonathan Resnick 3 years ago
parent
commit
7148772983
3 changed files with 84 additions and 39 deletions
  1. 3 3
      Jint.Tests/Runtime/EngineTests.cs
  2. 36 0
      Jint.Tests/Runtime/JsonTests.cs
  3. 45 36
      Jint/Native/Json/JsonParser.cs

+ 3 - 3
Jint.Tests/Runtime/EngineTests.cs

@@ -1006,8 +1006,8 @@ namespace Jint.Tests.Runtime
         [Fact]
         public void JsonParserShouldHandleEmptyString()
         {
-            var ex = Assert.Throws<ParserException>(() => _engine.Evaluate("JSON.parse('');"));
-            Assert.Equal("Line 1: Unexpected end of input", ex.Message);
+            var ex = Assert.Throws<JavaScriptException>(() => _engine.Evaluate("JSON.parse('');"));
+            Assert.Equal("Unexpected end of JSON input at position 0", ex.Message);
         }
 
         [Fact]
@@ -2661,7 +2661,7 @@ function output(x) {
         {
             var engine = new Engine();
             var ex = Assert.Throws<JavaScriptException>(() => engine.Evaluate("JSON.parse('[01]')"));
-            Assert.Equal("Unexpected token '1'", ex.Message);
+            Assert.Equal("Unexpected token '1' in JSON at position 2", ex.Message);
 
             var voidCompletion = engine.Evaluate("try { JSON.parse('01') } catch (e) {}");
             Assert.Equal(JsValue.Undefined, voidCompletion);

+ 36 - 0
Jint.Tests/Runtime/JsonTests.cs

@@ -1,3 +1,5 @@
+using Jint.Native.Json;
+using Jint.Runtime;
 using Xunit;
 
 namespace Jint.Tests.Runtime
@@ -12,5 +14,39 @@ namespace Jint.Tests.Runtime
              var obj = engine.Evaluate(script).AsObject();
              Assert.True(obj.HasOwnProperty("abc\tdef"));
         }
+
+        [Theory]
+        [InlineData("{\"a\":1", "Unexpected end of JSON input at position 6")]
+        [InlineData("{\"a\":1},", "Unexpected token ',' in JSON at position 7")]
+        [InlineData("{1}", "Unexpected number in JSON at position 1")]
+        [InlineData("{\"a\" \"a\"}", "Unexpected string in JSON at position 5")]
+        [InlineData("{true}", "Unexpected token 'true' in JSON at position 1")]
+        [InlineData("{null}", "Unexpected token 'null' in JSON at position 1")]
+        [InlineData("{:}", "Unexpected token ':' in JSON at position 1")]
+        [InlineData("\"\\xah\"", "Expected hexadecimal digit in JSON at position 4")]
+        [InlineData("0123", "Unexpected token '1' in JSON at position 1")]  // leading 0 (octal number) not allowed
+        [InlineData("1e+A", "Unexpected token 'A' in JSON at position 3")]
+        [InlineData("truE", "Unexpected token 'tru' in JSON at position 0")]
+        [InlineData("nul", "Unexpected token 'nul' in JSON at position 0")]
+        [InlineData("\"ab\t\"", "Invalid character in JSON at position 3")] // invalid char in string literal
+        [InlineData("\"ab", "Unexpected end of JSON input at position 3")] // unterminated string literal
+        [InlineData("alpha", "Unexpected token 'a' in JSON at position 0")]
+        [InlineData("[1,\na]", "Unexpected token 'a' in JSON at position 4")] // multiline
+        [InlineData("\x06", "Unexpected token '\x06' in JSON at position 0")] // control char
+        public void ShouldReportHelpfulSyntaxErrorForInvalidJson(string json, string expectedMessage)
+        {
+            var engine = new Engine();
+            var parser = new JsonParser(engine);
+            var ex = Assert.ThrowsAny<JavaScriptException>(() =>
+            {
+                parser.Parse(json);
+            });
+
+            Assert.Equal(expectedMessage, ex.Message);
+
+            var error = ex.Error as Native.Error.ErrorInstance;
+            Assert.NotNull(error);
+            Assert.Equal("SyntaxError", error.Get("name"));
+        }
     }
 }

+ 45 - 36
Jint/Native/Json/JsonParser.cs

@@ -97,7 +97,7 @@ namespace Jint.Native.Json
                 }
                 else
                 {
-                    ExceptionHelper.ThrowSyntaxError(_engine.Realm, $"Expected hexadecimal digit:{_source}");
+                    ThrowError(_index, Messages.ExpectedHexadecimalDigit);
                 }
             }
             return (char)code;
@@ -142,17 +142,19 @@ namespace Jint.Native.Json
                 case 126: // ~
                     ++_index;
 
+                    string value = TypeConverter.ToString(code);
                     return new Token
-                        {
-                            Type = Tokens.Punctuator,
-                            Value = TypeConverter.ToString(code),
-                            LineNumber = _lineNumber,
-                            LineStart = _lineStart,
-                            Range = new[] {start, _index}
-                        };
+                    {
+                        Type = Tokens.Punctuator,
+                        Text = value,
+                        Value = value,
+                        LineNumber = _lineNumber,
+                        LineStart = _lineStart,
+                        Range = new[] { start, _index }
+                    };
             }
 
-            ExceptionHelper.ThrowSyntaxError(_engine.Realm, string.Format(Messages.UnexpectedToken, code));
+            ThrowError(start, Messages.UnexpectedToken, code);
             return null;
         }
 
@@ -182,7 +184,7 @@ namespace Jint.Native.Json
                     // decimal number starts with '0' such as '09' is illegal.
                     if (ch > 0 && IsDecimalDigit(ch))
                     {
-                        ExceptionHelper.ThrowSyntaxError(_engine.Realm, string.Format(Messages.UnexpectedToken, ch));
+                        ThrowError(_index, Messages.UnexpectedToken, ch);
                     }
                 }
 
@@ -221,13 +223,14 @@ namespace Jint.Native.Json
                 }
                 else
                 {
-                    ExceptionHelper.ThrowSyntaxError(_engine.Realm, string.Format(Messages.UnexpectedToken, _source.CharCodeAt(_index)));
+                    ThrowError(_index, Messages.UnexpectedToken, _source.CharCodeAt(_index));
                 }
             }
 
             return new Token
                 {
                     Type = Tokens.Number,
+                    Text = number,
                     Value = Double.Parse(number, NumberStyles.AllowLeadingSign | NumberStyles.AllowDecimalPoint | NumberStyles.AllowExponent, CultureInfo.InvariantCulture),
                     LineNumber = _lineNumber,
                     LineStart = _lineStart,
@@ -250,6 +253,7 @@ namespace Jint.Native.Json
                 return new Token
                 {
                     Type = Tokens.BooleanLiteral,
+                    Text = s,
                     Value = s == "true",
                     LineNumber = _lineNumber,
                     LineStart = _lineStart,
@@ -257,7 +261,7 @@ namespace Jint.Native.Json
                 };
             }
 
-            ExceptionHelper.ThrowSyntaxError(_engine.Realm, string.Format(Messages.UnexpectedToken, s));
+            ThrowError(start, Messages.UnexpectedToken, s);
             return null;
         }
 
@@ -276,6 +280,7 @@ namespace Jint.Native.Json
                 return new Token
                 {
                     Type = Tokens.NullLiteral,
+                    Text = s,
                     Value = Null.Instance,
                     LineNumber = _lineNumber,
                     LineStart = _lineStart,
@@ -283,7 +288,7 @@ namespace Jint.Native.Json
                 };
             }
 
-            ExceptionHelper.ThrowSyntaxError(_engine.Realm, string.Format(Messages.UnexpectedToken, s));
+            ThrowError(start, Messages.UnexpectedToken, s);
             return null;
         }
 
@@ -308,7 +313,7 @@ namespace Jint.Native.Json
 
                 if (ch <= 31)
                 {
-                    ExceptionHelper.ThrowSyntaxError(_engine.Realm, $"Invalid character '{ch}', position:{_index}, string:{_source}");
+                    ThrowError(_index - 1, Messages.InvalidCharacter);
                 }
 
                 if (ch == '\\')
@@ -400,16 +405,19 @@ namespace Jint.Native.Json
 
             if (quote != 0)
             {
-                ExceptionHelper.ThrowSyntaxError(_engine.Realm, string.Format(Messages.UnexpectedToken, _source));
+                // unterminated string literal
+                ThrowError(_index, Messages.UnexpectedEOS);
             }
 
+            string value = sb.ToString();
             return new Token
-                {
+            {
                     Type = Tokens.String,
-                    Value = sb.ToString(),
+                    Text = value,
+                    Value = value,
                     LineNumber = _lineNumber,
                     LineStart = _lineStart,
-                    Range = new[] {start, _index}
+                    Range = new[] { start, _index }
                 };
         }
 
@@ -502,6 +510,7 @@ namespace Jint.Native.Json
                 _extra.Tokens.Add(new Token
                     {
                         Type = token.Type,
+                        Text = value,
                         Value = value,
                         Range = range,
                     });
@@ -604,17 +613,13 @@ namespace Jint.Native.Json
 
         private void ThrowError(Token token, string messageFormat, params object[] arguments)
         {
-            string msg = System.String.Format(messageFormat, arguments);
-            int lineNumber = token.LineNumber ?? _lineNumber;
-
-            var error = new ParseError(
-                    description: msg,
-                    source: _source,
-                    index: token.Range[0],
-                    position: new Position(token.LineNumber ?? _lineNumber, token.Range[0] - _lineStart + 1));
-            var exception = new ParserException("Line " + lineNumber  + ": " + msg, error);
+            ThrowError(token.Range[0], messageFormat, arguments);
+        }
 
-            throw exception;
+        private void ThrowError(int position, string messageFormat, params object[] arguments)
+        {
+            string msg = System.String.Format(messageFormat, arguments);
+            ExceptionHelper.ThrowSyntaxError(_engine.Realm, $"{msg} at position {position}");
         }
 
         // Throw an exception because of the token.
@@ -637,7 +642,7 @@ namespace Jint.Native.Json
             }
 
             // BooleanLiteral, NullLiteral, or Punctuator.
-            ThrowError(token, Messages.UnexpectedToken, token.Value as string);
+            ThrowError(token, Messages.UnexpectedToken, token.Text);
         }
 
         // Expect the next token to match the specified punctuator.
@@ -702,10 +707,11 @@ namespace Jint.Native.Json
                     ThrowUnexpected(Lex());
                 }
 
-                var name = Lex().Value.ToString();
+                var nameToken = Lex();
+                var name = nameToken.Value.ToString();
                 if (PropertyNameContainsInvalidCharacters(name))
                 {
-                    ExceptionHelper.ThrowSyntaxError(_engine.Realm, $"Invalid character in property name '{name}'");
+                    ThrowError(nameToken, Messages.InvalidCharacter);
                 }
 
                 Expect(":");
@@ -828,7 +834,7 @@ namespace Jint.Native.Json
 
                 if(_lookahead.Type != Tokens.EOF)
                 {
-                    ExceptionHelper.ThrowSyntaxError(_engine.Realm, $"Unexpected {_lookahead.Type} {_lookahead.Value}");
+                    ThrowError(_lookahead, Messages.UnexpectedToken, _lookahead.Text);
                 }
                 return jsv;
             }
@@ -860,6 +866,7 @@ namespace Jint.Native.Json
         {
             public Tokens Type;
             public object Value;
+            public string Text;
             public int[] Range;
             public int? LineNumber;
             public int LineStart;
@@ -867,10 +874,12 @@ namespace Jint.Native.Json
 
         static class Messages
         {
-            public const string UnexpectedToken = "Unexpected token '{0}'";
-            public const string UnexpectedNumber = "Unexpected number";
-            public const string UnexpectedString = "Unexpected string";
-            public const string UnexpectedEOS = "Unexpected end of input";
+            public const string InvalidCharacter = "Invalid character in JSON";
+            public const string ExpectedHexadecimalDigit = "Expected hexadecimal digit in JSON";
+            public const string UnexpectedToken = "Unexpected token '{0}' in JSON";
+            public const string UnexpectedNumber = "Unexpected number in JSON";
+            public const string UnexpectedString = "Unexpected string in JSON";
+            public const string UnexpectedEOS = "Unexpected end of JSON input";
         };
 
         struct State