Skip to content

Commit 79d0b6a

Browse files
author
Florian Kaiser
committed
attempt parsing of port only if colon was found
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
1 parent b8ebbf3 commit 79d0b6a

File tree

2 files changed

+40
-5
lines changed

2 files changed

+40
-5
lines changed

src/http.cc

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,18 +140,22 @@ Url Url::Parse(std::string value) {
140140
unsigned int port = 0;
141141
struct sockaddr_in6 dst;
142142
if (inet_pton(AF_INET6, host.c_str(), &(dst.sin6_addr)) <= 0) {
143-
if (host.front() != '[' || host.back() != ']') {
143+
if ((host.front() != '[' || host.back() != ']') && utils::Contains(host, ':')) {
144144
std::stringstream ss(host);
145145
std::string portstr;
146146
while (std::getline(ss, portstr, ':')) {
147147
}
148148

149149
if (!portstr.empty()) {
150-
try {
151-
port = static_cast<unsigned>(std::stoi(portstr));
150+
char* str_end{};
151+
const long l = std::strtol(portstr.c_str(), &str_end, 10);
152+
size_t length = std::distance(portstr.c_str(), const_cast<const char*>(str_end));
153+
if (length == portstr.size()) {
154+
if (l < 1 || l > 65535) {
155+
return Url{};
156+
}
157+
port = static_cast<int>(l);
152158
host = host.substr(0, host.rfind(":" + portstr));
153-
} catch (const std::invalid_argument&) {
154-
port = 0;
155159
}
156160
}
157161
}

tests/tests.cc

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,36 @@ class Tests {
699699
throw;
700700
}
701701
}
702+
703+
void TestBaseUrl() {
704+
std::cout << "TestBaseUrl()" << std::endl;
705+
std::vector<std::tuple<std::string, std::string, bool, int>> tests = {
706+
{"http://localhost:9000", "localhost", false, 9000},
707+
{"http://localhost", "localhost", false, 0},
708+
{"http://localhost:80", "localhost", false, 0},
709+
{"https://localhost:9443", "localhost", true, 9443},
710+
{"https://localhost", "localhost", true, 0},
711+
{"https://5.localhost.foo", "5.localhost.foo", true, 0},
712+
{"https://5.localhost.foo:9000", "5.localhost.foo", true, 9000},
713+
};
714+
for (auto& [url, host, https, port] : tests) {
715+
minio::s3::BaseUrl base_url(url);
716+
if (base_url.host != host) {
717+
throw std::runtime_error("BaseUrl(" + url +").host: expected: " + host +
718+
", got: " + base_url.host);
719+
}
720+
if (base_url.https != https) {
721+
throw std::runtime_error("BaseUrl("+ url +").https: expected: " +
722+
std::to_string(https) +
723+
", got: " + std::to_string(base_url.https));
724+
}
725+
if (base_url.port != port) {
726+
throw std::runtime_error("BaseUrl("+ url +").port: expected: " +
727+
std::to_string(port) +
728+
", got: " + std::to_string(base_url.port));
729+
}
730+
}
731+
}
702732
}; // class Tests
703733

704734
int main(int /*argc*/, char* /*argv*/[]) {
@@ -756,6 +786,7 @@ int main(int /*argc*/, char* /*argv*/[]) {
756786
tests.RemoveObjects();
757787
tests.SelectObjectContent();
758788
tests.ListenBucketNotification();
789+
tests.TestBaseUrl();
759790

760791
return EXIT_SUCCESS;
761792
}

0 commit comments

Comments
 (0)