Check BOM of USDA files and report errors if found

Author: roggiezhang-nv

pxr/usd/sdf/testenv/testSdfParsing.py

@@ -47,6 +47,9 @@ def test_Basic(self):
         # This will mean that your new test runs first and you can spot
         # failures much quicker.
         testFiles = '''
+        230_bad_layer_with_utf32_BOM.usda
+        229_bad_layer_with_utf16_BOM.usda
+        228_bad_layer_with_utf8_BOM.usda
         227_arrayEdits.usda
         226_version_1.1.usda
         225_multiline_with_SplineKnotParamList.usda

pxr/usd/sdf/testenv/testSdfParsing.testenv/228_bad_layer_with_utf8_BOM.usda

@@ -0,0 +1,5 @@
+#usda 1.1
+
+def "Prim1"
+{
+}

pxr/usd/sdf/testenv/testSdfParsing.testenv/229_bad_layer_with_utf16_BOM.usda

@@ -177,25 +177,76 @@ SdfUsdaFileFormat::~SdfUsdaFileFormat()
 namespace
 {
 
+bool _CheckBOM(const char* bufferRead, size_t bufferSize) {
+    // Check for UTF-8 BOM (EF BB BF)
+    if (bufferSize >= 3 && 
+        static_cast<unsigned char>(bufferRead[0]) == 0xEF &&
+        static_cast<unsigned char>(bufferRead[1]) == 0xBB &&
+        static_cast<unsigned char>(bufferRead[2]) == 0xBF) {
+        TF_WARN("Asset starts with UTF-8 BOM which is not supported, "
+                "please convert the file to UTF-8 without the BOM.");
+        return false;
+    }
+    
+    // Check for UTF-16 BOM markers (FE FF or FF FE)
+    if (bufferSize >= 2 &&
+        ((static_cast<unsigned char>(bufferRead[0]) == 0xFE &&
+         static_cast<unsigned char>(bufferRead[1]) == 0xFF) ||
+        (static_cast<unsigned char>(bufferRead[0]) == 0xFF &&
+         static_cast<unsigned char>(bufferRead[1]) == 0xFE))) {
+        TF_WARN("Asset starts with UTF-16 BOM marker which is not supported, "
+                "please convert the file to UTF-8 without the BOM.");
+        return false;
+    }
+
+    // Check for UTF-32 BOM markers (00 00 FE FF or FF FE 00 00)
+    if (bufferSize >= 4 &&
+        ((static_cast<unsigned char>(bufferRead[0]) == 0x00 &&
+         static_cast<unsigned char>(bufferRead[1]) == 0x00 &&
+         static_cast<unsigned char>(bufferRead[2]) == 0xFE &&
+         static_cast<unsigned char>(bufferRead[3]) == 0xFF) ||
+        (static_cast<unsigned char>(bufferRead[0]) == 0xFF &&
+         static_cast<unsigned char>(bufferRead[1]) == 0xFE &&
+         static_cast<unsigned char>(bufferRead[2]) == 0x00 &&
+         static_cast<unsigned char>(bufferRead[3]) == 0x00))) {
+        TF_WARN("Asset starts with UTF-32 BOM marker which is not supported, "
+                "please convert the file to UTF-8 without the BOM.");
+        return false;
+    }
+
+    return true;
+}
+
 bool
 _CanReadImpl(const std::shared_ptr<ArAsset>& asset,
-             const std::string& cookie)
+             const std::string& cookie,
+             bool bomCheckWarning = true)
 {
     TfErrorMark mark;
 
+    constexpr size_t BOM_CHECK_SIZE = 4;
     constexpr size_t COOKIE_BUFFER_SIZE = 512;
     char local[COOKIE_BUFFER_SIZE];
     std::unique_ptr<char []> remote;
     char *buf = local;
     size_t cookieLength = cookie.length();
-    if (cookieLength > COOKIE_BUFFER_SIZE - 1) {
-        remote.reset(new char[cookieLength + 1]);
+    if (BOM_CHECK_SIZE + cookieLength > COOKIE_BUFFER_SIZE - 1) {
+        remote = std::make_unique<char[]>(cookieLength + BOM_CHECK_SIZE + 1);
         buf = remote.get();
     }
-    if (asset->Read(buf, cookieLength, /* offset = */ 0) != cookieLength) {
+    // Maximum 4 bytes are needed to check for BOM markers.
+    size_t bytesRead = asset->Read(buf, BOM_CHECK_SIZE + cookieLength, /* offset = */ 0);
+    // At least the cookie length is needed to check for the cookie.
+    if (bytesRead < cookieLength) {
+        return false;
+    }
+
+    // Check bom markers if requested
+    if (bomCheckWarning && !_CheckBOM(buf, bytesRead)) {
         return false;
     }
 
+    // It doesn't have BOM markers, so we can check the cookie.
     buf[cookieLength] = '\0';
 
     // Don't allow errors to escape this function, since this function is
@@ -224,15 +275,15 @@ SdfUsdaFileFormat::CanRead(const string& filePath) const
 
     std::shared_ptr<ArAsset> asset = ArGetResolver().OpenAsset(
         ArResolvedPath(filePath));
-    return asset && _CanReadImpl(asset, GetFileCookie());
+    return asset && _CanReadImpl(asset, GetFileCookie(), false);
 }
 
 bool
 SdfUsdaFileFormat::_CanReadFromAsset(
     const std::string& resolvedPath,
     const std::shared_ptr<ArAsset>& asset) const
 {
-    return _CanReadImpl(asset, GetFileCookie());
+    return _CanReadImpl(asset, GetFileCookie(), false);
 }
 
 bool
@@ -261,7 +312,7 @@ SdfUsdaFileFormat::_ReadFromAsset(
 {
     // Quick check to see if the file has the magic cookie before spinning up
     // the parser.
-    if (!_CanReadImpl(asset, GetFileCookie())) {
+    if (!_CanReadImpl(asset, GetFileCookie(), true)) {
         TF_RUNTIME_ERROR("<%s> is not a valid %s layer",
                          resolvedPath.c_str(),
                          GetFormatId().GetText());

pxr/usd/sdf/testenv/testSdfParsing.testenv/230_bad_layer_with_utf32_BOM.usda

@@ -18,6 +18,13 @@ character and string. Users should think of UTF-8 strings as bytes representing
 "code points" in the Unicode code charts. A single code point may be represented 
 by 1, 2, 3, or 4 byte sequences.
 
+## Byte Order Mark (BOM)
+
+The USD parser does not support Byte Order Marks (BOMs) at the beginning of USDA 
+files. Files containing a BOM will be treated as invalid layers. To fix this, 
+open your USDA file in a text editor and ensure it's saved without a BOM (most 
+modern text editors default to UTF-8 without BOM).
+
 ### Replacement Code Point {#Usd_UTF_8_Encoding_Replacement}
 
 Not every 1, 2, 3, or 4 byte sequence represents a valid UTF-8 code point. When