Skip to content

Commit

Permalink
attempt parsing of port only if colon was found
Browse files Browse the repository at this point in the history
without this change, urls like 5.my.s3.cluster were misinterpreted and
  the '5' parsed as the port of the url.

Bug:
* the getline() function puts the entire input string into the
  destination string, if the delimiter was not found.
  This leads to the entire host string being fed to the stoi()
  function to parse the port.

Fix:
* applied suggestion by @balamurugana
  • Loading branch information
Florian Kaiser committed Jan 5, 2025
1 parent b8ebbf3 commit 79d0b6a
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 5 deletions.
14 changes: 9 additions & 5 deletions src/http.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,18 +140,22 @@ Url Url::Parse(std::string value) {
unsigned int port = 0;
struct sockaddr_in6 dst;
if (inet_pton(AF_INET6, host.c_str(), &(dst.sin6_addr)) <= 0) {
if (host.front() != '[' || host.back() != ']') {
if ((host.front() != '[' || host.back() != ']') && utils::Contains(host, ':')) {
std::stringstream ss(host);
std::string portstr;
while (std::getline(ss, portstr, ':')) {
}

if (!portstr.empty()) {
try {
port = static_cast<unsigned>(std::stoi(portstr));
char* str_end{};
const long l = std::strtol(portstr.c_str(), &str_end, 10);
size_t length = std::distance(portstr.c_str(), const_cast<const char*>(str_end));
if (length == portstr.size()) {
if (l < 1 || l > 65535) {
return Url{};
}
port = static_cast<int>(l);
host = host.substr(0, host.rfind(":" + portstr));
} catch (const std::invalid_argument&) {
port = 0;
}
}
}
Expand Down
31 changes: 31 additions & 0 deletions tests/tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,36 @@ class Tests {
throw;
}
}

void TestBaseUrl() {
std::cout << "TestBaseUrl()" << std::endl;
std::vector<std::tuple<std::string, std::string, bool, int>> tests = {
{"http://localhost:9000", "localhost", false, 9000},
{"http://localhost", "localhost", false, 0},
{"http://localhost:80", "localhost", false, 0},
{"https://localhost:9443", "localhost", true, 9443},
{"https://localhost", "localhost", true, 0},
{"https://5.localhost.foo", "5.localhost.foo", true, 0},
{"https://5.localhost.foo:9000", "5.localhost.foo", true, 9000},
};
for (auto& [url, host, https, port] : tests) {
minio::s3::BaseUrl base_url(url);
if (base_url.host != host) {
throw std::runtime_error("BaseUrl(" + url +").host: expected: " + host +
", got: " + base_url.host);
}
if (base_url.https != https) {
throw std::runtime_error("BaseUrl("+ url +").https: expected: " +
std::to_string(https) +
", got: " + std::to_string(base_url.https));
}
if (base_url.port != port) {
throw std::runtime_error("BaseUrl("+ url +").port: expected: " +
std::to_string(port) +
", got: " + std::to_string(base_url.port));
}
}
}
}; // class Tests

int main(int /*argc*/, char* /*argv*/[]) {
Expand Down Expand Up @@ -756,6 +786,7 @@ int main(int /*argc*/, char* /*argv*/[]) {
tests.RemoveObjects();
tests.SelectObjectContent();
tests.ListenBucketNotification();
tests.TestBaseUrl();

return EXIT_SUCCESS;
}

0 comments on commit 79d0b6a

Please sign in to comment.