diff --git a/docs/using_cloud_storage.md b/docs/using_cloud_storage.md index 6aee4c95b4..30aa922c99 100644 --- a/docs/using_cloud_storage.md +++ b/docs/using_cloud_storage.md @@ -56,6 +56,8 @@ openvino/model_server:latest \ Add the S3 path as the model_path and pass the credentials as environment variables to the Docker container. `S3_ENDPOINT` is optional for Amazon S3 storage and mandatory for MinIO and other S3-compatible storage types. +When `S3_ENDPOINT` is set without a scheme, OVMS uses `http://` by default for compatibility with MinIO and other S3-compatible deployments. To force TLS, provide the endpoint as `https://host:port` explicitly. + Example command with `s3:///:` ```bash diff --git a/src/custom_nodes/common/custom_node_library_internal_manager.hpp b/src/custom_nodes/common/custom_node_library_internal_manager.hpp index dc1b89b0d2..5185fb7c00 100644 --- a/src/custom_nodes/common/custom_node_library_internal_manager.hpp +++ b/src/custom_nodes/common/custom_node_library_internal_manager.hpp @@ -44,11 +44,12 @@ class CustomNodeLibraryInternalManager { template bool get_buffer(ovms::custom_nodes_common::CustomNodeLibraryInternalManager* internalManager, T** buffer, const char* buffersQueueName, uint64_t byte_size) { + *buffer = nullptr; auto buffersQueue = internalManager->getBuffersQueue(buffersQueueName); - if (!(buffersQueue == nullptr)) { + if (buffersQueue != nullptr && byte_size <= buffersQueue->getSingleBufferSize()) { *buffer = static_cast(buffersQueue->getBuffer()); } - if (*buffer == nullptr || buffersQueue == nullptr) { + if (*buffer == nullptr) { *buffer = (T*)malloc(byte_size); if (*buffer == nullptr) { return false; diff --git a/src/custom_nodes/model_zoo_intel_object_detection/model_zoo_intel_object_detection.cpp b/src/custom_nodes/model_zoo_intel_object_detection/model_zoo_intel_object_detection.cpp index 231be597f1..2bd3ef2c77 100644 --- a/src/custom_nodes/model_zoo_intel_object_detection/model_zoo_intel_object_detection.cpp +++ b/src/custom_nodes/model_zoo_intel_object_detection/model_zoo_intel_object_detection.cpp @@ -355,7 +355,9 @@ DLL_PUBLIC int execute(const struct CustomNodeTensor* inputs, int inputsCount, s NODE_ASSERT(boxes.size() == confidences.size(), "boxes and confidences are not equal length"); if (boxes.size() > maxOutputBatch) { boxes.resize(maxOutputBatch); + detections.resize(maxOutputBatch); confidences.resize(maxOutputBatch); + labelIds.resize(maxOutputBatch); } CustomNodeLibraryInternalManager* internalManager = static_cast(customNodeLibraryInternalManager); diff --git a/src/filesystem/s3filesystem.cpp b/src/filesystem/s3filesystem.cpp index c86ffe9b75..d2044b5eef 100644 --- a/src/filesystem/s3filesystem.cpp +++ b/src/filesystem/s3filesystem.cpp @@ -48,6 +48,21 @@ namespace ovms { namespace s3 = Aws::S3; namespace fs = std::filesystem; +std::pair S3FileSystem::parseEndpoint(const std::string& endpoint) { + std::string normalized = endpoint; + auto scheme = Aws::Http::Scheme::HTTP; + + if (normalized.rfind("http://", 0) == 0) { + normalized = normalized.substr(7); + scheme = Aws::Http::Scheme::HTTP; + } else if (normalized.rfind("https://", 0) == 0) { + normalized = normalized.substr(8); + scheme = Aws::Http::Scheme::HTTPS; + } + + return {normalized, scheme}; +} + StatusCode S3FileSystem::parsePath(const std::string& path, std::string* bucket, std::string* object) { std::smatch sm; @@ -127,12 +142,9 @@ S3FileSystem::S3FileSystem(const Aws::SDKOptions& options, const std::string& s3 config.scheme = Aws::Http::Scheme::HTTP; } if (s3_endpoint != nullptr) { - std::string endpoint(s3_endpoint); - if (endpoint.rfind("http://") != std::string::npos) { - endpoint = endpoint.substr(7); - } - config.endpointOverride = Aws::String(endpoint.c_str()); - config.scheme = Aws::Http::Scheme::HTTP; + auto parsed = S3FileSystem::parseEndpoint(s3_endpoint); + config.scheme = parsed.second; + config.endpointOverride = Aws::String(parsed.first.c_str()); } if (!default_proxy.empty()) { diff --git a/src/filesystem/s3filesystem.hpp b/src/filesystem/s3filesystem.hpp index c1dbd3e940..1326978368 100644 --- a/src/filesystem/s3filesystem.hpp +++ b/src/filesystem/s3filesystem.hpp @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -29,6 +30,14 @@ namespace ovms { class S3FileSystem : public FileSystem { public: + /** + * @brief Resolve an S3-compatible endpoint into a clean host:port string and scheme. + * + * If no scheme is provided, HTTP is used by default for compatibility with MinIO + * and other S3-compatible deployments that expose plain HTTP endpoints. + */ + static std::pair parseEndpoint(const std::string& endpoint); + /** * @brief Construct a new S3FileSystem object * diff --git a/src/kfs_frontend/kfs_utils.cpp b/src/kfs_frontend/kfs_utils.cpp index 15bc97d1ab..23cd61af6b 100644 --- a/src/kfs_frontend/kfs_utils.cpp +++ b/src/kfs_frontend/kfs_utils.cpp @@ -16,6 +16,7 @@ #include "kfs_utils.hpp" #include +#include #include #include #include @@ -323,11 +324,28 @@ Status buildShapeFromStringTensorRequest(const ::KFSRequest::InferInputTensor& s Status convertBinaryExtensionStringFromBufferToNativeOVTensor(const ::KFSRequest::InferInputTensor& src, ov::Tensor& tensor, const std::string* buffer) { std::vector stringSizes; - uint32_t totalStringsLength = 0; - while (totalStringsLength + stringSizes.size() * sizeof(uint32_t) + sizeof(uint32_t) <= buffer->size()) { - uint32_t inputSize = *(reinterpret_cast(buffer->data() + totalStringsLength + stringSizes.size() * sizeof(uint32_t))); + size_t totalStringsLength = 0; + while (true) { + const size_t headersLength = stringSizes.size() * sizeof(uint32_t); + if (headersLength > buffer->size()) { + break; + } + if (totalStringsLength > buffer->size() - headersLength) { + break; + } + const size_t currentOffset = totalStringsLength + headersLength; + if (buffer->size() - currentOffset < sizeof(uint32_t)) { + break; + } + uint32_t inputSize = 0; + std::memcpy(&inputSize, buffer->data() + currentOffset, sizeof(inputSize)); + const size_t remainingStringBytes = buffer->size() - currentOffset - sizeof(uint32_t); + if (static_cast(inputSize) > remainingStringBytes) { + SPDLOG_DEBUG("Input string format conversion failed"); + return StatusCode::INVALID_STRING_INPUT; + } stringSizes.push_back(inputSize); - totalStringsLength += inputSize; + totalStringsLength += static_cast(inputSize); } size_t batchSize = stringSizes.size(); if ((totalStringsLength + batchSize * sizeof(uint32_t)) != buffer->size()) { @@ -343,7 +361,12 @@ Status convertBinaryExtensionStringFromBufferToNativeOVTensor(const ::KFSRequest std::string* data = tensor.data(); size_t tensorStringsOffset = 0; for (size_t i = 0; i < stringSizes.size(); i++) { - data[i].assign(reinterpret_cast(buffer->data() + (i + 1) * sizeof(uint32_t) + tensorStringsOffset), stringSizes[i]); + const size_t sourceOffset = (i + 1) * sizeof(uint32_t) + tensorStringsOffset; + if (sourceOffset > buffer->size() || static_cast(stringSizes[i]) > buffer->size() - sourceOffset) { + SPDLOG_DEBUG("Input string format conversion failed"); + return StatusCode::INVALID_STRING_INPUT; + } + data[i].assign(reinterpret_cast(buffer->data() + sourceOffset), stringSizes[i]); tensorStringsOffset += stringSizes[i]; } return StatusCode::OK; diff --git a/src/tensor_conversion.cpp b/src/tensor_conversion.cpp index b996070997..fdbf2a1705 100644 --- a/src/tensor_conversion.cpp +++ b/src/tensor_conversion.cpp @@ -72,13 +72,24 @@ shape_t getShapeFromImages(const std::vector& images, const TensorInfo& ov::Tensor createTensorFromMats(const std::vector& images, const TensorInfo& tensorInfo) { OVMS_PROFILE_FUNCTION(); + if (images.empty()) { + return ov::Tensor(); + } ov::Shape shape = getShapeFromImages(images, tensorInfo); ov::element::Type precision = tensorInfo.getOvPrecision(); ov::Tensor tensor(precision, shape); char* ptr = (char*)tensor.data(); - for (cv::Mat image : images) { - memcpy(ptr, (char*)image.data, image.total() * image.elemSize()); - ptr += (image.total() * image.elemSize()); + const size_t firstImageSizeBytes = images[0].total() * images[0].elemSize(); + for (const auto& image : images) { + const size_t imageSizeBytes = image.total() * image.elemSize(); + if (imageSizeBytes != firstImageSizeBytes) { + SPDLOG_DEBUG("Image conversion failed due to inconsistent image size in one batch. Expected bytes: {} current bytes: {}", + firstImageSizeBytes, + imageSizeBytes); + return ov::Tensor(); + } + memcpy(ptr, (char*)image.data, imageSizeBytes); + ptr += imageSizeBytes; } return tensor; } diff --git a/src/tensor_conversion.hpp b/src/tensor_conversion.hpp index 2b4dc86175..98a3740f05 100644 --- a/src/tensor_conversion.hpp +++ b/src/tensor_conversion.hpp @@ -121,6 +121,14 @@ static Status convertTensorToMatsMatchingTensorInfo(const TensorType& src, std:: image = std::move(imageResized); } + if (firstImage != nullptr && image.channels() != firstImage->channels()) { + SPDLOG_DEBUG("Binary data sent to input: {} has mixed channel count within one batch. First image channels: {} current image channels: {}", + tensorInfo.getMappedName(), + firstImage->channels(), + image.channels()); + return StatusCode::INVALID_NO_OF_CHANNELS; + } + // if (i == 0 && src.contents().bytes_contents_size() > 1) { // // Multiply src.string_val_size() * image resolution * precision size // } diff --git a/src/tensor_conversion_common.cpp b/src/tensor_conversion_common.cpp index 328705baa4..6e6a76093c 100644 --- a/src/tensor_conversion_common.cpp +++ b/src/tensor_conversion_common.cpp @@ -236,13 +236,24 @@ shape_t getShapeFromImages(const std::vector& images, const TensorInfo& } ov::Tensor createTensorFromMats(const std::vector& images, const TensorInfo& tensorInfo) { OVMS_PROFILE_FUNCTION(); + if (images.empty()) { + return ov::Tensor(); + } ov::Shape shape = getShapeFromImages(images, tensorInfo); ov::element::Type precision = tensorInfo.getOvPrecision(); ov::Tensor tensor(precision, shape); char* ptr = (char*)tensor.data(); - for (cv::Mat image : images) { - memcpy(ptr, (char*)image.data, image.total() * image.elemSize()); - ptr += (image.total() * image.elemSize()); + const size_t firstImageSizeBytes = images[0].total() * images[0].elemSize(); + for (const auto& image : images) { + const size_t imageSizeBytes = image.total() * image.elemSize(); + if (imageSizeBytes != firstImageSizeBytes) { + SPDLOG_DEBUG("Image conversion failed due to inconsistent image size in one batch. Expected bytes: {} current bytes: {}", + firstImageSizeBytes, + imageSizeBytes); + return ov::Tensor(); + } + memcpy(ptr, (char*)image.data, imageSizeBytes); + ptr += imageSizeBytes; } return tensor; } diff --git a/src/test/localfilesystem_test.cpp b/src/test/localfilesystem_test.cpp index e53e9885d8..bbe73651d1 100644 --- a/src/test/localfilesystem_test.cpp +++ b/src/test/localfilesystem_test.cpp @@ -27,6 +27,9 @@ #include "src/filesystem/filesystem.hpp" #include "src/filesystem/localfilesystem.hpp" +#if CLOUD_DISABLE == 0 +#include "src/filesystem/s3filesystem.hpp" +#endif using namespace testing; using ::testing::UnorderedElementsAre; @@ -54,6 +57,18 @@ static void createTmpFiles() { std::filesystem::create_directories(TMP_PATH / TMP_DIR2); } +#if CLOUD_DISABLE == 0 +TEST(S3FileSystem, ParseEndpointUsesHttpByDefault) { + auto http_default = ovms::S3FileSystem::parseEndpoint("localhost:9000"); + EXPECT_EQ(http_default.first, "localhost:9000"); + EXPECT_EQ(http_default.second, Aws::Http::Scheme::HTTP); + + auto https_explicit = ovms::S3FileSystem::parseEndpoint("https://localhost:9000"); + EXPECT_EQ(https_explicit.first, "localhost:9000"); + EXPECT_EQ(https_explicit.second, Aws::Http::Scheme::HTTPS); +} +#endif + TEST(LocalFileSystem, FileExists) { ovms::LocalFileSystem lfs; bool exists = false; diff --git a/src/test/node_library_manager_test.cpp b/src/test/node_library_manager_test.cpp index ef5133a88c..a140ce8618 100644 --- a/src/test/node_library_manager_test.cpp +++ b/src/test/node_library_manager_test.cpp @@ -16,6 +16,9 @@ #include #include +#include +#include + #include "../dags/custom_node_library_manager.hpp" #include "constructor_enabled_model_manager.hpp" #include "platform_utils.hpp" @@ -109,6 +112,82 @@ TEST(NodeLibraryManagerTest, ErrorWhenLibraryPathNotEscaped) { EXPECT_EQ(status, StatusCode::PATH_INVALID); } +TEST(NodeLibraryManagerTest, ModelZooObjectDetectionCapsAllOutputsToMaxOutputBatch) { + CustomNodeLibraryManager manager; + NodeLibrary library; + auto status = manager.loadLibrary("model_zoo_object_detection", getGenericFullPathForBazelOut("/ovms/bazel-bin/src/libcustom_node_model_zoo_intel_object_detection.so")); + ASSERT_EQ(status, StatusCode::OK); + ASSERT_EQ(manager.getLibrary("model_zoo_object_detection", library), StatusCode::OK); + + std::array params = {{{"original_image_height", "4"}, + {"original_image_width", "4"}, + {"target_image_height", "2"}, + {"target_image_width", "2"}, + {"confidence_threshold", "0.5"}, + {"max_output_batch", "2"}, + {"buffer_queue_size", "2"}}}; + + void* customNodeLibraryInternalManager = nullptr; + ASSERT_EQ(library.initialize(&customNodeLibraryInternalManager, params.data(), params.size()), 0); + + std::vector imageData(1 * 3 * 4 * 4, 1.0f); + std::vector imageDims{1, 3, 4, 4}; + CustomNodeTensor imageTensor{ + "image", + reinterpret_cast(imageData.data()), + static_cast(imageData.size() * sizeof(float)), + imageDims.data(), + imageDims.size(), + FP32}; + + const uint64_t detectionsCount = 5; + const uint64_t featuresCount = 7; + std::vector detectionData(detectionsCount * featuresCount, 0.0f); + for (size_t i = 0; i < detectionsCount; ++i) { + auto* detection = detectionData.data() + i * featuresCount; + detection[0] = 0.0f; // image_id + detection[1] = 1.0f; // label_id + detection[2] = 0.99f; // confidence + detection[3] = 0.1f; + detection[4] = 0.1f; + detection[5] = 0.9f; + detection[6] = 0.9f; + } + std::vector detectionDims{1, 1, detectionsCount, featuresCount}; + CustomNodeTensor detectionTensor{ + "detection", + reinterpret_cast(detectionData.data()), + static_cast(detectionData.size() * sizeof(float)), + detectionDims.data(), + detectionDims.size(), + FP32}; + + std::array inputs{imageTensor, detectionTensor}; + CustomNodeTensor* outputs = nullptr; + int outputsCount = 0; + + ASSERT_EQ(library.execute(inputs.data(), inputs.size(), &outputs, &outputsCount, params.data(), params.size(), customNodeLibraryInternalManager), 0); + ASSERT_NE(outputs, nullptr); + ASSERT_EQ(outputsCount, 4); + + constexpr uint64_t maxOutputBatch = 2; + for (int i = 0; i < outputsCount; ++i) { + ASSERT_NE(outputs[i].dims, nullptr); + ASSERT_GT(outputs[i].dimsCount, 0); + EXPECT_EQ(outputs[i].dims[0], maxOutputBatch); + } + EXPECT_EQ(outputs[1].dataBytes, sizeof(int32_t) * 4 * maxOutputBatch); // coordinates + EXPECT_EQ(outputs[2].dataBytes, sizeof(float) * maxOutputBatch); // confidences + EXPECT_EQ(outputs[3].dataBytes, sizeof(int32_t) * maxOutputBatch); // label_ids + + for (int i = 0; i < outputsCount; ++i) { + library.release(outputs[i].data, customNodeLibraryInternalManager); + library.release(outputs[i].dims, customNodeLibraryInternalManager); + } + library.release(outputs, customNodeLibraryInternalManager); + EXPECT_EQ(library.deinitialize(customNodeLibraryInternalManager), 0); +} + class ModelManagerNodeLibraryTest : public TestWithTempDir {}; TEST_F(ModelManagerNodeLibraryTest, LoadCustomNodeLibrary) { diff --git a/src/test/tensor_conversion_test.cpp b/src/test/tensor_conversion_test.cpp index 31006ca0fa..6c5797f129 100644 --- a/src/test/tensor_conversion_test.cpp +++ b/src/test/tensor_conversion_test.cpp @@ -181,6 +181,33 @@ TYPED_TEST(NativeFileInputConversionTest, positive_batch_size_2) { } } +TYPED_TEST(NativeFileInputConversionTest, negative_mixed_channels_with_ranged_channel_dimension) { + TypeParam mixedChannelsRequestTensor; + + size_t rgbFilesize; + std::unique_ptr rgbImageBytes; + readRgbJpg(rgbFilesize, rgbImageBytes); + + std::ifstream grayscaleDataFile; + grayscaleDataFile.open(getGenericFullPathForSrcTest("/ovms/src/test/binaryutils/grayscale.jpg"), std::ios::binary); + ASSERT_TRUE(grayscaleDataFile.is_open()); + grayscaleDataFile.seekg(0, std::ios::end); + std::streampos endPos = grayscaleDataFile.tellg(); + ASSERT_NE(endPos, std::streampos(-1)); + size_t grayscaleFilesize = static_cast(endPos); + grayscaleDataFile.seekg(0, std::ios::beg); + std::unique_ptr grayscaleImageBytes(new char[grayscaleFilesize]); + ASSERT_TRUE(grayscaleDataFile.read(grayscaleImageBytes.get(), grayscaleFilesize).good()); + + mixedChannelsRequestTensor.mutable_contents()->add_bytes_contents(rgbImageBytes.get(), rgbFilesize); + mixedChannelsRequestTensor.mutable_contents()->add_bytes_contents(grayscaleImageBytes.get(), grayscaleFilesize); + + ov::Tensor tensor; + auto tensorInfo = std::make_shared("", ovms::Precision::U8, ovms::Shape{2, 1, 1, {1, 3}}, Layout{"NHWC"}); + + ASSERT_EQ(convertNativeFileFormatRequestTensorToOVTensor(mixedChannelsRequestTensor, tensor, *tensorInfo, nullptr), ovms::StatusCode::INVALID_NO_OF_CHANNELS); +} + TYPED_TEST(NativeFileInputConversionTest, positive_precision_changed) { uint8_t rgb_precision_changed_expected_tensor[] = {0x24, 0x00, 0x00, 0x00, 0x1b, 0x00, 0x00, 0x00, 0xed, 0x00, 0x00, 0x00}; @@ -594,6 +621,31 @@ TEST_F(NativeFileInputConversionTestKFSRawInputsContents, Negative_invalidFormat ASSERT_EQ(convertNativeFileFormatRequestTensorToOVTensor(this->requestTensor, tensor, *tensorInfo, &this->buffer), ovms::StatusCode::INVALID_BATCH_SIZE); } +TEST_F(NativeFileInputConversionTestKFSRawInputsContents, Negative_mixedChannelsWithRangedChannelDimension) { + this->requestTensor.mutable_shape()->Clear(); + this->requestTensor.mutable_shape()->Add(2); + + std::ifstream grayscaleDataFile; + grayscaleDataFile.open(getGenericFullPathForSrcTest("/ovms/src/test/binaryutils/grayscale.jpg"), std::ios::binary); + ASSERT_TRUE(grayscaleDataFile.is_open()); + grayscaleDataFile.seekg(0, std::ios::end); + std::streampos endPos = grayscaleDataFile.tellg(); + ASSERT_NE(endPos, std::streampos(-1)); + const size_t grayscaleFilesize = static_cast(endPos); + grayscaleDataFile.seekg(0, std::ios::beg); + std::unique_ptr grayscaleImageBytes(new char[grayscaleFilesize]); + ASSERT_TRUE(grayscaleDataFile.read(grayscaleImageBytes.get(), static_cast(grayscaleFilesize)).good()); + + uint32_t grayscaleSize = static_cast(grayscaleFilesize); + this->buffer.append(reinterpret_cast(&grayscaleSize), sizeof(grayscaleSize)); + this->buffer.append(grayscaleImageBytes.get(), grayscaleFilesize); + + ov::Tensor tensor; + auto tensorInfo = std::make_shared("", ovms::Precision::U8, ovms::Shape{2, 1, 1, {1, 3}}, Layout{"NHWC"}); + + ASSERT_EQ(convertNativeFileFormatRequestTensorToOVTensor(this->requestTensor, tensor, *tensorInfo, &this->buffer), ovms::StatusCode::INVALID_NO_OF_CHANNELS); +} + template class StringInputsConversionTest : public ::testing::Test { public: @@ -807,6 +859,30 @@ TEST(StringInputsConversionKFSTest, rawInputContents_native_ov_string_shape_mism ASSERT_EQ(convertStringRequestToOVTensor(requestTensor, tensor, &rawInputContents), ovms::StatusCode::INVALID_STRING_INPUT); } +TEST(StringInputsConversionKFSTest, rawInputContents_native_ov_string_overflowed_length_prefixes_invalid) { + // Malformed 20-byte BYTES payload where a wrapped length accumulator could pass + // consistency checks and produce an out-of-bounds read during string assignment. + ::KFSRequest::InferInputTensor requestTensor; + requestTensor.set_datatype("BYTES"); + + std::string rawInputContents; + rawInputContents.resize(5 * sizeof(uint32_t)); + uint32_t words[] = { + 0x00000000u, + 0x00000008u, + 0x00000000u, + 0x00000004u, + 0xFFFFFFF8u}; + size_t offset = 0; + for (uint32_t word : words) { + std::memcpy(rawInputContents.data() + offset, &word, sizeof(word)); + offset += sizeof(word); + } + + ov::Tensor tensor; + ASSERT_EQ(convertStringRequestToOVTensor(requestTensor, tensor, &rawInputContents), ovms::StatusCode::INVALID_STRING_INPUT); +} + template class StringOutputsConversionTest : public ::testing::Test { public: diff --git a/src/test/test_utils.cpp b/src/test/test_utils.cpp index 89ab7569fc..5a5028fe34 100644 --- a/src/test/test_utils.cpp +++ b/src/test/test_utils.cpp @@ -267,11 +267,20 @@ bool isShapeTheSame(const KFSShapeType& actual, const std::vector&& exp void readFile(const std::string& path, size_t& filesize, std::unique_ptr& bytes) { std::ifstream DataFile; DataFile.open(path, std::ios::binary); + if (!DataFile.is_open()) { + throw std::runtime_error("Failed to open file: " + path); + } DataFile.seekg(0, std::ios::end); - filesize = DataFile.tellg(); - DataFile.seekg(0); + std::streampos endPos = DataFile.tellg(); + if (endPos == std::streampos(-1)) { + throw std::runtime_error("Failed to determine file size for: " + path); + } + filesize = static_cast(endPos); + DataFile.seekg(0, std::ios::beg); bytes = std::make_unique(filesize); - DataFile.read(bytes.get(), filesize); + if (!DataFile.read(bytes.get(), static_cast(filesize)).good()) { + throw std::runtime_error("Failed to read file: " + path); + } } void readRgbJpg(size_t& filesize, std::unique_ptr& image_bytes) {