fix: Ensure that special chars in string values don't lead to invalid…

… JSON (atteneder#110) * feat(export): Added development-time checks for valid JSON string literals. * doc: Consistent periods at end of changelog entries. * feat: Added value-safe variants of `AddProperty`/`AddArrayProperty` to `JsonWriter`, which properly escape special characters. * fix: Using invariant culture `ToLower`/`ToUpper` variants on all non-language-specific data.
Unity-Technologies · Mar 5, 2024 · fd8d763 · fd8d763
1 parent 9bee04f
commit fd8d763
Show file tree

Hide file tree

Showing 12 changed files with 283 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,8 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## Unreleased
 
 ## Added
-- Runtime import tests
-- Runtime export tests
+- Runtime import tests.
+- Runtime export tests.
+- (Export) Added development-time checks for valid JSON string literals.
 
 ## Changed
 - Refactored test scripts folder layout.
@@ -21,7 +22,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Removed usage of obsolete APIs in High Definition Render Pipeline version 17 (2023.3)
 - (Export) Area light's range value is exported accurately (as shown in the inspector).
 - Various occasions of `NullReferenceException` when no logger is used/provided.
-- Proper error handling when trying to load unsupported sparse texture coordinates
+- Proper error handling when trying to load unsupported sparse texture coordinates.
+- Ensure that special chars in string values don't lead to invalid JSON.
+- Using invariant culture `ToLower`/`ToUpper` variants on all non-language-specific data.
 
 ## [6.2.0] - 2024-01-29
 

diff --git a/Runtime/Scripts/Export/MaterialExportBase.cs b/Runtime/Scripts/Export/MaterialExportBase.cs
@@ -107,7 +107,7 @@ protected static bool IsDoubleSided(UnityEngine.Material uMaterial, int cullProp
         /// <returns>True if material uses unlit shader, false otherwise</returns>
         protected static bool IsUnlit(UnityEngine.Material material)
         {
-            return material.shader.name.ToLower().Contains("unlit");
+            return material.shader.name.ToLowerInvariant().Contains("unlit");
         }
 
         /// <summary>

diff --git a/Runtime/Scripts/Schema/Asset.cs b/Runtime/Scripts/Schema/Asset.cs
@@ -39,11 +39,11 @@ internal void GltfSerialize(JsonWriter writer)
             }
             if (!string.IsNullOrEmpty(generator))
             {
-                writer.AddProperty("generator", generator);
+                writer.AddPropertySafe("generator", generator);
             }
             if (!string.IsNullOrEmpty(copyright))
             {
-                writer.AddProperty("copyright", copyright);
+                writer.AddPropertySafe("copyright", copyright);
             }
             if (!string.IsNullOrEmpty(minVersion))
             {

diff --git a/Runtime/Scripts/Schema/Buffer.cs b/Runtime/Scripts/Schema/Buffer.cs
@@ -26,7 +26,7 @@ internal void GltfSerialize(JsonWriter writer)
             writer.AddObject();
             if (!string.IsNullOrEmpty(uri))
             {
-                writer.AddProperty("uri", uri);
+                writer.AddPropertySafe("uri", uri);
             }
             writer.AddProperty("byteLength", byteLength);
             writer.Close();

diff --git a/Runtime/Scripts/Schema/GltfCamera.cs b/Runtime/Scripts/Schema/GltfCamera.cs
@@ -108,7 +108,7 @@ internal void GltfSerialize(JsonWriter writer)
         {
             writer.AddObject();
             GltfSerializeName(writer);
-            writer.AddProperty("type", m_TypeEnum.ToString().ToLower());
+            writer.AddProperty("type", m_TypeEnum.ToString().ToLowerInvariant());
             if (Perspective != null)
             {
                 writer.AddProperty("perspective");

diff --git a/Runtime/Scripts/Schema/Image.cs b/Runtime/Scripts/Schema/Image.cs
@@ -34,7 +34,7 @@ internal void GltfSerialize(JsonWriter writer)
             GltfSerializeName(writer);
             if (!string.IsNullOrEmpty(uri))
             {
-                writer.AddProperty("uri", uri);
+                writer.AddPropertySafe("uri", uri);
             }
             if (!string.IsNullOrEmpty(mimeType))
             {

diff --git a/Runtime/Scripts/Schema/JsonWriter.cs b/Runtime/Scripts/Schema/JsonWriter.cs
@@ -2,9 +2,11 @@
 // SPDX-License-Identifier: Apache-2.0
 
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Globalization;
 using System.IO;
 using UnityEngine;
+using UnityEngine.Assertions;
 
 namespace GLTFast.Schema
 {
@@ -30,6 +32,7 @@ public void OpenBrackets()
 
         public void AddProperty(string name)
         {
+            CertifyValidJsonString(name);
             Separate();
             m_Stream.Write('"');
             m_Stream.Write(name);
@@ -46,6 +49,7 @@ public void AddObject()
 
         public void AddArray(string name)
         {
+            CertifyValidJsonString(name);
             Separate();
             m_Stream.Write('"');
             m_Stream.Write(name);
@@ -86,6 +90,7 @@ public void AddArrayProperty(string name, IEnumerable<string> values)
             AddArray(name);
             foreach (var value in values)
             {
+                CertifyValidJsonString(value);
                 Separate();
                 m_Stream.Write('"');
                 m_Stream.Write(value);
@@ -94,8 +99,22 @@ public void AddArrayProperty(string name, IEnumerable<string> values)
             CloseArray();
         }
 
+        public void AddArrayPropertySafe(string name, IEnumerable<string> values)
+        {
+            AddArray(name);
+            foreach (var value in values)
+            {
+                Separate();
+                m_Stream.Write('"');
+                WriteStringValueSafe(value);
+                m_Stream.Write('"');
+            }
+            CloseArray();
+        }
+
         public void AddProperty<T>(string name, T value)
         {
+            CertifyValidJsonString(name);
             Separate();
             m_Stream.Write('"');
             m_Stream.Write(name);
@@ -105,6 +124,7 @@ public void AddProperty<T>(string name, T value)
 
         public void AddProperty(string name, float value)
         {
+            CertifyValidJsonString(name);
             Separate();
             m_Stream.Write('"');
             m_Stream.Write(name);
@@ -114,6 +134,8 @@ public void AddProperty(string name, float value)
 
         public void AddProperty(string name, string value)
         {
+            CertifyValidJsonString(name);
+            CertifyValidJsonString(value);
             Separate();
             m_Stream.Write('"');
             m_Stream.Write(name);
@@ -122,8 +144,20 @@ public void AddProperty(string name, string value)
             m_Stream.Write('"');
         }
 
+        public void AddPropertySafe(string name, string value)
+        {
+            CertifyValidJsonString(name);
+            Separate();
+            m_Stream.Write('"');
+            m_Stream.Write(name);
+            m_Stream.Write("\":\"");
+            WriteStringValueSafe(value);
+            m_Stream.Write('"');
+        }
+
         public void AddProperty(string name, bool value)
         {
+            CertifyValidJsonString(name);
             Separate();
             m_Stream.Write('"');
             m_Stream.Write(name);
@@ -145,5 +179,54 @@ public void Close()
             m_Stream.Write('}');
             m_Separation = true;
         }
+
+        void WriteStringValueSafe(string value)
+        {
+            foreach (var c in value)
+            {
+                switch (c)
+                {
+                    case '\\':
+                        m_Stream.Write(@"\\");
+                        break;
+                    case '\f':
+                        m_Stream.Write("\\f");
+                        break;
+                    case '\n':
+                        m_Stream.Write("\\n");
+                        break;
+                    case '\r':
+                        m_Stream.Write("\\r");
+                        break;
+                    case '\t':
+                        m_Stream.Write("\\t");
+                        break;
+                    case '"':
+                        m_Stream.Write("\\\"");
+                        break;
+                    default:
+                        m_Stream.Write(c);
+                        break;
+                }
+            }
+        }
+
+        [Conditional("DEBUG")]
+        static void CertifyValidJsonString(string value)
+        {
+#if DEBUG
+            var invalidChars = new[]
+            {
+                '\\',
+                '\f',
+                '\n',
+                '\r',
+                '\t',
+                '"'
+            };
+
+            Assert.IsTrue(value.IndexOfAny(invalidChars) < 0, "JSON string literal contains invalid characters");
+#endif
+        }
     }
 }
diff --git a/Runtime/Scripts/Schema/Material.cs b/Runtime/Scripts/Schema/Material.cs
@@ -285,7 +285,7 @@ internal void GltfSerialize(JsonWriter writer)
 #pragma warning restore CS0618 // Type or member is obsolete
             if (m_AlphaModeEnum.HasValue && m_AlphaModeEnum.Value != AlphaMode.Opaque)
             {
-                writer.AddProperty("alphaMode", m_AlphaModeEnum.Value.ToString().ToUpper());
+                writer.AddProperty("alphaMode", m_AlphaModeEnum.Value.ToString().ToUpperInvariant());
             }
             if (math.abs(alphaCutoff - .5f) > Constants.epsilon)
             {

diff --git a/Runtime/Scripts/Schema/Mesh.cs b/Runtime/Scripts/Schema/Mesh.cs
@@ -115,7 +115,7 @@ internal void GltfSerialize(JsonWriter writer)
         {
             if (targetNames != null)
             {
-                writer.AddArrayProperty("targetNames", targetNames);
+                writer.AddArrayPropertySafe("targetNames", targetNames);
             }
         }
     }

diff --git a/Runtime/Scripts/Schema/NamedObject.cs b/Runtime/Scripts/Schema/NamedObject.cs
@@ -20,7 +20,7 @@ internal void GltfSerializeName(JsonWriter writer)
         {
             if (!string.IsNullOrEmpty(name))
             {
-                writer.AddProperty("name", name);
+                writer.AddPropertySafe("name", name);
             }
         }
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -115,7 +115,7 @@ internal void GltfSerialize(JsonWriter writer) @@
             {
                 if (targetNames != null)
                 {
-                    writer.AddArrayProperty("targetNames", targetNames);
+                    writer.AddArrayPropertySafe("targetNames", targetNames);
                 }
             }
         }
@@ Expand Down @@