diff --git a/src/MeshGroup.cpp b/src/MeshGroup.cpp index ce87b52be9..2c8a3ce2b9 100644 --- a/src/MeshGroup.cpp +++ b/src/MeshGroup.cpp @@ -3,6 +3,8 @@ #include "MeshGroup.h" +#include +#include #include #include #include @@ -179,30 +181,53 @@ bool loadMeshSTL_ascii(Mesh* mesh, const char* filename, const Matrix4x3D& matri bool loadMeshSTL_binary(Mesh* mesh, const char* filename, const Matrix4x3D& matrix, const std::vector* uv_coordinates = nullptr) { FILE* f = fopen(filename, "rb"); + if (f == nullptr) + { + return false; + } fseek(f, 0L, SEEK_END); - long long file_size = ftell(f); // The file size is the position of the cursor after seeking to the end. + const long long file_size = ftell(f); // Cursor position at EOF == file size, or -1 on error. rewind(f); // Seek back to start. - size_t face_count = (file_size - 80 - sizeof(uint32_t)) / 50; // Subtract the size of the header. Every face uses exactly 50 bytes. - char buffer[80]; + // A binary STL is an 80-byte header, a uint32 triangle count, then exactly 50 bytes per triangle. + // Reject files too small to even hold the header (or for which ftell() returned -1) before the size + // arithmetic, so the unsigned face-count computation below cannot underflow into a huge value and + // drive a wild reserve()/read loop. + constexpr long long header_size = 80 + static_cast(sizeof(uint32_t)); + constexpr size_t bytes_per_face = 50; // Normal (3 floats) + 3 vertices (9 floats) + 2-byte attribute. + if (file_size < header_size) + { + fclose(f); + return false; + } + const size_t face_count = static_cast(file_size - header_size) / bytes_per_face; + + std::array header; // Skip the header - if (fread(buffer, 80, 1, f) != 1) + if (fread(header.data(), header.size(), 1, f) != 1) { fclose(f); return false; } - uint32_t reported_face_count; - // Read the face count. We'll use it as a sort of redundancy code to check for file corruption. - if (fread(&reported_face_count, sizeof(uint32_t), 1, f) != 1) + uint32_t reported_face_count = 0; + // Read the header's triangle count and cross-check it against the size-derived count to surface + // corrupt or malformed files. We keep parsing the size-derived count (some exporters append trailing + // data), so a mismatch only warns rather than rejecting the file. + if (fread(&reported_face_count, sizeof(reported_face_count), 1, f) != 1) { fclose(f); return false; } if (reported_face_count != face_count) { - spdlog::warn("Face count reported by file ({}) is not equal to actual face count ({}). File could be corrupt!", reported_face_count, face_count); + const uint64_t expected_size = static_cast(header_size) + static_cast(reported_face_count) * bytes_per_face; + spdlog::warn( + "STL triangle count in header ({}) is inconsistent with the file size (header implies {} bytes, file is {} bytes). File could be corrupt!", + reported_face_count, + expected_size, + file_size); } // For each face read: @@ -214,16 +239,22 @@ bool loadMeshSTL_binary(Mesh* mesh, const char* filename, const Matrix4x3D& matr size_t vertex_index = 0; for (size_t i = 0; i < face_count; i++) { - if (fread(buffer, 50, 1, f) != 1) + std::array face_record; + if (fread(face_record.data(), face_record.size(), 1, f) != 1) { fclose(f); return false; } - float* v = reinterpret_cast(buffer) + 3; - - Point3LL v0 = matrix.apply(Point3F(v[0], v[1], v[2]).toPoint3d()); - Point3LL v1 = matrix.apply(Point3F(v[3], v[4], v[5]).toPoint3d()); - Point3LL v2 = matrix.apply(Point3F(v[6], v[7], v[8]).toPoint3d()); + // Decode the 12 little-endian floats with memcpy instead of aliasing a float* over the byte + // buffer, removing pointer-alignment and strict-aliasing assumptions while preserving the + // existing layout: floats 3..11 (bytes 12..47) are the three vertices; floats 0..2 are the + // discarded normal. + std::array floats; + std::memcpy(floats.data(), face_record.data(), sizeof(floats)); + + Point3LL v0 = matrix.apply(Point3F(floats[3], floats[4], floats[5]).toPoint3d()); + Point3LL v1 = matrix.apply(Point3F(floats[6], floats[7], floats[8]).toPoint3d()); + Point3LL v2 = matrix.apply(Point3F(floats[9], floats[10], floats[11]).toPoint3d()); // Handle UV coordinates if provided if (uv_coordinates && vertex_index + 2 < uv_coordinates->size()) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6c61b9a3ba..1a924fa527 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -13,6 +13,7 @@ set(TESTS_SRC_BASE GCodeTemplateResolverTest InfillTest LayerPlanTest + MeshGroupTest PathOrderOptimizerTest PathOrderMonotonicTest TimeEstimateCalculatorTest diff --git a/tests/MeshGroupTest.cpp b/tests/MeshGroupTest.cpp new file mode 100644 index 0000000000..86ddd05d08 --- /dev/null +++ b/tests/MeshGroupTest.cpp @@ -0,0 +1,115 @@ +// Copyright (c) 2024 UltiMaker +// CuraEngine is released under the terms of the AGPLv3 or higher + +#include "MeshGroup.h" // The unit under test: loadMeshIntoMeshGroup -> binary STL loader. + +#include +#include +#include +#include +#include +#include + +#include + +#include "Application.h" // To set up a slice so that settings can be requested. +#include "Slice.h" +#include "settings/Settings.h" +#include "utils/Matrix4x3D.h" + +// NOLINTBEGIN(*-magic-numbers) +namespace cura +{ + +class MeshGroupTest : public testing::Test +{ +public: + void SetUp() override + { + Application::getInstance().startThreadPool(); + Application::getInstance().current_slice_ = std::make_shared(1); + } + + static void appendFloat(std::vector& bytes, const float value) + { + std::array raw{}; + std::memcpy(raw.data(), &value, sizeof(float)); + bytes.insert(bytes.end(), raw.begin(), raw.end()); + } + + //! Build a minimal binary STL: 80-byte header, \p reported_count as the uint32 triangle count, then + //! one 50-byte record per triangle (each triangle is 9 vertex floats; the normal is zeroed). + static std::vector makeBinaryStl(const uint32_t reported_count, const std::vector>& triangles) + { + std::vector bytes(80, 0x00); + std::array count_raw{}; + std::memcpy(count_raw.data(), &reported_count, sizeof(uint32_t)); + bytes.insert(bytes.end(), count_raw.begin(), count_raw.end()); + for (const std::array& triangle : triangles) + { + for (int normal = 0; normal < 3; ++normal) + { + appendFloat(bytes, 0.0F); + } + for (const float coord : triangle) + { + appendFloat(bytes, coord); + } + bytes.push_back(0); // 2-byte attribute + bytes.push_back(0); + } + return bytes; + } + + static std::string writeTempFile(const std::string& name, const std::vector& bytes) + { + const std::filesystem::path path = std::filesystem::temp_directory_path() / name; + std::ofstream out(path, std::ios::binary | std::ios::trunc); + out.write(reinterpret_cast(bytes.data()), static_cast(bytes.size())); + out.close(); + return path.string(); + } +}; + +// Regression: a binary STL smaller than the 84-byte header must be rejected instead of underflowing the +// triangle-count arithmetic into a gigantic reserve() (which previously aborted the process). +TEST_F(MeshGroupTest, BinaryStlSmallerThanHeaderIsRejected) +{ + Settings& settings = Application::getInstance().current_slice_->scene.settings; + MeshGroup mesh_group; + // 50 bytes, not starting with "solid", so it is routed to the binary loader. + const std::string path = writeTempFile("curaengine_short.stl", std::vector(50, 0x01)); + EXPECT_FALSE(loadMeshIntoMeshGroup(&mesh_group, path.c_str(), Matrix4x3D(), settings)); + std::filesystem::remove(path); +} + +// Regression: a header triangle count that disagrees with the file size must not change what is parsed. +// The loader trusts the size-derived count and reads no out-of-bounds data. +TEST_F(MeshGroupTest, BinaryStlWithInconsistentHeaderCountStillLoads) +{ + Settings& settings = Application::getInstance().current_slice_->scene.settings; + MeshGroup mesh_group; + const std::vector> triangles = { + { 0, 0, 0, 10, 0, 0, 0, 10, 0 }, // well-separated, non-degenerate + { 0, 0, 20, 10, 0, 20, 0, 10, 20 }, + }; + const std::string path = writeTempFile("curaengine_badcount.stl", makeBinaryStl(9999, triangles)); // header lies: 9999 + ASSERT_TRUE(loadMeshIntoMeshGroup(&mesh_group, path.c_str(), Matrix4x3D(), settings)); + ASSERT_EQ(mesh_group.meshes.size(), size_t(1)); + EXPECT_EQ(mesh_group.meshes.back().faces_.size(), size_t(2)); // size-derived count, not the bogus header count + std::filesystem::remove(path); +} + +// Behaviour preservation: the memcpy-based decoder still loads the real binary STL fixture. +TEST_F(MeshGroupTest, BinaryStlFixtureLoads) +{ + Settings& settings = Application::getInstance().current_slice_->scene.settings; + MeshGroup mesh_group; + const std::string path = std::filesystem::path(__FILE__).parent_path().append("testModel.stl").string(); + ASSERT_TRUE(loadMeshIntoMeshGroup(&mesh_group, path.c_str(), Matrix4x3D(), settings)); + ASSERT_EQ(mesh_group.meshes.size(), size_t(1)); + EXPECT_GT(mesh_group.meshes.back().faces_.size(), size_t(0)); +} + +} // namespace cura +// NOLINTEND(*-magic-numbers)