From 4fea26f6c8652f545560807bccc934cf0cdd86dd Mon Sep 17 00:00:00 2001 From: Syoyo Fujita Date: Wed, 24 Jan 2024 05:43:27 +0900 Subject: [PATCH] Allow zero-sized BIN chunk. Fixes #440 --- .../zero-sized-bin-chunk-issue-440.glb | Bin 0 -> 736 bytes tests/tester.cc | 24 +++++ tiny_gltf.h | 84 +++++++++++------- 3 files changed, 75 insertions(+), 33 deletions(-) create mode 100644 models/regression/zero-sized-bin-chunk-issue-440.glb diff --git a/models/regression/zero-sized-bin-chunk-issue-440.glb b/models/regression/zero-sized-bin-chunk-issue-440.glb new file mode 100644 index 0000000000000000000000000000000000000000..9ab140cf57ae96cb1de26e5807bbf3dc0afc3090 GIT binary patch literal 736 zcmZuv+e*Vg5UqYmndeY)Ypr<^Efy-;iY>lKA2!J(T}XDr?nW)8;Ai-Geu}e6#Flnp zm${xfGdp>)nqKyLy&rnM={_tM^Bu4xL9M0I5WVdnuCol)E5{FrTn7?aR#Koat8Il4 z4Lv`w##$JNNGjPUW%+B8xFE?T$uyC$oZLR%zQEo&#`Ml%j!rSXf$P}o4lHJe-s+vL ztMht431Ck#Olu@oJIGO>Vn!;6z;Ln63zLBZ@2E7F#1j00OS}$RJ#4*cJKh?6M3I{f zL`2Rom0Ro}ydDMr9i4;*6l+r#S=MkFAE)}3W;@`n;z5}(wBC{#RGby9Kb3V{;F719 zLS$5p4pgyvLc@xJCoi(jH-i4oHXSpK6In<#Wl7WeTRXk+b#typ-f--NzDL75F;V1t z-&9Av!M1dzc*zZa?^I@t;&E-Px`XG%a<-Z+<`8)_M=nxME7VNmkAJJkly-*tve<%@ o7P_P=JZoB#j- literal 0 HcmV?d00001 diff --git a/tests/tester.cc b/tests/tester.cc index 409df1c6..168f2496 100644 --- a/tests/tester.cc +++ b/tests/tester.cc @@ -902,3 +902,27 @@ TEST_CASE("serialize-empty-scene", "[issue-464]") { CHECK(m.scenes[0] == scene); } } + +TEST_CASE("zero-sized-bin-chunk-glb", "[issue-440]") { + + tinygltf::Model model; + tinygltf::TinyGLTF ctx; + std::string err; + std::string warn; + + // Input glb has zero-sized data in bin chunk(8 bytes for BIN chunk, and chunksize == 0) + // The spec https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#binary-buffer says + // + // When the binary buffer is empty or when it is stored by other means, this chunk SHOULD be omitted. + // + // 'SHOULD' mean 'RECOMMENDED', so we'll need to allow such zero-sized bin chunk is NOT omitted. + bool ret = ctx.LoadBinaryFromFile(&model, &err, &warn, "../models/regression/zero-sized-bin-chunk-issue-440.glb"); + if (!warn.empty()) { + std::cout << "WARN: " << warn << "\n"; + } + if (!err.empty()) { + std::cerr << err << std::endl; + } + + REQUIRE(true == ret); +} diff --git a/tiny_gltf.h b/tiny_gltf.h index d470bf47..7c7227cc 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h @@ -6652,7 +6652,7 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err, memcpy(&version, bytes + 4, 4); swap4(&version); - memcpy(&length, bytes + 8, 4); + memcpy(&length, bytes + 8, 4); // Total glb size, including header and all chunks. swap4(&length); memcpy(&chunk0_length, bytes + 12, 4); // JSON data length swap4(&chunk0_length); @@ -6708,68 +6708,86 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err, bin_size_ = 0; } else { // Read Chunk1 info(BIN data) - // At least Chunk1 should have 12 bytes(8 bytes(header) + 4 bytes(bin - // payload could be 1~3 bytes, but need to be aligned to 4 bytes) - if ((header_and_json_size + 12ull) > uint64_t(length)) { + // + // issue-440: + // 'SHOULD' in glTF spec means 'RECOMMENDED', + // So there is a situation that Chunk1(BIN) is composed of zero-sized BIN data + // (chunksize(0) + binformat(BIN) = 8bytes). + // + if ((header_and_json_size + 8ull) > uint64_t(length)) { if (err) { (*err) = "Insufficient storage space for Chunk1(BIN data). At least Chunk1 " - "Must have 4 or more bytes, but got " + + "Must have 8 or more bytes, but got " + std::to_string((header_and_json_size + 8ull) - uint64_t(length)) + ".\n"; } return false; } - unsigned int chunk1_length; // 4 bytes - unsigned int chunk1_format; // 4 bytes; + unsigned int chunk1_length{0}; // 4 bytes + unsigned int chunk1_format{0}; // 4 bytes; memcpy(&chunk1_length, bytes + header_and_json_size, - 4); // JSON data length + 4); // Bin data length swap4(&chunk1_length); memcpy(&chunk1_format, bytes + header_and_json_size + 4, 4); swap4(&chunk1_format); - // std::cout << "chunk1_length = " << chunk1_length << "\n"; - - if (chunk1_length < 4) { + if (chunk1_format != 0x004e4942) { if (err) { - (*err) = "Insufficient Chunk1(BIN) data size."; + (*err) = "Invalid chunkType for Chunk1."; } return false; } - if ((chunk1_length % 4) != 0) { - if (strictness_==ParseStrictness::Permissive) { - if (warn) { - (*warn) += "BIN Chunk end is not aligned to a 4-byte boundary.\n"; + if (chunk1_length == 0) { + + if (header_and_json_size + 8 > uint64_t(length)) { + if (err) { + (*err) = "BIN Chunk header location exceeds the GLB size."; } + return false; } - else { + + bin_data_ = nullptr; + + } else { + + // When BIN chunk size is not zero, at least Chunk1 should have 12 bytes(8 bytes(header) + 4 bytes(bin + // payload could be 1~3 bytes, but need to be aligned to 4 bytes) + + if (chunk1_length < 4) { if (err) { - (*err) = "BIN Chunk end is not aligned to a 4-byte boundary."; + (*err) = "Insufficient Chunk1(BIN) data size."; } return false; } - } - if (uint64_t(chunk1_length) + header_and_json_size > uint64_t(length)) { - if (err) { - (*err) = "BIN Chunk data length exceeds the GLB size."; + if ((chunk1_length % 4) != 0) { + if (strictness_==ParseStrictness::Permissive) { + if (warn) { + (*warn) += "BIN Chunk end is not aligned to a 4-byte boundary.\n"; + } + } + else { + if (err) { + (*err) = "BIN Chunk end is not aligned to a 4-byte boundary."; + } + return false; + } } - return false; - } - if (chunk1_format != 0x004e4942) { - if (err) { - (*err) = "Invalid type for chunk1 data."; + // +8 chunk1 header size. + if (uint64_t(chunk1_length) + header_and_json_size + 8 > uint64_t(length)) { + if (err) { + (*err) = "BIN Chunk data length exceeds the GLB size."; + } + return false; } - return false; - } - - // std::cout << "chunk1_length = " << chunk1_length << "\n"; - bin_data_ = bytes + header_and_json_size + - 8; // 4 bytes (bin_buffer_length) + 4 bytes(bin_buffer_format) + bin_data_ = bytes + header_and_json_size + + 8; // 4 bytes (bin_buffer_length) + 4 bytes(bin_buffer_format) + } bin_size_ = size_t(chunk1_length); }