Skip to content

Add callback handler to report internal HTTP parsing errors (431, 400, 505) #1891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pdeltour-syna wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add callback handler to report internal HTTP parsing errors (431, 400, 505) #1891

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e9b8c43
Add callback handler to report internal HTTP parsing errors(431, 400,…
pdeltour-syna Oct 2, 2025
44cdf05
renamed callack to log
pdeltour-syna Oct 7, 2025
1a1b8a3
performance - remove switch and check fro callback
pdeltour-syna Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/App.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,13 @@ struct TemplatedApp {
return std::move(static_cast<TemplatedApp &&>(*this));
}

/* Attaches an error handler for internal HTTP parsing errors */
TemplatedApp &&onHttpParsingError(MoveOnlyFunction<void(HttpRequest *, int, std::string_view)> &&errorHandler) {
Copy link
Copy Markdown
Contributor

@uNetworkingAB uNetworkingAB Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly this one needs a better name

Copy link
Copy Markdown
Contributor

@uNetworkingAB uNetworkingAB Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error is a bad name. People will register it and think they need to handle it in some way. That's common for other libs. It sucks.

Log or logging could be used like App::log that simply gives you information of happenings that you don't need to handle.

Copy link
Copy Markdown
Contributor

@uNetworkingAB uNetworkingAB Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

App::log could get things like

  • HTTP error being returned to client
  • Connection was opened
  • Connection was closed

Neither of these events need handling, they just need logging, so App::log makes more sense than anything containing "error".

One of the main benefits of the uWS interface is that error handling and regular close is the same handler, meaning the same business flow. This is different from other libs that have open/error, open/close as two different flows of logic. That sucks, and it makes the app more complex than it has to be.

Copy link
Copy Markdown
Author

@pdeltour-syna pdeltour-syna Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially looked at using App:filter, but it seemed more like a socket level notification. Also the info I would like to collect on unhandled http requests is :

  • request info (method / URL / queries / headers)
  • status code
  • nice to have - response body (detail of the status)

I agree that there could be a better naming for the callback handler. e.g. App::log (or App::logEvent ?)

I don't mind combining HTTP error / connection open / connection close into a single callback, where there could be even more events in the future (but maybe that would open the door for overpopulating this with events). On the other hand the information returned for connection open/close and HTTP error is completely different.
How could the function signature look like then ?
App::log(int eventType, HttpRequest *req, int statusCode, string_view responseBody)
where eventType:

  • -1 : means 'Connection closed' (req == null / statusCode N/A / responseBody empty)
  • 0 : means 'Unhandled http request' (i.e. request was not valid and produced internal error) with valid Req / statusCode and responseBody
  • 1 : means 'Connection opened' (req == null / statusCode N/A / responseBody empty)

Alternatively when not combining the connection open/close with the unhandled request, the handler could be called something like : App::unhandledReq

httpContext->onHttpParsingError(std::move(errorHandler));

return std::move(static_cast<TemplatedApp &&>(*this));
}

/* Same as publish, but takes a prepared message */
bool publishPrepared(std::string_view topic, PreparedMessage &preparedMessage) {
/* It is assumed by heuristics that a prepared message ought to be big,
Expand Down
25 changes: 25 additions & 0 deletions src/HttpContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "HttpResponseData.h"
#include "AsyncSocket.h"
#include "WebSocketData.h"
#include "HttpErrors.h"

#include <string_view>
#include <iostream>
Expand Down Expand Up @@ -251,6 +252,25 @@ struct HttpContext {
}
}
return user;
}, [httpContextData](/*void *s, */HttpRequest *httpRequest, unsigned int errorCode) -> void {
/* Call the high-level error handler if one is registered */
if (httpContextData->httpParsingErrorHandler) {
/* Map internal error codes to HTTP status codes and response bodies */
int statusCode;
switch (errorCode) {
case HTTP_ERROR_505_HTTP_VERSION_NOT_SUPPORTED:
statusCode = 505;
break;
case HTTP_ERROR_431_REQUEST_HEADER_FIELDS_TOO_LARGE:
statusCode = 431;
break;
case HTTP_ERROR_400_BAD_REQUEST:
default:
statusCode = 400;
break;
}
httpContextData->httpParsingErrorHandler(httpRequest, statusCode, httpErrorResponses[errorCode]);
}
});

/* Mark that we are no longer parsing Http */
Expand Down Expand Up @@ -421,6 +441,11 @@ struct HttpContext {
getSocketContextData()->filterHandlers.emplace_back(std::move(filterHandler));
}

/* Register an HTTP parsing error handler */
void onHttpParsingError(MoveOnlyFunction<void(HttpRequest *, int, std::string_view)> &&errorHandler) {
getSocketContextData()->httpParsingErrorHandler = std::move(errorHandler);
}

/* Register an HTTP route handler acording to URL pattern */
void onHttp(std::string method, std::string pattern, MoveOnlyFunction<void(HttpResponse<SSL> *, HttpRequest *)> &&handler, bool upgrade = false) {
HttpContextData<SSL> *httpContextData = getSocketContextData();
Expand Down
3 changes: 3 additions & 0 deletions src/HttpContextData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ struct alignas(16) HttpContextData {
std::vector<MoveOnlyFunction<void(HttpResponse<SSL> *, int)>> filterHandlers;

MoveOnlyFunction<void(const char *hostname)> missingServerNameHandler;

/* HTTP parsing error handler */
MoveOnlyFunction<void(HttpRequest *, int, std::string_view)> httpParsingErrorHandler;

struct RouterData {
HttpResponse<SSL> *httpResponse;
Expand Down
27 changes: 22 additions & 5 deletions src/HttpParser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,9 @@ struct HttpParser {
data += consumed;
length -= consumed;
consumedTotal += consumed;
/* Parse query */
const char *querySeparatorPtr = (const char *) memchr(req->headers->value.data(), '?', req->headers->value.length());
req->querySeparator = (unsigned int) ((querySeparatorPtr ? querySeparatorPtr : req->headers->value.data() + req->headers->value.length()) - req->headers->value.data());
Copy link
Copy Markdown
Author

@pdeltour-syna pdeltour-syna Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this code, because HttpRequest::getUrl() requires querySeparator to be set, the below return on error will call the new callback handler and this allows to get the URL of the failed request for these cases


/* Even if we could parse it, check for length here as well */
if (consumed > MAX_FALLBACK_SIZE) {
Expand Down Expand Up @@ -529,10 +532,6 @@ struct HttpParser {
return {HTTP_ERROR_400_BAD_REQUEST, FULLPTR};
}

/* Parse query */
const char *querySeparatorPtr = (const char *) memchr(req->headers->value.data(), '?', req->headers->value.length());
req->querySeparator = (unsigned int) ((querySeparatorPtr ? querySeparatorPtr : req->headers->value.data() + req->headers->value.length()) - req->headers->value.data());

/* If returned socket is not what we put in we need
* to break here as we either have upgraded to
* WebSockets or otherwise closed the socket. */
Expand Down Expand Up @@ -615,7 +614,7 @@ struct HttpParser {
}

public:
std::pair<unsigned int, void *> consumePostPadded(char *data, unsigned int length, void *user, void *reserved, MoveOnlyFunction<void *(void *, HttpRequest *)> &&requestHandler, MoveOnlyFunction<void *(void *, std::string_view, bool)> &&dataHandler) {
std::pair<unsigned int, void *> consumePostPadded(char *data, unsigned int length, void *user, void *reserved, MoveOnlyFunction<void *(void *, HttpRequest *)> &&requestHandler, MoveOnlyFunction<void *(void *, std::string_view, bool)> &&dataHandler, MoveOnlyFunction<void(HttpRequest *, unsigned int)> &&errorHandler = nullptr) {

/* This resets BloomFilter by construction, but later we also reset it again.
* Optimize this to skip resetting twice (req could be made global) */
Expand All @@ -630,6 +629,9 @@ struct HttpParser {
dataHandler(user, chunk, chunk.length() == 0);
}
if (isParsingInvalidChunkedEncoding(remainingStreamingBytes)) {
if (errorHandler) {
errorHandler(&req, HTTP_ERROR_400_BAD_REQUEST);
}
return {HTTP_ERROR_400_BAD_REQUEST, FULLPTR};
}
data = (char *) dataToConsume.data();
Expand Down Expand Up @@ -667,6 +669,9 @@ struct HttpParser {
// break here on break
std::pair<unsigned int, void *> consumed = fenceAndConsumePostPadded<true>(fallback.data(), (unsigned int) fallback.length(), user, reserved, &req, requestHandler, dataHandler);
if (consumed.second != user) {
if (errorHandler) {
errorHandler(&req, consumed.first);
}
return consumed;
}

Expand All @@ -687,6 +692,9 @@ struct HttpParser {
dataHandler(user, chunk, chunk.length() == 0);
}
if (isParsingInvalidChunkedEncoding(remainingStreamingBytes)) {
if (errorHandler) {
errorHandler(&req, HTTP_ERROR_400_BAD_REQUEST);
}
return {HTTP_ERROR_400_BAD_REQUEST, FULLPTR};
}
data = (char *) dataToConsume.data();
Expand Down Expand Up @@ -714,6 +722,9 @@ struct HttpParser {

} else {
if (fallback.length() == MAX_FALLBACK_SIZE) {
if (errorHandler) {
errorHandler(&req, HTTP_ERROR_431_REQUEST_HEADER_FIELDS_TOO_LARGE);
}
return {HTTP_ERROR_431_REQUEST_HEADER_FIELDS_TOO_LARGE, FULLPTR};
}
return {0, user};
Expand All @@ -722,6 +733,9 @@ struct HttpParser {

std::pair<unsigned int, void *> consumed = fenceAndConsumePostPadded<false>(data, length, user, reserved, &req, requestHandler, dataHandler);
if (consumed.second != user) {
if (errorHandler) {
errorHandler(&req, consumed.first);
}
return consumed;
}

Expand All @@ -732,6 +746,9 @@ struct HttpParser {
if (length < MAX_FALLBACK_SIZE) {
fallback.append(data, length);
} else {
if (errorHandler) {
errorHandler(&req, HTTP_ERROR_431_REQUEST_HEADER_FIELDS_TOO_LARGE);
}
return {HTTP_ERROR_431_REQUEST_HEADER_FIELDS_TOO_LARGE, FULLPTR};
}
}
Expand Down