Skip to content

Commit 4d9cc28

Browse files
authored
[EXPORTER] fix clang-tidy warnings in UrlParser (open-telemetry#3146)
1 parent 8e7a192 commit 4d9cc28

File tree

2 files changed

+43
-39
lines changed

2 files changed

+43
-39
lines changed

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

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <cstdint>
88
#include <cstdlib>
99
#include <string>
10+
#include <utility>
1011

1112
#include "opentelemetry/version.h"
1213

@@ -34,7 +35,7 @@ class UrlParser
3435
std::string query_;
3536
bool success_;
3637

37-
UrlParser(std::string url) : url_(url), success_(true)
38+
UrlParser(std::string url) : url_(std::move(url)), success_(true)
3839
{
3940
if (url_.length() == 0)
4041
{
@@ -50,15 +51,15 @@ class UrlParser
5051
}
5152
else
5253
{
53-
scheme_ = std::string(url_.begin() + cpos, url_.begin() + pos);
54+
scheme_ = url_.substr(cpos, pos - cpos);
5455
cpos = pos + 3;
5556
}
5657

5758
// credentials
58-
size_t pos1 = url_.find_first_of("@", cpos);
59-
size_t pos2 = url_.find_first_of("/", cpos);
59+
size_t pos1 = url_.find_first_of('@', cpos);
6060
if (pos1 != std::string::npos)
6161
{
62+
size_t pos2 = url_.find_first_of('/', cpos);
6263
// TODO - handle credentials
6364
if (pos2 == std::string::npos || pos1 < pos2)
6465
{
@@ -72,15 +73,19 @@ class UrlParser
7273
{
7374
// port missing. Used default 80 / 443
7475
if (scheme_ == "http")
76+
{
7577
port_ = 80;
76-
if (scheme_ == "https")
78+
}
79+
else if (scheme_ == "https")
80+
{
7781
port_ = 443;
82+
}
7883
}
7984
else
8085
{
8186
// port present
8287
is_port = true;
83-
host_ = std::string(url_.begin() + cpos, url_.begin() + pos);
88+
host_ = url_.substr(cpos, pos - cpos);
8489
cpos = pos + 1;
8590
}
8691
pos = url_.find_first_of("/?", cpos);
@@ -89,27 +94,23 @@ class UrlParser
8994
path_ = std::string("/"); // use default path
9095
if (is_port)
9196
{
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);
97+
auto port_str = url_.substr(cpos);
98+
port_ = GetPort(port_str);
9799
}
98100
else
99101
{
100-
host_ = std::string(url_.begin() + cpos, url_.begin() + url_.length());
102+
host_ = url_.substr(cpos);
101103
}
102104
return;
103105
}
104106
if (is_port)
105107
{
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);
108+
auto port_str = url_.substr(cpos, pos - cpos);
109+
port_ = GetPort(port_str);
109110
}
110111
else
111112
{
112-
host_ = std::string(url_.begin() + cpos, url_.begin() + pos);
113+
host_ = url_.substr(cpos, pos - cpos);
113114
}
114115
cpos = pos;
115116

@@ -118,21 +119,20 @@ class UrlParser
118119
pos = url_.find('?', cpos);
119120
if (pos == std::string::npos)
120121
{
121-
path_ = std::string(url_.begin() + cpos, url_.begin() + url_.length());
122-
query_ = "";
122+
path_ = url_.substr(cpos);
123123
}
124124
else
125125
{
126-
path_ = std::string(url_.begin() + cpos, url_.begin() + pos);
126+
path_ = url_.substr(cpos, pos - cpos);
127127
cpos = pos + 1;
128-
query_ = std::string(url_.begin() + cpos, url_.begin() + url_.length());
128+
query_ = url_.substr(cpos);
129129
}
130130
return;
131131
}
132132
path_ = std::string("/");
133133
if (url_[cpos] == '?')
134134
{
135-
query_ = std::string(url_.begin() + cpos, url_.begin() + url_.length());
135+
query_ = url_.substr(cpos);
136136
}
137137
}
138138

@@ -193,36 +193,39 @@ class UrlDecoder
193193
std::string result;
194194
result.reserve(encoded.size());
195195

196+
auto hex_to_int = [](int ch) -> int {
197+
if (ch >= '0' && ch <= '9')
198+
return ch - '0';
199+
if (ch >= 'a' && ch <= 'f')
200+
return ch - 'a' + 10;
201+
if (ch >= 'A' && ch <= 'F')
202+
return ch - 'A' + 10;
203+
return -1;
204+
};
205+
196206
for (size_t pos = 0; pos < encoded.size(); pos++)
197207
{
198-
if (encoded[pos] == '%')
208+
auto c = encoded[pos];
209+
if (c == '%')
199210
{
200-
201-
// Invalid input: less than two characters left after '%'
202-
if (encoded.size() < pos + 3)
211+
if (pos + 2 >= encoded.size())
203212
{
204213
return encoded;
205214
}
206215

207-
char hex[3] = {0};
208-
hex[0] = encoded[++pos];
209-
hex[1] = encoded[++pos];
210-
211-
char *endptr;
212-
long value = strtol(hex, &endptr, 16);
216+
int hi = hex_to_int(encoded[pos + 1]);
217+
int lo = hex_to_int(encoded[pos + 2]);
213218

214-
// Invalid input: no valid hex characters after '%'
215-
if (endptr != &hex[2])
219+
if (hi == -1 || lo == -1)
216220
{
217221
return encoded;
218222
}
219223

220-
result.push_back(static_cast<char>(value));
221-
}
222-
else
223-
{
224-
result.push_back(encoded[pos]);
224+
c = static_cast<char>((hi << 4) | lo);
225+
pos += 2;
225226
}
227+
228+
result.push_back(c);
226229
}
227230

228231
return result;

ext/test/http/url_parser_test.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,8 @@ TEST(UrlDecoderTests, BasicTests)
238238
{"%2x", "%2x"},
239239
{"%20", " "},
240240
{"text%2", "text%2"},
241-
};
241+
{"%20test%zztest", "%20test%zztest"},
242+
{"%20test%2", "%20test%2"}};
242243

243244
for (auto &testsample : testdata)
244245
{

0 commit comments

Comments
 (0)