Skip to content

Commit 6bbe61b

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 cd4ef14 commit 6bbe61b

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
@@ -698,6 +698,36 @@ class Tests {
698698
throw;
699699
}
700700
}
701+
702+
void TestBaseUrl() {
703+
std::cout << "TestBaseUrl()" << std::endl;
704+
std::vector<std::tuple<std::string, std::string, bool, int>> tests = {
705+
{"http://localhost:9000", "localhost", false, 9000},
706+
{"http://localhost", "localhost", false, 0},
707+
{"http://localhost:80", "localhost", false, 0},
708+
{"https://localhost:9443", "localhost:9443", true, 9443},
709+
{"https://localhost", "localhost", true, 0},
710+
{"https://5.localhost.foo", "5.localhost.foo", true, 0},
711+
{"https://5.localhost.foo:9000", "5.localhost.foo", true, 9000},
712+
};
713+
for (auto& [url, host, https, port] : tests) {
714+
minio::s3::BaseUrl base_url(url);
715+
if (base_url.host != host) {
716+
throw std::runtime_error("BaseUrl.Host(): expected: " + host +
717+
", got: " + base_url.host);
718+
}
719+
if (base_url.https != https) {
720+
throw std::runtime_error("BaseUrl.Secure(): expected: " +
721+
std::to_string(https) +
722+
", got: " + std::to_string(base_url.https));
723+
}
724+
if (base_url.port != port) {
725+
throw std::runtime_error("BaseUrl.Port(): expected: " +
726+
std::to_string(port) +
727+
", got: " + std::to_string(base_url.port));
728+
}
729+
}
730+
}
701731
}; // class Tests
702732

703733
int main(int /*argc*/, char* /*argv*/[]) {
@@ -755,6 +785,7 @@ int main(int /*argc*/, char* /*argv*/[]) {
755785
tests.RemoveObjects();
756786
tests.SelectObjectContent();
757787
tests.ListenBucketNotification();
788+
tests.TestBaseUrl();
758789

759790
return EXIT_SUCCESS;
760791
}

0 commit comments

Comments
 (0)