Skip to content

Commit 149d29c

Browse files
authored
[EXPORTER] handling of invalid ports in UrlParser (#3142)
1 parent c81a3d4 commit 149d29c

File tree

2 files changed

+59
-5
lines changed

2 files changed

+59
-5
lines changed

ext/include/opentelemetry/ext/http/common/url_parser.h

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33

44
#pragma once
55

6-
#include <stdlib.h>
6+
#include <cerrno>
77
#include <cstdint>
8+
#include <cstdlib>
89
#include <string>
910

1011
#include "opentelemetry/version.h"
@@ -88,8 +89,11 @@ class UrlParser
8889
path_ = std::string("/"); // use default path
8990
if (is_port)
9091
{
91-
port_ = static_cast<uint16_t>(
92-
std::stoi(std::string(url_.begin() + cpos, url_.begin() + url_.length())));
92+
std::string port_str(
93+
url_.begin() + static_cast<std::string::difference_type>(cpos),
94+
url_.begin() + static_cast<std::string::difference_type>(url_.length()));
95+
96+
port_ = GetPort(port_str);
9397
}
9498
else
9599
{
@@ -99,8 +103,9 @@ class UrlParser
99103
}
100104
if (is_port)
101105
{
102-
port_ =
103-
static_cast<uint16_t>(std::stoi(std::string(url_.begin() + cpos, url_.begin() + pos)));
106+
std::string port_str(url_.begin() + static_cast<std::string::difference_type>(cpos),
107+
url_.begin() + static_cast<std::string::difference_type>(pos));
108+
port_ = GetPort(port_str);
104109
}
105110
else
106111
{
@@ -164,6 +169,20 @@ class UrlParser
164169

165170
return std::string::npos;
166171
}
172+
173+
std::uint16_t GetPort(const std::string &s)
174+
{
175+
char *e = nullptr;
176+
errno = 0;
177+
auto port = std::strtol(s.c_str(), &e, 10);
178+
if (e == s.c_str() || e != s.c_str() + s.size() || errno == ERANGE || port < 0 || port > 65535)
179+
{
180+
success_ = false;
181+
return 0;
182+
}
183+
184+
return static_cast<uint16_t>(port);
185+
}
167186
};
168187

169188
class UrlDecoder

ext/test/http/url_parser_test.cc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,41 @@ TEST(UrlParserTests, BasicTests)
179179
{"path", "/path1@bbb/path2"},
180180
{"query", "q1=a1&q2=a2"},
181181
{"success", "true"}}},
182+
{"https://https://example.com/some/path",
183+
{{"host", "https"},
184+
{"port", "0"},
185+
{"scheme", "https"},
186+
{"path", "//example.com/some/path"},
187+
{"query", ""},
188+
{"success", "false"}}},
189+
{"https://example.com:-1/some/path",
190+
{{"host", "example.com"},
191+
{"port", "0"},
192+
{"scheme", "https"},
193+
{"path", "/some/path"},
194+
{"query", ""},
195+
{"success", "false"}}},
196+
{"https://example.com:65536/some/path",
197+
{{"host", "example.com"},
198+
{"port", "0"},
199+
{"scheme", "https"},
200+
{"path", "/some/path"},
201+
{"query", ""},
202+
{"success", "false"}}},
203+
{"https://example.com:80a/some/path",
204+
{{"host", "example.com"},
205+
{"port", "0"},
206+
{"scheme", "https"},
207+
{"path", "/some/path"},
208+
{"query", ""},
209+
{"success", "false"}}},
210+
{"https://example.com:18446744073709551616/some/path",
211+
{{"host", "example.com"},
212+
{"port", "0"},
213+
{"scheme", "https"},
214+
{"path", "/some/path"},
215+
{"query", ""},
216+
{"success", "false"}}},
182217
};
183218
for (auto &url_map : urls_map)
184219
{

0 commit comments

Comments
 (0)