Skip to content

Commit e45f765

Browse files
RamseyKRamseyK
committed
Bugfixes sync'd from ByteBufferCpp improving HTTP parsing and eliminating a few bugs
1 parent 1cd1016 commit e45f765

File tree

6 files changed

+75
-56
lines changed

6 files changed

+75
-56
lines changed

‎src/ByteBuffer.cpp‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#ifdef BB_UTILITY
2222
#include <iomanip>
2323
#include <iostream>
24+
#include <print>
2425
#include <string>
2526
#endif
2627

@@ -65,7 +66,7 @@ ByteBuffer::ByteBuffer(const uint8_t* arr, uint32_t size) {
6566
* @return Number of bytes from rpos to the end (size())
6667
*/
6768
uint32_t ByteBuffer::bytesRemaining() const {
68-
return size() - rpos;
69+
return rpos >= size() ? 0 : size() - rpos;
6970
}
7071

7172
/**
@@ -329,44 +330,43 @@ std::string ByteBuffer::getName() const {
329330

330331
void ByteBuffer::printInfo() const {
331332
uint32_t length = buf.size();
332-
std::cout << "ByteBuffer " << name << " Length: " << length << ". Info Print" << std::endl;
333+
std::print("ByteBuffer {} Length: {}. Info Print\n", name, length);
333334
}
334335

335336
void ByteBuffer::printAH() const {
336337
uint32_t length = buf.size();
337-
std::cout << "ByteBuffer " << name << " Length: " << length << ". ASCII & Hex Print" << std::endl;
338+
std::print("ByteBuffer {} Length: {}. ASCII & Hex Print\n", name, length);
338339
for (uint32_t i = 0; i < length; i++) {
339340
std::cout << "0x" << std::setw(2) << std::setfill('0') << std::hex << int(buf[i]) << " ";
340341
}
341-
std::printf("\n");
342+
std::print("\n");
342343
for (uint32_t i = 0; i < length; i++) {
343-
std::cout << (char)buf[i] << " ";
344+
std::print("{} ", (char)buf[i]);
344345
}
345-
std::cout << std::endl;
346+
std::print("\n");
346347
}
347348

348349
void ByteBuffer::printAscii() const {
349350
uint32_t length = buf.size();
350-
std::cout << "ByteBuffer " << name << " Length: " << length << ". ASCII Print" << std::endl;
351+
std::print("ByteBuffer {} Length: {}. ASCII Print\n", name, length);
351352
for (uint32_t i = 0; i < length; i++) {
352-
std::cout << (char)buf[i] << " ";
353+
std::print("{} ", (char)buf[i]);
353354
}
354-
std::cout << std::endl;
355+
std::print("\n");
355356
}
356357

357358
void ByteBuffer::printHex() const {
358359
uint32_t length = buf.size();
359-
std::cout << "ByteBuffer " << name << " Length: " << length << ". Hex Print" << std::endl;
360+
std::print("ByteBuffer {} Length: {}. Hex Print\n", name, length);
360361
for (uint32_t i = 0; i < length; i++) {
361362
std::cout << "0x" << std::setw(2) << std::setfill('0') << std::hex << int(buf[i]) << " ";
362363
}
363-
std::cout << std::endl;
364+
std::print("\n");
364365
}
365366

366367
void ByteBuffer::printPosition() const {
367368
uint32_t length = buf.size();
368-
std::cout << "ByteBuffer " << name << " Length: " << length << " Read Pos: " << rpos << ". Write Pos: "
369-
<< wpos << std::endl;
369+
std::print("ByteBuffer {} Length: {} Read Pos: {}. Write Pos: {}\n", name, length, rpos, wpos);
370370
}
371371

372372
#endif // BB_UTILITY

‎src/ByteBuffer.h‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -159,22 +159,23 @@ class ByteBuffer {
159159
}
160160

161161
template<typename T> void append(T data) {
162-
uint32_t s = sizeof(data);
162+
constexpr size_t s = sizeof(T);
163163

164-
if (size() < (wpos + s))
165-
buf.resize(wpos + s);
164+
if (size() < static_cast<size_t>(wpos) + s)
165+
buf.resize(static_cast<size_t>(wpos) + s);
166166
memcpy(&buf[wpos], (uint8_t*)&data, s);
167167

168168
wpos += s;
169169
}
170170

171171
template<typename T> void insert(T data, uint32_t index) {
172-
if ((index + sizeof(data)) > size()) {
173-
buf.resize(size() + (index + sizeof(data)));
172+
const size_t end = static_cast<size_t>(index) + sizeof(T);
173+
if (end > size()) {
174+
buf.resize(end);
174175
}
175176

176-
memcpy(&buf[index], (uint8_t*)&data, sizeof(data));
177-
wpos = index + sizeof(data);
177+
memcpy(&buf[index], (uint8_t*)&data, sizeof(T));
178+
wpos = index + sizeof(T);
178179
}
179180
};
180181

‎src/HTTPMessage.cpp‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
#include <format>
2424
#include <memory>
2525
#include <print>
26+
#include <ranges>
2627

2728
#include <cctype> // to std::tolower
29+
#include <charconv>
2830

2931

3032
HTTPMessage::HTTPMessage() : ByteBuffer(4096) {
@@ -136,19 +138,19 @@ std::string HTTPMessage::getStrElement(char delim) {
136138
if (startPos > endPos)
137139
return "";
138140

139-
// Calculate the size based on the found ending position
140-
uint32_t size = (endPos + 1) - startPos;
141-
if (size <= 0)
141+
// Token spans [startPos, endPos); delimiter sits at endPos
142+
uint32_t tokenLen = static_cast<uint32_t>(endPos - startPos);
143+
if (tokenLen == 0)
142144
return "";
143145

144-
// Grab the std::string from the ByteBuffer up to the delimiter
145-
auto str = std::make_unique<char[]>(size);
146-
getBytes((uint8_t*)str.get(), size);
147-
str[size - 1] = 0x00; // NULL termination
148-
std::string ret = str.get();
146+
// Grab the token bytes (excludes delimiter), then step past the delimiter
147+
auto str = std::make_unique<char[]>(tokenLen + 1);
148+
getBytes(reinterpret_cast<uint8_t*>(str.get()), tokenLen);
149+
str[tokenLen] = '\0';
150+
std::string ret(str.get(), tokenLen);
149151

150-
// Increment the read position PAST the delimiter
151-
setReadPos(endPos + 1);
152+
// Advance the read position past the delimiter
153+
setReadPos(static_cast<uint32_t>(endPos) + 1);
152154

153155
return ret;
154156
}
@@ -170,7 +172,7 @@ void HTTPMessage::parseHeaders() {
170172
while (hline.size() > 0) {
171173
// Case where values are on multiple lines ending with a comma
172174
app = hline;
173-
while (app[app.size() - 1] == ',') {
175+
while (!app.empty() && app[app.size() - 1] == ',') {
174176
app = getLine();
175177
hline += app;
176178
}
@@ -196,16 +198,26 @@ bool HTTPMessage::parseBody() {
196198
return true;
197199

198200
uint32_t remainingLen = bytesRemaining();
199-
uint32_t contentLen = atoi(hlenstr.c_str());
201+
uint32_t contentLen = 0;
202+
{
203+
auto [ptr, ec] = std::from_chars(hlenstr.data(), hlenstr.data() + hlenstr.size(), contentLen);
204+
if (ec != std::errc{}) {
205+
parseErrorStr = std::format("Invalid Content-Length value: {}", hlenstr);
206+
this->dataLen = 0;
207+
return false;
208+
}
209+
}
200210

201211
// contentLen should NOT exceed the remaining number of bytes in the buffer
202212
// Add 1 to bytesRemaining so it includes the byte at the current read position
203-
if (contentLen > remainingLen + 1) {
213+
if (static_cast<uint64_t>(contentLen) > static_cast<uint64_t>(remainingLen) + 1) {
204214
// If it exceeds, there's a potential security issue and we can't reliably parse
205215
parseErrorStr = std::format("Content-Length ({}) is greater than remaining bytes ({})", hlenstr, remainingLen);
216+
this->dataLen = 0;
206217
return false;
207218
} else if (remainingLen > contentLen) {
208219
parseErrorStr = std::format("ByteBuffer remaining size to read ({}) is greater than provided Content-Length {}", remainingLen, contentLen);
220+
this->dataLen = 0;
209221
return false;
210222
} else if (contentLen == 0) {
211223
// Nothing to read, which is fine
@@ -215,15 +227,9 @@ bool HTTPMessage::parseBody() {
215227
this->dataLen = contentLen;
216228
}
217229

218-
// Create a big enough buffer to store the data
219-
this->data = new uint8_t[this->dataLen];
220-
221-
// Grab all the bytes from the current position to the end
222-
uint32_t dIdx = 0;
223-
for (uint32_t i = getReadPos(); i < remainingLen; i++) {
224-
this->data[dIdx] = get(i);
225-
dIdx++;
226-
}
230+
// Create a big enough buffer to store the data and read from the current position
231+
this->data = std::make_unique<uint8_t[]>(this->dataLen);
232+
getBytes(this->data.get(), this->dataLen);
227233

228234
// We could handle chunked Request/Response parsing (with footers) here, but, we won't.
229235

@@ -262,7 +268,7 @@ void HTTPMessage::addHeader(std::string const& line) {
262268

263269
// Skip all leading spaces in the value
264270
int32_t i = 0;
265-
while (i < value.size() && value.at(i) == 0x20) {
271+
while (i < static_cast<int32_t>(value.size()) && value[i] == 0x20) {
266272
i++;
267273
}
268274
value = value.substr(i, value.size());
@@ -308,9 +314,9 @@ std::string HTTPMessage::getHeaderValue(std::string const& key) const {
308314
// Key wasn't found, try an all lowercase variant as some clients won't always use proper capitalization
309315
if (it == headers.end()) {
310316

311-
auto key_lower = std::string(key);
312-
std::ranges::transform(key_lower.begin(), key_lower.end(), key_lower.begin(),
313-
[](unsigned char c){ return std::tolower(c); });
317+
auto key_lower = key
318+
| std::views::transform([](unsigned char c){ return std::tolower(c); })
319+
| std::ranges::to<std::string>();
314320

315321
// Still not found, return empty string to indicate the Header value doesnt exist
316322
it = headers.find(key_lower);
@@ -360,4 +366,3 @@ uint32_t HTTPMessage::getNumHeaders() const {
360366
void HTTPMessage::clearHeaders() {
361367
headers.clear();
362368
}
363-

‎src/HTTPMessage.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#define _HTTPMESSAGE_H_
2121

2222
#include <array>
23+
#include <cstring>
2324
#include <map>
2425
#include <memory>
2526
#include <string>
@@ -90,7 +91,7 @@ class HTTPMessage : public ByteBuffer {
9091
std::string version = DEFAULT_HTTP_VERSION; // By default, all create() will indicate the version is whatever DEFAULT_HTTP_VERSION is
9192

9293
// Message Body Data (Resource in the case of a response, extra parameters in the case of a request)
93-
uint8_t* data = nullptr;
94+
std::unique_ptr<uint8_t[]> data;
9495
uint32_t dataLen = 0;
9596

9697
public:
@@ -135,13 +136,14 @@ class HTTPMessage : public ByteBuffer {
135136
return version;
136137
}
137138

138-
void setData(uint8_t* d, uint32_t len) {
139-
data = d;
139+
void setData(const uint8_t* d, uint32_t len) {
140+
data = std::make_unique<uint8_t[]>(len);
141+
std::memcpy(data.get(), d, len);
140142
dataLen = len;
141143
}
142144

143145
uint8_t* getData() {
144-
return data;
146+
return data.get();
145147
}
146148

147149
uint32_t getDataLength() const {

‎src/HTTPRequest.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ std::unique_ptr<uint8_t[]> HTTPRequest::create() {
9393
putHeaders();
9494

9595
// If theres body data, add it now
96-
if ((this->data != nullptr) && this->dataLen > 0) {
97-
putBytes(this->data, this->dataLen);
96+
if (this->data && this->dataLen > 0) {
97+
putBytes(this->data.get(), this->dataLen);
9898
}
9999

100100
// Allocate space for the returned byte array and return it

‎src/HTTPResponse.cpp‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "HTTPMessage.h"
2020
#include "HTTPResponse.h"
2121

22+
#include <charconv>
2223
#include <format>
2324
#include <string>
2425
#include <memory>
@@ -101,8 +102,8 @@ std::unique_ptr<uint8_t[]> HTTPResponse::create() {
101102
putHeaders();
102103

103104
// If theres body data, add it now
104-
if ((this->data != nullptr) && this->dataLen > 0) {
105-
putBytes(this->data, this->dataLen);
105+
if (this->data && this->dataLen > 0) {
106+
putBytes(this->data.get(), this->dataLen);
106107
}
107108

108109
// Allocate space for the returned byte array and return it
@@ -125,9 +126,19 @@ bool HTTPResponse::parse() {
125126
// Get elements from the status line: <version> <status code> <reason>\r\n
126127
version = getStrElement();
127128
statusstr = getStrElement();
128-
determineStatusCode();
129129
reason = getLine(); // Pull till \r\n termination
130130

131+
// Parse the status code integer directly; fall back to reason-string matching if malformed
132+
{
133+
int32_t code = 0;
134+
auto [ptr, ec] = std::from_chars(statusstr.data(), statusstr.data() + statusstr.size(), code);
135+
if (ec == std::errc{}) {
136+
status = code;
137+
} else {
138+
determineStatusCode();
139+
}
140+
}
141+
131142
// Optional - Validate the HTTP version. If there is a mismatch, discontinue parsing
132143
// if (strcmp(version.c_str(), HTTP_VERSION) != 0) {
133144
// parseErrorStr = "Supported HTTP version does not match";

0 commit comments

Comments
 (0)