Skip to content

Commit 56953ec

Browse files
aalkinclaude
aalkin
and
claude
committed
Do not leak input buffers in test_HTTPParser
Request and frame inputs were strdup'ed and never freed. Use string_view on literals, make parse_http_request take char const*, and free the malloc'ed buffers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 4569fff commit 56953ec

3 files changed

Lines changed: 46 additions & 45 deletions

File tree

‎Framework/Core/src/HTTPParser.cxx‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,14 +226,13 @@ std::string encode_websocket_handshake_reply(char const* nonce, const char* prot
226226
return fmt::format(res, protocol && protocol[0] ? fmt::format("Sec-WebSocket-Protocol: {}\r\n", protocol) : "", HTTPParserHelpers::calculateAccept(nonce));
227227
}
228228

229-
void parse_http_request(char* start, size_t size, HTTPParser* parser)
229+
void parse_http_request(char const* start, size_t size, HTTPParser* parser)
230230
{
231-
enum HTTPState state = HTTPState::IN_START;
232231
// Too short, let's try again...
233232
if (size < 2) {
234233
parser->remaining += std::string_view(start, size);
235234
}
236-
char* cur = start;
235+
char* cur = (char*)start;
237236
char* next = cur;
238237
std::string_view lastToken(cur, 0);
239238
std::string_view lastKey;

‎Framework/Core/src/HTTPParser.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ struct HTTPParserHelpers {
236236
static std::string calculateAccept(const char* nonce);
237237
};
238238

239-
void parse_http_request(char* start, size_t size, HTTPParser* parser);
239+
void parse_http_request(char const* start, size_t size, HTTPParser* parser);
240240

241241
std::pair<std::string, unsigned short> parse_websocket_url(const char* s);
242242
} // namespace o2::framework

‎Framework/Core/test/test_HTTPParser.cxx‎

Lines changed: 43 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class DPLParser : public HTTPParser
3939
{
4040
mHeaders[std::string(k)] = v;
4141
}
42-
void body(char* buf, size_t s) override
42+
void body(char* buf, size_t /*s*/) override
4343
{
4444
mBody = buf;
4545
}
@@ -71,7 +71,7 @@ class DPLClientParser : public HTTPParser
7171
{
7272
mHeaders[std::string(k)] = v;
7373
}
74-
void body(char* buf, size_t s) override
74+
void body(char* buf, size_t /*s*/) override
7575
{
7676
mBody = buf;
7777
}
@@ -92,23 +92,23 @@ class TestWSHandler : public WebSocketHandler
9292
TEST_CASE("HTTPParser1")
9393
{
9494
{
95-
char* request = strdup(
95+
std::string_view request =
9696
"GET / HTTP/1.1\r\n"
97-
"x-dpl-pid: 124679842\r\n\r\nCONTROL QUIT");
97+
"x-dpl-pid: 124679842\r\n\r\nCONTROL QUIT";
9898
DPLParser parser;
99-
parse_http_request(request, strlen(request), &parser);
99+
parse_http_request(request.data(), request.size(), &parser);
100100
REQUIRE(std::string(parser.mMethod) == std::string("GET"));
101101
REQUIRE(std::string(parser.mPath) == std::string("/"));
102102
REQUIRE(std::string(parser.mVersion) == std::string("HTTP/1.1"));
103103
REQUIRE(parser.mHeaders.size() == 1);
104104
}
105105
{
106-
char* request = strdup(
106+
std::string_view request =
107107
"GET / HTTP/1.1\r\n"
108108
"x-dpl-pid: 124679842\r\n"
109-
"Somethingelse: cjnjsdnjks\r\n\r\nCONTROL QUIT");
109+
"Somethingelse: cjnjsdnjks\r\n\r\nCONTROL QUIT";
110110
DPLParser parser;
111-
parse_http_request(request, strlen(request), &parser);
111+
parse_http_request(request.data(), request.size(), &parser);
112112
REQUIRE(std::string(parser.mMethod) == std::string("GET"));
113113
REQUIRE(std::string(parser.mPath) == std::string("/"));
114114
REQUIRE(std::string(parser.mVersion) == std::string("HTTP/1.1"));
@@ -119,38 +119,38 @@ TEST_CASE("HTTPParser1")
119119
}
120120
{
121121
// handle continuations...
122-
char* request = strdup(
122+
std::string_view request =
123123
"GET / HTTP/1.1\r\n"
124124
"x-dpl-pid: 124679842\r\n"
125-
"Somethingelse: cjnjsdnjks\r\n\r\nCONTROL QUIT");
126-
char* request2 = strdup("FOO BAR");
125+
"Somethingelse: cjnjsdnjks\r\n\r\nCONTROL QUIT";
126+
std::string_view request2 = "FOO BAR";
127127
DPLParser parser;
128-
parse_http_request(request, strlen(request), &parser);
128+
parse_http_request(request.data(), request.size(), &parser);
129129
REQUIRE(std::string(parser.mMethod) == std::string("GET"));
130130
REQUIRE(std::string(parser.mPath) == std::string("/"));
131131
REQUIRE(std::string(parser.mVersion) == std::string("HTTP/1.1"));
132132
REQUIRE(parser.mHeaders.size() == 2);
133133
REQUIRE(parser.mHeaders["x-dpl-pid"] == "124679842");
134134
REQUIRE(parser.mHeaders["Somethingelse"] == "cjnjsdnjks");
135135
REQUIRE(parser.mBody == "CONTROL QUIT");
136-
parse_http_request(request2, strlen(request2), &parser);
136+
parse_http_request(request2.data(), request2.size(), &parser);
137137
REQUIRE(parser.mBody == "FOO BAR");
138138
}
139139

140140
{
141141
// WebSocket example
142-
char* request = strdup(
142+
std::string_view request =
143143
"GET /chat HTTP/1.1\r\n"
144144
"Host: server.example.com\r\n"
145145
"Upgrade: websocket\r\n"
146146
"Connection: Upgrade\r\n"
147147
"Sec-WebSocket-Key: x3JJHMbDL1EzLkh9GBhXDw==\r\n"
148148
"Sec-WebSocket-Protocol: chat, superchat\r\n"
149149
"Sec-WebSocket-Version: 13\r\n"
150-
"Origin: http://example.com\r\n\r\n");
150+
"Origin: http://example.com\r\n\r\n";
151151

152152
DPLParser parser;
153-
parse_http_request(request, strlen(request), &parser);
153+
parse_http_request(request.data(), request.size(), &parser);
154154
REQUIRE(std::string(parser.mMethod) == std::string("GET"));
155155
REQUIRE(std::string(parser.mPath) == std::string("/chat"));
156156
REQUIRE(std::string(parser.mVersion) == std::string("HTTP/1.1"));
@@ -159,16 +159,16 @@ TEST_CASE("HTTPParser1")
159159
}
160160
{
161161
// WebSocket example
162-
char* request = strdup(
162+
std::string_view request =
163163
"HTTP/1.1 101 Switching Protocols\r\n"
164164
"Upgrade: websocket\r\n"
165165
"Connection: Upgrade\r\n"
166166
"Access-Control-Allow-Origin: \"*\"\r\n"
167-
"Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n\r\n");
167+
"Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n\r\n";
168168

169169
DPLClientParser parser;
170170
parser.states.push_back(HTTPState::IN_START_REPLY);
171-
parse_http_request(request, strlen(request), &parser);
171+
parse_http_request(request.data(), request.size(), &parser);
172172
REQUIRE(std::string(parser.mReplyCode) == std::string("101"));
173173
REQUIRE(std::string(parser.mReplyMessage) == std::string("Switching Protocols"));
174174
REQUIRE(std::string(parser.mReplyVersion) == std::string("HTTP/1.1"));
@@ -178,23 +178,23 @@ TEST_CASE("HTTPParser1")
178178
}
179179
{
180180
// WebSocket frame encoding / decoding
181-
char* buffer = strdup("hello websockets!");
181+
std::string_view buffer = "hello websockets!";
182182
std::vector<uv_buf_t> encoded;
183-
encode_websocket_frames(encoded, buffer, strlen(buffer) + 1, WebSocketOpCode::Binary, 0);
183+
encode_websocket_frames(encoded, buffer.data(), buffer.size() + 1, WebSocketOpCode::Binary, 0);
184184
REQUIRE(encoded.size() == 1);
185185
TestWSHandler handler;
186-
REQUIRE(encoded[0].len == strlen(buffer) + 1 + 2);
186+
REQUIRE(encoded[0].len == buffer.size() + 1 + 2);
187187
; // 1 for the 0, 2 for the header
188188
decode_websocket(encoded[0].base, encoded[0].len, handler);
189-
REQUIRE(handler.mSize[0] == strlen(buffer) + 1);
189+
REQUIRE(handler.mSize[0] == buffer.size() + 1);
190190
REQUIRE(handler.mFrame[0].substr(0, handler.mSize[0] - 1) == std::string(buffer));
191191
std::ranges::for_each(encoded, [](auto& buf) { free(buf.base); });
192192
}
193193
{
194194
// WebSocket multiple frame encoding / decoding
195-
char* buffer = strdup("hello websockets!");
195+
std::string_view buffer = "hello websockets!";
196196
std::vector<uv_buf_t> encoded;
197-
encode_websocket_frames(encoded, buffer, strlen(buffer) + 1, WebSocketOpCode::Binary, 0);
197+
encode_websocket_frames(encoded, buffer.data(), buffer.size() + 1, WebSocketOpCode::Binary, 0);
198198
REQUIRE(encoded.size() == 1);
199199
char const* prototype = "and again.";
200200
char* buffer2 = (char*)malloc(0x20000);
@@ -218,12 +218,14 @@ TEST_CASE("HTTPParser1")
218218
REQUIRE(handler.mFrame[0].substr(0, handler.mSize[0] - 1) == std::string(buffer));
219219
REQUIRE(handler.mFrame[1].substr(0, handler.mSize[1] - 1) == std::string(buffer2));
220220
std::ranges::for_each(encoded, [](auto& buf) { free(buf.base); });
221+
free(multiBuffer);
222+
free(buffer2);
221223
}
222224
{
223225
// Decode a frame which is split in two.
224-
char* buffer = strdup("hello websockets!1");
226+
std::string_view buffer = "hello websockets!1";
225227
std::vector<uv_buf_t> encoded;
226-
encode_websocket_frames(encoded, buffer, strlen(buffer) + 1, WebSocketOpCode::Binary, 0);
228+
encode_websocket_frames(encoded, buffer.data(), buffer.size() + 1, WebSocketOpCode::Binary, 0);
227229
REQUIRE(encoded.size() == 1);
228230

229231
TestWSHandler handler;
@@ -236,9 +238,9 @@ TEST_CASE("HTTPParser1")
236238
}
237239
{
238240
// Decode a long frame which is split in two.
239-
char* buffer = strdup("string with more than 127 characters: cdsklcmalkmc cdmslkc adslkccmkadsc adslkmc dsa ckdls cdksclknds lkndnc anslkc klsad ckl lksad clkas ccdascnkjancjnjkascsa cdascds clsad nclksad ncklsd clkadns lkc sadnlk cklsa cnaksld csad");
241+
std::string_view buffer = "string with more than 127 characters: cdsklcmalkmc cdmslkc adslkccmkadsc adslkmc dsa ckdls cdksclknds lkndnc anslkc klsad ckl lksad clkas ccdascnkjancjnjkascsa cdascds clsad nclksad ncklsd clkadns lkc sadnlk cklsa cnaksld csad";
240242
std::vector<uv_buf_t> encoded;
241-
encode_websocket_frames(encoded, buffer, strlen(buffer) + 1, WebSocketOpCode::Binary, 0);
243+
encode_websocket_frames(encoded, buffer.data(), buffer.size() + 1, WebSocketOpCode::Binary, 0);
242244
REQUIRE(encoded.size() == 1);
243245

244246
TestWSHandler handler;
@@ -251,12 +253,12 @@ TEST_CASE("HTTPParser1")
251253
}
252254
{
253255
// WebSocket multiple frame encoding / decoding, long frames
254-
char* buffer = strdup("dwqnocewnclkanklcdanslkcndklsnclkdsnckldsnclk cnldcl dsklc dslk cljdnsck sdlakcn askc sdkla cnsd c sdcn dsklncn dklsc nsdkl cklds clkds ckls dklc shello websockets!");
256+
std::string_view buffer = "dwqnocewnclkanklcdanslkcndklsnclkdsnckldsnclk cnldcl dsklc dslk cljdnsck sdlakcn askc sdkla cnsd c sdcn dsklncn dklsc nsdkl cklds clkds ckls dklc shello websockets!";
255257
std::vector<uv_buf_t> encoded;
256-
encode_websocket_frames(encoded, buffer, strlen(buffer) + 1, WebSocketOpCode::Binary, 0);
258+
encode_websocket_frames(encoded, buffer.data(), buffer.size() + 1, WebSocketOpCode::Binary, 0);
257259
REQUIRE(encoded.size() == 1);
258-
char const* buffer2 = "xsanjkcnsadjknc dsjc nsdnc dlscndsck dsc ds clds cds vnlsfl nklnjk nj nju n nio nkmnklfmdkl mkld mkl mkl mkl mlk m lkm klfdnkln jkafdnk nk mkldfm lkdamlkdmlkdmlk m klml km lkm kl.";
259-
encode_websocket_frames(encoded, buffer2, strlen(buffer2) + 1, WebSocketOpCode::Binary, 0);
260+
std::string_view buffer2 = "xsanjkcnsadjknc dsjc nsdnc dlscndsck dsc ds clds cds vnlsfl nklnjk nj nju n nio nkmnklfmdkl mkld mkl mkl mkl mlk m lkm klfdnkln jkafdnk nk mkldfm lkdamlkdmlkdmlk m klml km lkm kl.";
261+
encode_websocket_frames(encoded, buffer2.data(), buffer2.size() + 1, WebSocketOpCode::Binary, 0);
260262
REQUIRE(encoded.size() == 1);
261263

262264
TestWSHandler handler;
@@ -269,12 +271,12 @@ TEST_CASE("HTTPParser1")
269271
}
270272
{
271273
// Decode a long frame which is split in two, after the first byte.
272-
char* buffer = strdup("string with more than 127 characters: cdsklcmalkmc cdmslkc adslkccmkadsc adslkmc dsa ckdls cdksclknds lkndnc anslkc klsad ckl lksad clkas ccdascnkjancjnjkascsa cdascds clsad nclksad ncklsd clkadns lkc sadnlk cklsa cnaksld csad");
274+
std::string_view buffer = "string with more than 127 characters: cdsklcmalkmc cdmslkc adslkccmkadsc adslkmc dsa ckdls cdksclknds lkndnc anslkc klsad ckl lksad clkas ccdascnkjancjnjkascsa cdascds clsad nclksad ncklsd clkadns lkc sadnlk cklsa cnaksld csad";
273275
std::vector<uv_buf_t> encoded;
274-
encode_websocket_frames(encoded, buffer, strlen(buffer) + 1, WebSocketOpCode::Binary, 0);
276+
encode_websocket_frames(encoded, buffer.data(), buffer.size() + 1, WebSocketOpCode::Binary, 0);
275277
REQUIRE(encoded.size() == 1);
276278

277-
for (size_t i = 1; i < strlen(buffer); ++i) {
279+
for (size_t i = 1; i < buffer.size(); ++i) {
278280
char buffer1[1024];
279281
char buffer2[1024];
280282
memset(buffer1, 0xfa, 1024);
@@ -292,13 +294,13 @@ TEST_CASE("HTTPParser1")
292294
}
293295
{
294296
// Decode a long frame which is split in two, after the first byte.
295-
char* buffer = strdup("string with more than 127 characters: cdsklcmalkmc cdmslkc adslkccmkadsc adslkmc dsa ckdls cdksclknds lkndnc anslkc klsad ckl lksad clkas ccdascnkjancjnjkascsa cdascds clsad nclksad ncklsd clkadns lkc sadnlk cklsa cnaksld csad");
297+
std::string_view buffer = "string with more than 127 characters: cdsklcmalkmc cdmslkc adslkccmkadsc adslkmc dsa ckdls cdksclknds lkndnc anslkc klsad ckl lksad clkas ccdascnkjancjnjkascsa cdascds clsad nclksad ncklsd clkadns lkc sadnlk cklsa cnaksld csad";
296298
std::vector<uv_buf_t> encoded;
297-
encode_websocket_frames(encoded, buffer, strlen(buffer) + 1, WebSocketOpCode::Binary, 0);
299+
encode_websocket_frames(encoded, buffer.data(), buffer.size() + 1, WebSocketOpCode::Binary, 0);
298300
REQUIRE(encoded.size() == 1);
299301

300-
for (size_t i = 0; i < strlen(buffer) - 1; ++i) {
301-
for (size_t j = i + 1; j < strlen(buffer); ++j) {
302+
for (size_t i = 0; i < buffer.size() - 1; ++i) {
303+
for (size_t j = i + 1; j < buffer.size(); ++j) {
302304
char buffer1[1024];
303305
char buffer2[1024];
304306
char buffer3[1024];
@@ -335,7 +337,7 @@ TEST_CASE("HTTPParser1")
335337
"Connection: Upgrade\r\n"
336338
"Access-Control-Allow-Origin: \"*\"\r\n"
337339
"Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n\r\n";
338-
int someSeed = 123;
340+
339341
std::string result = encode_websocket_handshake_request("/chat", "myprotocol", 13, "dGhlIHNhbXBsZSBub25jZQ==");
340342
REQUIRE(result == checkRequest);
341343

0 commit comments

Comments
 (0)