diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 7182cd8f9..5dd93f6d9 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -14,6 +14,10 @@ - Add high-level and simplified core.hpp file for simpler include experience for customers. +### Bug Fixes + +- Prevent pipeline of length zero to be created. + ## 1.0.0-beta.2 (2020-10-09) ### Breaking Changes diff --git a/sdk/core/azure-core/inc/azure/core/http/pipeline.hpp b/sdk/core/azure-core/inc/azure/core/http/pipeline.hpp index 6a50dcf0f..837e422e9 100644 --- a/sdk/core/azure-core/inc/azure/core/http/pipeline.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/pipeline.hpp @@ -37,9 +37,16 @@ namespace Azure { namespace Core { namespace Http { * * @param policies A sequence of #HttpPolicy representing a stack, first element corresponding * to the top of the stack. + * + * @throw `std::invalid_argument` when policies is empty. */ explicit HttpPipeline(const std::vector>& policies) { + if (policies.size() == 0) + { + throw std::invalid_argument("policies cannot be empty"); + } + m_policies.reserve(policies.size()); for (auto&& policy : policies) { @@ -52,13 +59,25 @@ namespace Azure { namespace Core { namespace Http { * * @param policies A sequence of #HttpPolicy representing a stack, first element corresponding * to the top of the stack. + * + * @throw `std::invalid_argument` when policies is empty. */ explicit HttpPipeline(std::vector>&& policies) : m_policies(std::move(policies)) { + if (m_policies.size() == 0) + { + throw std::invalid_argument("policies cannot be empty"); + } } - /// Copy constructor. + /** + * @brief Copy constructor. + * + * @remark \p other is expected to have at least one policy. + * + * @param other + */ HttpPipeline(const HttpPipeline& other) { m_policies.reserve(other.m_policies.size()); @@ -78,6 +97,8 @@ namespace Azure { namespace Core { namespace Http { */ std::unique_ptr Send(Context const& ctx, Request& request) const { + // Accessing position zero is fine because pipeline must be constructed with at least one + // policy. return m_policies[0]->Send(ctx, request, NextHttpPolicy(0, &m_policies)); } }; diff --git a/sdk/core/azure-core/test/ut/CMakeLists.txt b/sdk/core/azure-core/test/ut/CMakeLists.txt index 4ee64796f..ba167ea7f 100644 --- a/sdk/core/azure-core/test/ut/CMakeLists.txt +++ b/sdk/core/azure-core/test/ut/CMakeLists.txt @@ -28,6 +28,7 @@ add_executable ( logging.cpp main.cpp nullable.cpp + pipeline.cpp string.cpp telemetry_policy.cpp transport_adapter.cpp diff --git a/sdk/core/azure-core/test/ut/pipeline.cpp b/sdk/core/azure-core/test/ut/pipeline.cpp new file mode 100644 index 000000000..9ae0be292 --- /dev/null +++ b/sdk/core/azure-core/test/ut/pipeline.cpp @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#include "gtest/gtest.h" +#include +#include + +#include + +TEST(Logging, createPipeline) +{ + // Construct pipeline without exception + std::vector> policies; + policies.push_back(std::make_unique("test", "test")); + + EXPECT_NO_THROW(Azure::Core::Http::HttpPipeline pipeline(policies)); +} + +TEST(Logging, createEmptyPipeline) +{ + // throw invalid arg for empty policies + std::vector> policies; + EXPECT_THROW(Azure::Core::Http::HttpPipeline pipeline(policies), std::invalid_argument); +} + +TEST(Logging, clonePipeline) +{ + // Construct pipeline without exception and clone + std::vector> policies; + policies.push_back(std::make_unique("test", "test")); + + Azure::Core::Http::HttpPipeline pipeline(policies); + EXPECT_NO_THROW(Azure::Core::Http::HttpPipeline pipeline2(pipeline)); +} + +TEST(Logging, refrefPipeline) +{ + // Construct pipeline without exception + EXPECT_NO_THROW(Azure::Core::Http::HttpPipeline pipeline( + std::vector>(1))); +} + +TEST(Logging, refrefEmptyPipeline) +{ + // Construct pipeline with invalid exception with move constructor + EXPECT_THROW( + Azure::Core::Http::HttpPipeline pipeline( + std::vector>(0)), + std::invalid_argument); +}