From 2eaa475f8853f2b01ac5591421dcabd7a44f79ce Mon Sep 17 00:00:00 2001 From: George Pollard Date: Wed, 17 Apr 2024 12:22:10 +1200 Subject: [PATCH] Author fuzz tests and fix a comment-related issue (#2935) Co-authored-by: James Newton-King --- Build/build.ps1 | 4 + Src/Newtonsoft.Json.FuzzTests/FuzzTests.cs | 85 ++++++++++++ .../Newtonsoft.Json.FuzzTests.csproj | 21 +++ .../FuzzRegressionTests.cs | 127 ++++++++++++++++++ .../JsonTextWriterTest.cs | 19 ++- Src/Newtonsoft.Json.sln | 14 ++ Src/Newtonsoft.Json/JsonTextWriter.cs | 25 +++- 7 files changed, 288 insertions(+), 7 deletions(-) create mode 100644 Src/Newtonsoft.Json.FuzzTests/FuzzTests.cs create mode 100644 Src/Newtonsoft.Json.FuzzTests/Newtonsoft.Json.FuzzTests.csproj create mode 100644 Src/Newtonsoft.Json.Tests/FuzzRegressionTests.cs diff --git a/Build/build.ps1 b/Build/build.ps1 index 5ae7929c..cbe08679 100644 --- a/Build/build.ps1 +++ b/Build/build.ps1 @@ -133,6 +133,10 @@ task Package -depends Build { robocopy $sourceDir $workingDir\Package\Source\Src /MIR /NFL /NDL /NJS /NC /NS /NP /XD bin obj TestResults AppPackages .vs artifacts /XF *.suo *.user *.lock.json | Out-Default robocopy $buildDir $workingDir\Package\Source\Build /MIR /NFL /NDL /NJS /NC /NS /NP /XD Temp /XF runbuild.txt | Out-Default robocopy $docDir $workingDir\Package\Source\Doc /MIR /NFL /NDL /NJS /NC /NS /NP | Out-Default + + # include fuzz tests in ADO pipeline artifacts + mkdir $workingDir\FuzzTests + Copy-Item -Path $sourceDir\Newtonsoft.Json.FuzzTests\bin\Release\net6.0\* -Destination $workingDir\FuzzTests Compress-Archive -Path $workingDir\Package\* -DestinationPath $workingDir\$zipFileName } diff --git a/Src/Newtonsoft.Json.FuzzTests/FuzzTests.cs b/Src/Newtonsoft.Json.FuzzTests/FuzzTests.cs new file mode 100644 index 00000000..51c1916d --- /dev/null +++ b/Src/Newtonsoft.Json.FuzzTests/FuzzTests.cs @@ -0,0 +1,85 @@ +using System; +using System.IO; + +namespace Newtonsoft.Json.FuzzTests +{ + public static class Fuzzers + { + static readonly JsonSerializer jsonSerializer = new(); + + public static void FuzzDeserialization(ReadOnlySpan buffer) + { + try + { + using var ms = new MemoryStream(buffer.ToArray()); + using var sr = new StreamReader(ms); + using var reader = new JsonTextReader(sr); + jsonSerializer.Deserialize(reader); + } + catch (JsonException) + { + // this can be JsonReaderException, JsonWriterException, or JsonSerializationException; + // the latter two can be thrown by deserializing into a json object + return; + // ignore - known/expected exceptions are okay + } + } + + public static void FuzzSerialization(ReadOnlySpan buffer) + { + object? deserialized; + try + { + deserialized = Deserialize(buffer); + } + catch + { + return; + } + + // see if this throws + Serialize(deserialized); + } + + public static void FuzzIdempotent(ReadOnlySpan buffer) + { + string serialized1; + try + { + serialized1 = Serialize(Deserialize(buffer)); + } + catch + { + return; + } + + var serialized2 = Serialize(Deserialize(serialized1)); + if (serialized1 != serialized2) + { + throw new Exception($"not idempotent: {serialized1} {serialized2}"); + } + } + + private static string Serialize(object? o) + { + using var sw1 = new StringWriter(); + jsonSerializer.Serialize(sw1, o); + return sw1.ToString(); + } + + private static object? Deserialize(string input) + { + using var sr = new StringReader(input); + using var tr = new JsonTextReader(sr); + return jsonSerializer.Deserialize(tr); + } + + private static object? Deserialize(ReadOnlySpan bytes) + { + using var ms = new MemoryStream(bytes.ToArray()); + using var sr = new StreamReader(ms); + using var reader = new JsonTextReader(sr); + return jsonSerializer.Deserialize(reader); + } + } +} diff --git a/Src/Newtonsoft.Json.FuzzTests/Newtonsoft.Json.FuzzTests.csproj b/Src/Newtonsoft.Json.FuzzTests/Newtonsoft.Json.FuzzTests.csproj new file mode 100644 index 00000000..552450db --- /dev/null +++ b/Src/Newtonsoft.Json.FuzzTests/Newtonsoft.Json.FuzzTests.csproj @@ -0,0 +1,21 @@ + + + net6.0 + 9.0 + 1.0 + James Newton-King + Newtonsoft + Json.NET + en-US + Copyright © James Newton-King 2008 + Newtonsoft.Json.FuzzTests + Newtonsoft.Json.FuzzTests + false + enable + + false + + + + + diff --git a/Src/Newtonsoft.Json.Tests/FuzzRegressionTests.cs b/Src/Newtonsoft.Json.Tests/FuzzRegressionTests.cs new file mode 100644 index 00000000..e641d117 --- /dev/null +++ b/Src/Newtonsoft.Json.Tests/FuzzRegressionTests.cs @@ -0,0 +1,127 @@ +#region License +// Copyright (c) 2007 James Newton-King +// +// Permission is hereby granted, free of charge, to any person +// obtaining a copy of this software and associated documentation +// files (the "Software"), to deal in the Software without +// restriction, including without limitation the rights to use, +// copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +// OTHER DEALINGS IN THE SOFTWARE. +#endregion + +using System.IO; + +#if DNXCORE50 +using Xunit; +using Test = Xunit.FactAttribute; +using Assert = Newtonsoft.Json.Tests.XUnitAssert; +#else +using NUnit.Framework; +#endif + +#pragma warning disable xUnit1013 + +namespace Newtonsoft.Json.Tests +{ + [TestFixture] + public class FuzzRegressionTests : TestFixtureBase + { + private static readonly JsonSerializer jsonSerializer = new(); + + public static void Roundtrip(string buffer) + { + var serialized1 = Serialize(Deserialize(buffer)); + var serialized2 = Serialize(Deserialize(serialized1)); + Assert.AreEqual(serialized1, serialized2); + } + + private static string Serialize(object o) + { + using var sw1 = new StringWriter(); + jsonSerializer.Serialize(sw1, o); + return sw1.ToString(); + } + + private static object Deserialize(string input) + { + using var sr = new StringReader(input); + using var tr = new JsonTextReader(sr); + return jsonSerializer.Deserialize(tr); + } + + [Test] + public void LineCommentWithStarSlashCases() + { + // Fuzzer found these cases: + // The Json "// comment with */" would be serialized as "/* comment with */*/". + // It is now serialized as "// comment with */". + // + // Ideally this would be a [Theory] but we are targetting both NUnit and XUnit. + // + // This is a list of inputs and where they caused a crash. + // - Note that these are not minimal/reduced inputs. + var cases = new[] { + "[//*/I33/\n91.//3", // ParsePositiveInfinity + "[//*/t3.9/\n91675795,77//3", // ParseTrue + "[//*/f33339/\n91675795878,787,[]//3", // ParseFalse + "[//*/N333331/\n,[]//3", // ParseNumberNaN + "[//*/{/\n,7//J:[@", // ParseProperty + "[//*/06.6/\n,[]//3", // ParseNumber + "[5,78,,[//*/3,3,new 4* ,,,,,,33/\n,,,,[],33,,,,,,17911055//3814", // ParseConstructor + "[//*/---------------------------------------------------------------------------------------------------------16.6/\n,[]//3", // ParseReadNumber + "[//6*/)3333/\n,[]//3", // ValidateEnd + "[//*/{'33/\n,33,33//3", // ReadStringIntoBuffer + "[//*//\n8 ,8 ", // ParseComment + @"[//ô [ / +//""ÿ /'/*//// +//[7 +// +//"" @'/// +//// +//// +//[6 +// +// +//[1 +// "" x + +///*** + //[2 +// +,'/J "" ' ", // WriteToken + "[2//***;****/,7*", // ReadNumberCharIntoBuffer + "[2///ÿ¢ ¢¢********/,,* ", // ParseValue + @"[// *//[ +7// +// ""JJ·  +//',J + +// +//',o@7,7 +//',o@/ / "" ]  +//',o@7,7 +//',o@Ó", // ParseComment + "[2//*/*", // ParsePostValue + "[//*/{A73/\n]1.//3:{\"'\":", // ReadUnquotedPropertyReportIfDone + }; + + foreach (var c in cases) { + Roundtrip(c); + } + } + } +} diff --git a/Src/Newtonsoft.Json.Tests/JsonTextWriterTest.cs b/Src/Newtonsoft.Json.Tests/JsonTextWriterTest.cs index dcab597d..c0b4776f 100644 --- a/Src/Newtonsoft.Json.Tests/JsonTextWriterTest.cs +++ b/Src/Newtonsoft.Json.Tests/JsonTextWriterTest.cs @@ -1699,7 +1699,8 @@ null//comment w.WriteToken(r, true); - StringAssert.AreEqual(@"/*comment*//*hi*/*/{/*comment*/ + StringAssert.AreEqual(@"//comment*//*hi*/ +{/*comment*/ ""Name"": /*comment*/ true/*comment after true*//*comment after comma*/, ""ExpiryDate"": /*comment*/ new Constructor( /*comment*/, @@ -1715,6 +1716,20 @@ null//comment }/*comment *//*comment 1 */", sw.ToString()); } + [Test] + public void NewlinesInSingleLineComments() + { + // it’s not possible for this to be created by parsing JSON, + // but if someone gets creative with the API… + var sw = new StringWriter(); + using (var w = new JsonTextWriter(sw)) + { + w.WriteComment("*/\nsomething else"); + } + + StringAssert.AreEqual("//*/\n//something else\n", sw.ToString()); + } + [Test] public void DisposeSupressesFinalization() { @@ -1820,4 +1835,4 @@ null//comment throw new NotImplementedException(); } } -} \ No newline at end of file +} diff --git a/Src/Newtonsoft.Json.sln b/Src/Newtonsoft.Json.sln index 10feeda9..43b77487 100644 --- a/Src/Newtonsoft.Json.sln +++ b/Src/Newtonsoft.Json.sln @@ -16,6 +16,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Newtonsoft.Json.Tests", "Ne EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Newtonsoft.Json.TestConsole", "Newtonsoft.Json.TestConsole\Newtonsoft.Json.TestConsole.csproj", "{3CC9C2DF-CD0A-4096-BF46-B4AFDF0147D2}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Newtonsoft.Json.FuzzTests", "Newtonsoft.Json.FuzzTests\Newtonsoft.Json.FuzzTests.csproj", "{00867C2B-409D-45B2-A78F-F291B6817F70}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -62,6 +64,18 @@ Global {3CC9C2DF-CD0A-4096-BF46-B4AFDF0147D2}.Release|x64.Build.0 = Release|Any CPU {3CC9C2DF-CD0A-4096-BF46-B4AFDF0147D2}.Release|x86.ActiveCfg = Release|Any CPU {3CC9C2DF-CD0A-4096-BF46-B4AFDF0147D2}.Release|x86.Build.0 = Release|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Debug|Any CPU.Build.0 = Debug|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Debug|x64.ActiveCfg = Debug|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Debug|x64.Build.0 = Debug|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Debug|x86.ActiveCfg = Debug|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Debug|x86.Build.0 = Debug|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Release|Any CPU.ActiveCfg = Release|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Release|Any CPU.Build.0 = Release|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Release|x64.ActiveCfg = Release|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Release|x64.Build.0 = Release|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Release|x86.ActiveCfg = Release|Any CPU + {00867C2B-409D-45B2-A78F-F291B6817F70}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/Src/Newtonsoft.Json/JsonTextWriter.cs b/Src/Newtonsoft.Json/JsonTextWriter.cs index 888bb04d..08f08eeb 100644 --- a/Src/Newtonsoft.Json/JsonTextWriter.cs +++ b/Src/Newtonsoft.Json/JsonTextWriter.cs @@ -784,10 +784,25 @@ namespace Newtonsoft.Json public override void WriteComment(string? text) { InternalWriteComment(); - - _writer.Write("/*"); - _writer.Write(text); - _writer.Write("*/"); + + // if text contains "*/" then it must have been a line comment + if (text != null && text.IndexOf("*/", StringComparison.Ordinal) > -1) + { + // each line must be emitted separately + var parts = text.Split('\n'); + foreach (var part in parts) + { + _writer.Write("//"); + _writer.Write(part); + _writer.Write("\n"); + } + } + else + { + _writer.Write("/*"); + _writer.Write(text); + _writer.Write("*/"); + } } /// @@ -924,4 +939,4 @@ namespace Newtonsoft.Json return totalLength; } } -} \ No newline at end of file +}