From 4b2e335d35c9320ff18b61647f1e8362d713d352 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Thu, 5 Nov 2020 16:01:08 -0800 Subject: [PATCH] Add GetPort to URL (#904) * Add GetPort to URL --- sdk/core/azure-core/CHANGELOG.md | 1 + .../azure-core/inc/azure/core/http/http.hpp | 13 +- sdk/core/azure-core/src/http/url.cpp | 26 +- sdk/core/azure-core/test/ut/CMakeLists.txt | 1 + sdk/core/azure-core/test/ut/http.cpp | 171 ------------ sdk/core/azure-core/test/ut/url.cpp | 255 ++++++++++++++++++ 6 files changed, 292 insertions(+), 175 deletions(-) create mode 100644 sdk/core/azure-core/test/ut/url.cpp diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 1f4d14ab2..63cd983c1 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -10,6 +10,7 @@ ### New Features - Added `strings.hpp` with `Azure::Core::Strings::LocaleInvariantCaseInsensitiveEqual` and `Azure::Core::Strings::ToLower`. - Added `OperationCanceledException`. +- Added `GetPort()` to `Url`. ### Other changes and Improvements diff --git a/sdk/core/azure-core/inc/azure/core/http/http.hpp b/sdk/core/azure-core/inc/azure/core/http/http.hpp index 10c74d803..9bb13986c 100644 --- a/sdk/core/azure-core/inc/azure/core/http/http.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/http.hpp @@ -234,7 +234,7 @@ namespace Azure { namespace Core { namespace Http { private: std::string m_scheme; std::string m_host; - int m_port{-1}; + uint16_t m_port{0}; std::string m_encodedPath; // query parameters are all encoded std::map m_encodedQueryParameters; @@ -384,6 +384,17 @@ namespace Azure { namespace Core { namespace Http { */ const std::string& GetPath() const { return m_encodedPath; } + /** + * @brief Get the port number set for the URL. + * + * @remark If the port was not set for the url, the returned port is 0. An HTTP request cannot + * be performed to port zero, an HTTP client is expected to set the default port depending on + * the request's schema when the port was not defined in the URL. + * + * @return The port number from the URL. + */ + uint16_t GetPort() const { return m_port; } + /** * @brief Provides a copy to the list of query parameters from the URL. * diff --git a/sdk/core/azure-core/src/http/url.cpp b/sdk/core/azure-core/src/http/url.cpp index 26f527603..967ff50af 100644 --- a/sdk/core/azure-core/src/http/url.cpp +++ b/sdk/core/azure-core/src/http/url.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include using namespace Azure::Core::Http; @@ -35,11 +36,30 @@ Url::Url(const std::string& url) { auto port_ite = std::find_if_not( pos + 1, url.end(), [](char c) { return std::isdigit(static_cast(c)); }); - m_port = std::stoi(std::string(pos + 1, port_ite)); + auto portNumber = std::stoi(std::string(pos + 1, port_ite)); + + // stoi will throw out_of_range when `int` is overflow, but we need to throw if uint16 is + // overflow + auto maxPortNumberSupported = std::numeric_limits::max(); + if (portNumber > maxPortNumberSupported) + { + throw std::out_of_range( + "The port number is out of range. The max supported number is " + + std::to_string(maxPortNumberSupported) + "."); + } + // cast is safe because the overflow was detected before + m_port = static_cast(portNumber); pos = port_ite; } - if (pos != url.end() && *pos == '/') + if (pos != url.end() && (*pos != '/') && (*pos != '?')) + { + // only char `\` or `?` is valid after the port (or the end of the url). Any other char is an + // invalid input + throw std::invalid_argument("The port number contains invalid characters."); + } + + if (pos != url.end() && (*pos == '/')) { auto pathIter = std::find(pos + 1, url.end(), '?'); m_encodedPath = std::string(pos + 1, pathIter); @@ -183,7 +203,7 @@ std::string Url::GetAbsoluteUrl() const full_url += m_scheme + "://"; } full_url += m_host; - if (m_port != -1) + if (m_port != 0) { full_url += ":" + std::to_string(m_port); } diff --git a/sdk/core/azure-core/test/ut/CMakeLists.txt b/sdk/core/azure-core/test/ut/CMakeLists.txt index 5054fcb9f..9d3ddc893 100644 --- a/sdk/core/azure-core/test/ut/CMakeLists.txt +++ b/sdk/core/azure-core/test/ut/CMakeLists.txt @@ -41,6 +41,7 @@ add_executable ( telemetry_policy.cpp transport_adapter.cpp transport_adapter_file_upload.cpp + url.cpp uuid.cpp ) diff --git a/sdk/core/azure-core/test/ut/http.cpp b/sdk/core/azure-core/test/ut/http.cpp index 0ac6909ae..9cf2dd565 100644 --- a/sdk/core/azure-core/test/ut/http.cpp +++ b/sdk/core/azure-core/test/ut/http.cpp @@ -13,177 +13,6 @@ using namespace Azure::Core; namespace Azure { namespace Core { namespace Test { - TEST(TestHttp, getters) - { - Http::HttpMethod httpMethod = Http::HttpMethod::Get; - Http::Url url("http://test.url.com"); - Http::Request req(httpMethod, url); - - // EXPECT_PRED works better than just EQ because it will print values in log - EXPECT_PRED2( - [](Http::HttpMethod a, Http::HttpMethod b) { return a == b; }, req.GetMethod(), httpMethod); - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - req.GetUrl().GetAbsoluteUrl(), - url.GetAbsoluteUrl()); - - EXPECT_NO_THROW(req.AddHeader("Name", "value")); - EXPECT_NO_THROW(req.AddHeader("naME2", "value2")); - - auto headers = req.GetHeaders(); - - // Headers should be lower-cased - EXPECT_TRUE(headers.count("name")); - EXPECT_TRUE(headers.count("name2")); - EXPECT_FALSE(headers.count("newHeader")); - - auto value = headers.find("name"); - EXPECT_PRED2([](std::string a, std::string b) { return a == b; }, value->second, "value"); - auto value2 = headers.find("name2"); - EXPECT_PRED2([](std::string a, std::string b) { return a == b; }, value2->second, "value2"); - - // now add to retry headers - req.StartTry(); - - // same headers first, then one new - EXPECT_NO_THROW(req.AddHeader("namE", "retryValue")); - EXPECT_NO_THROW(req.AddHeader("HEADER-to-Lower-123", "retryValue2")); - EXPECT_NO_THROW(req.AddHeader("newHeader", "new")); - - headers = req.GetHeaders(); - - EXPECT_TRUE(headers.count("name")); - EXPECT_TRUE(headers.count("header-to-lower-123")); - EXPECT_TRUE(headers.count("newheader")); - - value = headers.find("name"); - EXPECT_PRED2([](std::string a, std::string b) { return a == b; }, value->second, "retryValue"); - value2 = headers.find("header-to-lower-123"); - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, value2->second, "retryValue2"); - auto value3 = headers.find("newheader"); - EXPECT_PRED2([](std::string a, std::string b) { return a == b; }, value3->second, "new"); - - req.RemoveHeader("name"); - headers = req.GetHeaders(); - EXPECT_FALSE(headers.count("name")); - EXPECT_TRUE(headers.count("header-to-lower-123")); - EXPECT_TRUE(headers.count("newheader")); - - req.RemoveHeader("header-to-lower-123"); - headers = req.GetHeaders(); - EXPECT_FALSE(headers.count("name")); - EXPECT_FALSE(headers.count("header-to-lower-123")); - EXPECT_TRUE(headers.count("newheader")); - - req.RemoveHeader("newheader"); - headers = req.GetHeaders(); - EXPECT_FALSE(headers.count("name")); - EXPECT_FALSE(headers.count("header-to-lower-123")); - EXPECT_FALSE(headers.count("newheader")); - } - - TEST(TestHttp, query_parameter) - { - Http::HttpMethod httpMethod = Http::HttpMethod::Put; - Http::Url url("http://test.com"); - EXPECT_NO_THROW(url.AppendQueryParameter("query", "value")); - - Http::Request req(httpMethod, url); - - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - req.GetUrl().GetAbsoluteUrl(), - url.GetAbsoluteUrl()); - - Http::Url url_with_query("http://test.com?query=1"); - Http::Request req_with_query(httpMethod, url_with_query); - - // override if adding same query parameter key that is already in url - EXPECT_NO_THROW(req_with_query.GetUrl().AppendQueryParameter("query", "value")); - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - req_with_query.GetUrl().GetAbsoluteUrl(), - "http://test.com?query=value"); - - // retry query params testing - req_with_query.StartTry(); - // same query parameter should override previous - EXPECT_NO_THROW(req_with_query.GetUrl().AppendQueryParameter("query", "retryValue")); - - EXPECT_TRUE(req_with_query.m_retryModeEnabled); - - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - req_with_query.GetUrl().GetAbsoluteUrl(), - "http://test.com?query=retryValue"); - } - - TEST(TestHttp, query_parameter_encode_decode) - { - Http::HttpMethod httpMethod = Http::HttpMethod::Put; - Http::Url url("http://test.com"); - EXPECT_NO_THROW(url.AppendQueryParameter("query", Http::Url::Encode("va=lue"))); - - // Default encoder from URL won't encode an equal symbol - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - url.GetAbsoluteUrl(), - "http://test.com?query=va%3Dlue"); - - // Provide symbol to do not encode - EXPECT_NO_THROW(url.AppendQueryParameter("query", Http::Url::Encode("va=lue", "="))); - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - url.GetAbsoluteUrl(), - "http://test.com?query=va=lue"); - - // Provide more than one extra symbol to be encoded - EXPECT_NO_THROW(url.AppendQueryParameter("query", Http::Url::Encode("va=l u?e", " ?"))); - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - url.GetAbsoluteUrl(), - "http://test.com?query=va%3Dl u?e"); - - // default, encode it all - EXPECT_NO_THROW(url.AppendQueryParameter("query", Http::Url::Encode("va=l u?e"))); - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - url.GetAbsoluteUrl(), - "http://test.com?query=va%3Dl%20u%3Fe"); - } - - TEST(TestHttp, add_path) - { - Http::HttpMethod httpMethod = Http::HttpMethod::Post; - Http::Url url("http://test.com"); - Http::Request req(httpMethod, url); - - EXPECT_NO_THROW(req.GetUrl().AppendPath("path")); - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - req.GetUrl().GetAbsoluteUrl(), - "http://test.com/path"); - - EXPECT_NO_THROW(req.GetUrl().AppendQueryParameter("query", "value")); - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - req.GetUrl().GetAbsoluteUrl(), - "http://test.com/path?query=value"); - - EXPECT_NO_THROW(req.GetUrl().AppendPath("path2")); - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - req.GetUrl().GetAbsoluteUrl(), - "http://test.com/path/path2?query=value"); - - EXPECT_NO_THROW(req.GetUrl().AppendPath("path3")); - EXPECT_PRED2( - [](std::string a, std::string b) { return a == b; }, - req.GetUrl().GetAbsoluteUrl(), - "http://test.com/path/path2/path3?query=value"); - } - // Request - Add header TEST(TestHttp, add_headers) { diff --git a/sdk/core/azure-core/test/ut/url.cpp b/sdk/core/azure-core/test/ut/url.cpp new file mode 100644 index 000000000..ebc188725 --- /dev/null +++ b/sdk/core/azure-core/test/ut/url.cpp @@ -0,0 +1,255 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#include "gtest/gtest.h" + +#include + +using namespace Azure::Core; + +namespace Azure { namespace Core { namespace Test { + + TEST(URL, getters) + { + Http::HttpMethod httpMethod = Http::HttpMethod::Get; + Http::Url url("http://test.url.com"); + Http::Request req(httpMethod, url); + + // EXPECT_PRED works better than just EQ because it will print values in log + EXPECT_PRED2( + [](Http::HttpMethod a, Http::HttpMethod b) { return a == b; }, req.GetMethod(), httpMethod); + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + req.GetUrl().GetAbsoluteUrl(), + url.GetAbsoluteUrl()); + + EXPECT_NO_THROW(req.AddHeader("Name", "value")); + EXPECT_NO_THROW(req.AddHeader("naME2", "value2")); + + auto headers = req.GetHeaders(); + + // Headers should be lower-cased + EXPECT_TRUE(headers.count("name")); + EXPECT_TRUE(headers.count("name2")); + EXPECT_FALSE(headers.count("newHeader")); + + auto value = headers.find("name"); + EXPECT_PRED2([](std::string a, std::string b) { return a == b; }, value->second, "value"); + auto value2 = headers.find("name2"); + EXPECT_PRED2([](std::string a, std::string b) { return a == b; }, value2->second, "value2"); + + // now add to retry headers + req.StartTry(); + + // same headers first, then one new + EXPECT_NO_THROW(req.AddHeader("namE", "retryValue")); + EXPECT_NO_THROW(req.AddHeader("HEADER-to-Lower-123", "retryValue2")); + EXPECT_NO_THROW(req.AddHeader("newHeader", "new")); + + headers = req.GetHeaders(); + + EXPECT_TRUE(headers.count("name")); + EXPECT_TRUE(headers.count("header-to-lower-123")); + EXPECT_TRUE(headers.count("newheader")); + + value = headers.find("name"); + EXPECT_PRED2([](std::string a, std::string b) { return a == b; }, value->second, "retryValue"); + value2 = headers.find("header-to-lower-123"); + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, value2->second, "retryValue2"); + auto value3 = headers.find("newheader"); + EXPECT_PRED2([](std::string a, std::string b) { return a == b; }, value3->second, "new"); + + req.RemoveHeader("name"); + headers = req.GetHeaders(); + EXPECT_FALSE(headers.count("name")); + EXPECT_TRUE(headers.count("header-to-lower-123")); + EXPECT_TRUE(headers.count("newheader")); + + req.RemoveHeader("header-to-lower-123"); + headers = req.GetHeaders(); + EXPECT_FALSE(headers.count("name")); + EXPECT_FALSE(headers.count("header-to-lower-123")); + EXPECT_TRUE(headers.count("newheader")); + + req.RemoveHeader("newheader"); + headers = req.GetHeaders(); + EXPECT_FALSE(headers.count("name")); + EXPECT_FALSE(headers.count("header-to-lower-123")); + EXPECT_FALSE(headers.count("newheader")); + } + + TEST(URL, query_parameter) + { + Http::HttpMethod httpMethod = Http::HttpMethod::Put; + Http::Url url("http://test.com"); + EXPECT_NO_THROW(url.AppendQueryParameter("query", "value")); + + Http::Request req(httpMethod, url); + + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + req.GetUrl().GetAbsoluteUrl(), + url.GetAbsoluteUrl()); + + Http::Url url_with_query("http://test.com?query=1"); + Http::Request req_with_query(httpMethod, url_with_query); + + // override if adding same query parameter key that is already in url + EXPECT_NO_THROW(req_with_query.GetUrl().AppendQueryParameter("query", "value")); + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + req_with_query.GetUrl().GetAbsoluteUrl(), + "http://test.com?query=value"); + + // retry query params testing + req_with_query.StartTry(); + // same query parameter should override previous + EXPECT_NO_THROW(req_with_query.GetUrl().AppendQueryParameter("query", "retryValue")); + + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + req_with_query.GetUrl().GetAbsoluteUrl(), + "http://test.com?query=retryValue"); + } + + TEST(URL, query_parameter_encode_decode) + { + Http::HttpMethod httpMethod = Http::HttpMethod::Put; + Http::Url url("http://test.com"); + EXPECT_NO_THROW(url.AppendQueryParameter("query", Http::Url::Encode("va=lue"))); + + // Default encoder from URL won't encode an equal symbol + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + url.GetAbsoluteUrl(), + "http://test.com?query=va%3Dlue"); + + // Provide symbol to do not encode + EXPECT_NO_THROW(url.AppendQueryParameter("query", Http::Url::Encode("va=lue", "="))); + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + url.GetAbsoluteUrl(), + "http://test.com?query=va=lue"); + + // Provide more than one extra symbol to be encoded + EXPECT_NO_THROW(url.AppendQueryParameter("query", Http::Url::Encode("va=l u?e", " ?"))); + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + url.GetAbsoluteUrl(), + "http://test.com?query=va%3Dl u?e"); + + // default, encode it all + EXPECT_NO_THROW(url.AppendQueryParameter("query", Http::Url::Encode("va=l u?e"))); + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + url.GetAbsoluteUrl(), + "http://test.com?query=va%3Dl%20u%3Fe"); + } + + TEST(URL, add_path) + { + Http::HttpMethod httpMethod = Http::HttpMethod::Post; + Http::Url url("http://test.com"); + Http::Request req(httpMethod, url); + + EXPECT_NO_THROW(req.GetUrl().AppendPath("path")); + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + req.GetUrl().GetAbsoluteUrl(), + "http://test.com/path"); + + EXPECT_NO_THROW(req.GetUrl().AppendQueryParameter("query", "value")); + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + req.GetUrl().GetAbsoluteUrl(), + "http://test.com/path?query=value"); + + EXPECT_NO_THROW(req.GetUrl().AppendPath("path2")); + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + req.GetUrl().GetAbsoluteUrl(), + "http://test.com/path/path2?query=value"); + + EXPECT_NO_THROW(req.GetUrl().AppendPath("path3")); + EXPECT_PRED2( + [](std::string a, std::string b) { return a == b; }, + req.GetUrl().GetAbsoluteUrl(), + "http://test.com/path/path2/path3?query=value"); + } + + TEST(URL, getPort) + { + Http::Url url("http://test.com:9090"); + uint16_t expected = 9090; + + EXPECT_PRED2( + [](int expectedValue, int value) { return expectedValue == value; }, + url.GetPort(), + expected); + } + + TEST(URL, getPortConst) + { + Http::Url const url("https://test.com:500"); + uint16_t expected = 500; + + EXPECT_PRED2( + [](uint16_t expectedValue, uint16_t code) { return expectedValue == code; }, + url.GetPort(), + expected); + } + + TEST(URL, getPortMax) { EXPECT_THROW(Http::Url url("http://test.com:65540"), std::out_of_range); } + + TEST(URL, getPortAfterSet) + { + Http::Url url("http://test.com"); + url.SetPort(40); + uint16_t expected = 40; + + EXPECT_PRED2( + [](uint16_t expectedValue, uint16_t code) { return expectedValue == code; }, + url.GetPort(), + expected); + + url.SetPort(90); + expected = 90; + + EXPECT_PRED2( + [](uint16_t expectedValue, uint16_t code) { return expectedValue == code; }, + url.GetPort(), + expected); + } + + TEST(URL, getPortDefault) + { + Http::Url url("http://test.com"); + uint16_t expected = 0; + + EXPECT_PRED2( + [](uint16_t expectedValue, uint16_t code) { return expectedValue == code; }, + url.GetPort(), + expected); + } + + TEST(URL, getPorStartAsNonDigit) + { + EXPECT_THROW(Http::Url url("http://test.com:A1"), std::invalid_argument); + } + + TEST(URL, getPortInvalidInput) + { + EXPECT_THROW(Http::Url url("http://test.com:4A"), std::invalid_argument); + } + + TEST(URL, getPortInvalidArg) + { + EXPECT_THROW(Http::Url url("http://test.com:ThisIsNotAPort"), std::invalid_argument); + } + + TEST(URL, getPortOutfRange) + { + EXPECT_THROW(Http::Url url("http://test.com:99999999999999999"), std::out_of_range); + } +}}} // namespace Azure::Core::Test