Skip to content

Commit 71988af

Browse files
VolkerChristianVolkerChristian
committed
Increase robustness suggested by ChatGPT
1 parent be88ddf commit 71988af

File tree

3 files changed

+112
-59
lines changed

3 files changed

+112
-59
lines changed

‎src/web/http/Parser.cpp‎

Lines changed: 94 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,47 @@
5050

5151
#include "web/http/http_utils.h"
5252

53+
#include <charconv>
54+
#include <limits>
55+
#include <system_error>
5356
#include <tuple>
5457
#include <utility>
5558

5659
#endif /* DOXYGEN_SHOULD_SKIP_THIS */
5760

5861
namespace web::http {
5962

63+
static bool parseContentLengthStrict(const std::string& s, std::size_t& out) {
64+
bool success = false;
65+
66+
unsigned long long value = 0;
67+
68+
if (!s.empty()) {
69+
const char* first = s.data();
70+
const char* last = s.data() + s.size();
71+
72+
const auto [ptr, ec] = std::from_chars(first, last, value, 10);
73+
74+
if (ec == std::errc{} && ptr == last && value <= static_cast<unsigned long long>(std::numeric_limits<std::size_t>::max())) {
75+
out = static_cast<std::size_t>(value);
76+
success = true;
77+
}
78+
}
79+
80+
return success;
81+
}
82+
83+
static bool transferEncodingHasChunked(CiStringMap<std::string>& headers) {
84+
bool hasChunked = false;
85+
86+
if (headers.contains("Transfer-Encoding")) {
87+
const std::string& encoding = headers["Transfer-Encoding"];
88+
hasChunked = web::http::ciContains(encoding, "chunked");
89+
}
90+
91+
return hasChunked;
92+
}
93+
6094
// HTTP/1.0 and HTTP/1.1
6195
const std::regex Parser::httpVersionRegex("^HTTP/([1])[.]([0-1])$");
6296

@@ -81,7 +115,7 @@ namespace web::http {
81115
contentLength = 0;
82116
contentLengthRead = 0;
83117

84-
for (ContentDecoder* contentDecoder : decoderQueue) {
118+
for (const ContentDecoder* contentDecoder : decoderQueue) {
85119
delete contentDecoder;
86120
}
87121
decoderQueue.clear();
@@ -158,58 +192,75 @@ namespace web::http {
158192
}
159193

160194
void Parser::analyzeHeader() {
161-
if (headers.contains("Content-Length")) {
162-
contentLength = std::stoul(headers["Content-Length"]);
163-
decoderQueue.emplace_back(new web::http::decoder::Identity(socketContext, contentLength));
164-
}
165-
if (headers.contains("Transfer-Encoding")) {
166-
const std::string& encoding = headers["Transfer-Encoding"];
195+
bool success = true;
167196

168-
if (web::http::ciContains(encoding, "chunked")) {
169-
transferEncoding = TransferEncoding::Chunked;
170-
decoderQueue.emplace_back(new web::http::decoder::Chunked(socketContext));
197+
// Determine message framing.
198+
// RFC 9112 §6.3: Transfer-Encoding (chunked) overrides Content-Length.
199+
const bool hasChunked = transferEncodingHasChunked(headers);
171200

172-
if (headers.contains("Trailer")) {
173-
std::string trailers = headers["Trailer"];
201+
if (hasChunked) {
202+
transferEncoding = TransferEncoding::Chunked;
203+
decoderQueue.emplace_back(new web::http::decoder::Chunked(socketContext));
174204

175-
while (!trailers.empty()) {
176-
std::string trailerField;
177-
std::tie(trailerField, trailers) = httputils::str_split(trailers, ',');
178-
httputils::str_trimm(trailerField);
179-
trailerFieldsExpected.insert(trailerField);
180-
trailerField.clear();
181-
}
182-
trailerDecoder.setFieldsExpected(trailerFieldsExpected);
205+
if (headers.contains("Trailer")) {
206+
std::string trailers = headers["Trailer"];
207+
208+
while (!trailers.empty()) {
209+
std::string trailerField;
210+
std::tie(trailerField, trailers) = httputils::str_split(trailers, ',');
211+
httputils::str_trimm(trailerField);
212+
trailerFieldsExpected.insert(trailerField);
213+
trailerField.clear();
183214
}
215+
trailerDecoder.setFieldsExpected(trailerFieldsExpected);
184216
}
185-
if (web::http::ciContains(encoding, "compressed")) {
186-
// decoderQueue.emplace_back(new web::http::decoder::Compress(socketContext));
187-
}
188-
if (web::http::ciContains(encoding, "deflate")) {
189-
// decoderQueue.emplace_back(new web::http::decoder::Deflate(socketContext));
190-
}
191-
if (web::http::ciContains(encoding, "gzip")) {
192-
// decoderQueue.emplace_back(new web::http::decoder::GZip(socketContext));
217+
} else if (headers.contains("Content-Length")) {
218+
std::size_t length = 0;
219+
220+
if (!parseContentLengthStrict(headers["Content-Length"], length)) {
221+
parseError(400, "Invalid Content-Length");
222+
success = false;
223+
} else {
224+
contentLength = length;
225+
decoderQueue.emplace_back(new web::http::decoder::Identity(socketContext, contentLength));
193226
}
194227
}
195-
if (decoderQueue.empty()) {
196-
decoderQueue.emplace_back(new web::http::decoder::HTTP10Response(socketContext));
197-
}
198-
199-
if (headers.contains("Content-Encoding")) {
200-
const std::string& encoding = headers["Content-Encoding"];
201228

202-
if (web::http::ciContains(encoding, "compressed")) {
203-
// decoderQueue.emplace_back(new web::http::decoder::Compress(socketContext));
204-
}
205-
if (web::http::ciContains(encoding, "deflate")) {
206-
// decoderQueue.emplace_back(new web::http::decoder::Deflate(socketContext));
229+
if (success) {
230+
// Transfer-Encoding (other than chunked) is currently not implemented, but we keep the
231+
// existing behavior of not altering the decoder queue here.
232+
if (headers.contains("Transfer-Encoding")) {
233+
const std::string& encoding = headers["Transfer-Encoding"];
234+
if (web::http::ciContains(encoding, "compressed")) {
235+
// decoderQueue.emplace_back(new web::http::decoder::Compress(socketContext));
236+
}
237+
if (web::http::ciContains(encoding, "deflate")) {
238+
// decoderQueue.emplace_back(new web::http::decoder::Deflate(socketContext));
239+
}
240+
if (web::http::ciContains(encoding, "gzip")) {
241+
// decoderQueue.emplace_back(new web::http::decoder::GZip(socketContext));
242+
}
207243
}
208-
if (web::http::ciContains(encoding, "gzip")) {
209-
// decoderQueue.emplace_back(new web::http::decoder::GZip(socketContext));
244+
245+
if (decoderQueue.empty()) {
246+
decoderQueue.emplace_back(new web::http::decoder::HTTP10Response(socketContext));
210247
}
211-
if (web::http::ciContains(encoding, "br")) {
212-
// decoderQueue.emplace_back(new web::http::decoder::Br(socketContext));
248+
249+
if (headers.contains("Content-Encoding")) {
250+
const std::string& encoding = headers["Content-Encoding"];
251+
252+
if (web::http::ciContains(encoding, "compressed")) {
253+
// decoderQueue.emplace_back(new web::http::decoder::Compress(socketContext));
254+
}
255+
if (web::http::ciContains(encoding, "deflate")) {
256+
// decoderQueue.emplace_back(new web::http::decoder::Deflate(socketContext));
257+
}
258+
if (web::http::ciContains(encoding, "gzip")) {
259+
// decoderQueue.emplace_back(new web::http::decoder::GZip(socketContext));
260+
}
261+
if (web::http::ciContains(encoding, "br")) {
262+
// decoderQueue.emplace_back(new web::http::decoder::Br(socketContext));
263+
}
213264
}
214265
}
215266
}

‎src/web/websocket/SocketContextUpgrade.hpp‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@
4646

4747
#include "utils/hexdump.h"
4848

49+
#include <cstring>
50+
#include <vector>
51+
4952
#endif /* DOXYGEN_SHOULD_SKIP_THIS */
5053

5154
namespace web::websocket {
@@ -93,18 +96,18 @@ namespace web::websocket {
9396
template <typename SubProtocol, typename Request, typename Response>
9497
void
9598
SocketContextUpgrade<SubProtocol, Request, Response>::sendClose(uint16_t statusCode, const char* reason, std::size_t reasonLength) {
96-
std::size_t closePayloadLength = reasonLength + 2;
97-
char* closePayload = new char[closePayloadLength];
99+
const std::size_t closePayloadLength = reasonLength + 2;
100+
std::vector<char> closePayload(closePayloadLength);
98101

99-
*reinterpret_cast<uint16_t*>(closePayload) = htobe16(statusCode); // cppcheck-suppress uninitdata
102+
// Avoid unaligned stores.
103+
const uint16_t beStatus = htobe16(statusCode);
104+
std::memcpy(closePayload.data(), &beStatus, sizeof(beStatus));
100105

101106
if (reasonLength > 0) {
102-
memcpy(closePayload + 2, reason, reasonLength);
107+
std::memcpy(closePayload.data() + 2, reason, reasonLength);
103108
}
104109

105-
sendClose(closePayload, closePayloadLength);
106-
107-
delete[] closePayload;
110+
sendClose(closePayload.data(), closePayloadLength);
108111
}
109112

110113
template <typename SubProtocol, typename Request, typename Response>

‎src/web/websocket/Transmitter.cpp‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848

4949
#include <endian.h>
5050
#include <string>
51+
#include <vector>
5152

5253
#endif /* DOXYGEN_SHOULD_SKIP_THIS */
5354

@@ -144,17 +145,15 @@ namespace web::websocket {
144145
if (masking) {
145146
sendFrameData(maskingKeyAsArray.keyAsBytes, 4);
146147

148+
// Never mutate the caller-provided payload buffer.
149+
std::vector<char> maskedPayload(payload, payload + payloadLength);
147150
for (uint64_t i = 0; i < payloadLength; i++) {
148-
*(const_cast<char*>(payload) + i) = static_cast<char>(*(payload + i) ^ *(maskingKeyAsArray.keyAsBytes + i % 4));
149-
}
150-
}
151-
152-
sendFrameData(payload, payloadLength);
153-
154-
if (masking) {
155-
for (uint64_t i = 0; i < payloadLength; i++) {
156-
*(const_cast<char*>(payload) + i) = static_cast<char>(*(payload + i) ^ *(maskingKeyAsArray.keyAsBytes + i % 4));
151+
maskedPayload[static_cast<std::size_t>(i)] =
152+
static_cast<char>(maskedPayload[static_cast<std::size_t>(i)] ^ maskingKeyAsArray.keyAsBytes[i % 4]);
157153
}
154+
sendFrameData(maskedPayload.data(), payloadLength);
155+
} else {
156+
sendFrameData(payload, payloadLength);
158157
}
159158
}
160159

0 commit comments

Comments
 (0)