From ad0a9614307f0b3f4def859d560010b4b178f360 Mon Sep 17 00:00:00 2001 From: Alex Hultman Date: Mon, 30 Nov 2020 22:47:34 +0100 Subject: [PATCH] Experimental: Fix all signed/unsigned warnings --- src/App.h | 6 +++--- src/AsyncSocket.h | 30 +++++++++++++++++------------- src/HttpContext.h | 2 +- src/HttpParser.h | 38 +++++++++++++++++++------------------- src/HttpResponse.h | 4 ++-- src/HttpRouter.h | 4 ++-- src/LoopData.h | 4 ++-- src/QueryParser.h | 2 +- src/WebSocket.h | 4 ++-- src/WebSocketContext.h | 4 ++-- src/WebSocketContextData.h | 2 +- src/WebSocketData.h | 2 +- src/WebSocketExtensions.h | 6 +++--- src/WebSocketHandshake.h | 2 +- src/WebSocketProtocol.h | 2 +- 15 files changed, 58 insertions(+), 54 deletions(-) diff --git a/src/App.h b/src/App.h index 4a586c8..4493f58 100644 --- a/src/App.h +++ b/src/App.h @@ -117,9 +117,9 @@ public: struct WebSocketBehavior { CompressOptions compression = DISABLED; - int maxPayloadLength = 16 * 1024; - int idleTimeout = 120; - int maxBackpressure = 1 * 1024 * 1024; + unsigned int maxPayloadLength = 16 * 1024; + unsigned int idleTimeout = 120; + unsigned int maxBackpressure = 1 * 1024 * 1024; fu2::unique_function *, HttpRequest *, struct us_socket_context_t *)> upgrade = nullptr; fu2::unique_function *)> open = nullptr; fu2::unique_function *, std::string_view, uWS::OpCode)> message = nullptr; diff --git a/src/AsyncSocket.h b/src/AsyncSocket.h index 5052deb..427dc51 100644 --- a/src/AsyncSocket.h +++ b/src/AsyncSocket.h @@ -20,6 +20,10 @@ /* This class implements async socket memory management strategies */ +/* NOTE: Many unsigned/signed conversion warnings could be solved by moving from int length + * to unsigned length for everything to/from uSockets - this would however remove the opportunity + * to signal error with -1 (which is how the entire UNIX syscalling is built). */ + #include "LoopData.h" #include "AsyncSocketData.h" @@ -87,7 +91,7 @@ protected: LoopData *loopData = getLoopData(); if (loopData->corkedSocket == this && loopData->corkOffset + size < LoopData::CORK_BUFFER_SIZE) { char *sendBuffer = loopData->corkBuffer + loopData->corkOffset; - loopData->corkOffset += (int) size; + loopData->corkOffset += (unsigned int) size; return {sendBuffer, false}; } else { /* Slow path for now, we want to always be corked if possible */ @@ -127,7 +131,7 @@ protected: static thread_local char buf[16]; int ipLength = 16; us_socket_remote_address(SSL, (us_socket_t *) this, buf, &ipLength); - return std::string_view(buf, ipLength); + return std::string_view(buf, (unsigned int) ipLength); } /* Returns the text representation of IP */ @@ -156,14 +160,14 @@ protected: if ((unsigned int) written < asyncSocketData->buffer.length()) { /* Update buffering (todo: we can do better here if we keep track of what happens to this guy later on) */ - asyncSocketData->buffer = asyncSocketData->buffer.substr(written); + asyncSocketData->buffer = asyncSocketData->buffer.substr((size_t) written); if (optionally) { /* Thankfully we can exit early here */ return {0, true}; } else { /* This path is horrible and points towards erroneous usage */ - asyncSocketData->buffer.append(src, length); + asyncSocketData->buffer.append(src, (unsigned int) length); return {length, true}; } @@ -176,21 +180,21 @@ protected: if (length) { if (loopData->corkedSocket == this) { /* We are corked */ - if (LoopData::CORK_BUFFER_SIZE - loopData->corkOffset >= length) { + if (LoopData::CORK_BUFFER_SIZE - loopData->corkOffset >= (unsigned int) length) { /* If the entire chunk fits in cork buffer */ - memcpy(loopData->corkBuffer + loopData->corkOffset, src, length); - loopData->corkOffset += length; + memcpy(loopData->corkBuffer + loopData->corkOffset, src, (unsigned int) length); + loopData->corkOffset += (unsigned int) length; /* Fall through to default return */ } else { /* Strategy differences between SSL and non-SSL regarding syscall minimizing */ if constexpr (SSL) { /* Cork up as much as we can */ - int stripped = LoopData::CORK_BUFFER_SIZE - loopData->corkOffset; + unsigned int stripped = LoopData::CORK_BUFFER_SIZE - loopData->corkOffset; memcpy(loopData->corkBuffer + loopData->corkOffset, src, stripped); loopData->corkOffset = LoopData::CORK_BUFFER_SIZE; - auto [written, failed] = uncork(src + stripped, length - stripped, optionally); - return {written + stripped, failed}; + auto [written, failed] = uncork(src + stripped, length - (int) stripped, optionally); + return {written + (int) stripped, failed}; } /* For non-SSL we take the penalty of two syscalls */ @@ -210,11 +214,11 @@ protected: /* Fall back to worst possible case (should be very rare for HTTP) */ /* At least we can reserve room for next chunk if we know it up front */ if (nextLength) { - asyncSocketData->buffer.reserve(asyncSocketData->buffer.length() + length - written + nextLength); + asyncSocketData->buffer.reserve(asyncSocketData->buffer.length() + (size_t) (length - written + nextLength)); } /* Buffer this chunk */ - asyncSocketData->buffer.append(src + written, length - written); + asyncSocketData->buffer.append(src + written, (size_t) (length - written)); /* Return the failure */ return {length, true}; @@ -237,7 +241,7 @@ protected: if (loopData->corkOffset) { /* Corked data is already accounted for via its write call */ - auto [written, failed] = write(loopData->corkBuffer, loopData->corkOffset, false, length); + auto [written, failed] = write(loopData->corkBuffer, (int) loopData->corkOffset, false, length); loopData->corkOffset = 0; if (failed) { diff --git a/src/HttpContext.h b/src/HttpContext.h index 07c6921..28f4847 100644 --- a/src/HttpContext.h +++ b/src/HttpContext.h @@ -133,7 +133,7 @@ private: #endif /* The return value is entirely up to us to interpret. The HttpParser only care for whether the returned value is DIFFERENT or not from passed user */ - void *returnedSocket = httpResponseData->consumePostPadded(data, length, s, proxyParser, [httpContextData](void *s, uWS::HttpRequest *httpRequest) -> void * { + void *returnedSocket = httpResponseData->consumePostPadded(data, (unsigned int) length, s, proxyParser, [httpContextData](void *s, uWS::HttpRequest *httpRequest) -> void * { /* For every request we reset the timeout and hang until user makes action */ /* Warning: if we are in shutdown state, resetting the timer is a security issue! */ us_socket_timeout(SSL, (us_socket_t *) s, 0); diff --git a/src/HttpParser.h b/src/HttpParser.h index 022e65a..4f2abc0 100644 --- a/src/HttpParser.h +++ b/src/HttpParser.h @@ -45,7 +45,7 @@ private: struct Header { std::string_view key, value; } headers[MAX_HEADERS]; - int querySeparator; + unsigned int querySeparator; bool didYield; BloomFilter bf; std::pair currentParameters; @@ -111,7 +111,7 @@ public: /* Returns the raw querystring as a whole, still encoded */ std::string_view getQuery() { - if (querySeparator < (int) headers->value.length()) { + if (querySeparator < headers->value.length()) { /* Strip the initial ? */ return std::string_view(headers->value.data() + querySeparator + 1, headers->value.length() - querySeparator - 1); } else { @@ -151,8 +151,8 @@ private: static unsigned int toUnsignedInteger(std::string_view str) { unsigned int unsignedIntegerValue = 0; - for (unsigned char c : str) { - unsignedIntegerValue = unsignedIntegerValue * 10 + (c - '0'); + for (char c : str) { + unsignedIntegerValue = unsignedIntegerValue * 10 + ((unsigned char) c - '0'); } return unsignedIntegerValue; } @@ -173,7 +173,7 @@ private: headers->key = std::string_view(preliminaryKey, (size_t) (postPaddedBuffer - preliminaryKey)); for (postPaddedBuffer++; (*postPaddedBuffer == ':' || *postPaddedBuffer < 33) && *postPaddedBuffer != '\r'; postPaddedBuffer++); preliminaryValue = postPaddedBuffer; - postPaddedBuffer = (char *) memchr(postPaddedBuffer, '\r', end - postPaddedBuffer); + postPaddedBuffer = (char *) memchr(postPaddedBuffer, '\r', (size_t) (end - postPaddedBuffer)); if (postPaddedBuffer && postPaddedBuffer[1] == '\n') { headers->value = std::string_view(preliminaryValue, (size_t) (postPaddedBuffer - preliminaryValue)); postPaddedBuffer += 2; @@ -188,17 +188,17 @@ private: // the only caller of getHeaders template - std::pair fenceAndConsumePostPadded(char *data, int length, void *user, void *reserved, HttpRequest *req, fu2::unique_function &requestHandler, fu2::unique_function &dataHandler) { + std::pair fenceAndConsumePostPadded(char *data, unsigned int length, void *user, void *reserved, HttpRequest *req, fu2::unique_function &requestHandler, fu2::unique_function &dataHandler) { /* How much data we CONSUMED (to throw away) */ - int consumedTotal = 0; + unsigned int consumedTotal = 0; #ifdef UWS_WITH_PROXY /* ProxyParser is passed as reserved parameter */ ProxyParser *pp = (ProxyParser *) reserved; /* Parse PROXY protocol */ - auto [done, offset] = pp->parse({data, (unsigned int) length}); + auto [done, offset] = pp->parse({data, /*(unsigned int)*/ length}); if (!done) { return {0, user}; } else { @@ -212,13 +212,13 @@ private: /* Fence one byte past end of our buffer (buffer has post padded margins) */ data[length] = '\r'; - for (int consumed; length && (consumed = getHeaders(data, data + length, req->headers, &req->bf)); ) { + for (unsigned int consumed; length && (consumed = getHeaders(data, data + length, req->headers, &req->bf)); ) { data += consumed; length -= consumed; consumedTotal += consumed; /* Strip away tail of first "header value" aka URL */ - req->headers->value = std::string_view(req->headers->value.data(), std::max(0, (int) req->headers->value.length() - 9)); + req->headers->value = std::string_view(req->headers->value.data(), std::max(0, req->headers->value.length() - 9)); /* Add all headers to bloom filter */ req->bf.reset(); @@ -228,7 +228,7 @@ private: /* Parse query */ const char *querySeparatorPtr = (const char *) memchr(req->headers->value.data(), '?', req->headers->value.length()); - req->querySeparator = (int) ((querySeparatorPtr ? querySeparatorPtr : req->headers->value.data() + req->headers->value.length()) - req->headers->value.data()); + 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 @@ -267,7 +267,7 @@ private: } public: - void *consumePostPadded(char *data, int length, void *user, void *reserved, fu2::unique_function &&requestHandler, fu2::unique_function &&dataHandler, fu2::unique_function &&errorHandler) { + void *consumePostPadded(char *data, unsigned int length, void *user, void *reserved, fu2::unique_function &&requestHandler, fu2::unique_function &&dataHandler, fu2::unique_function &&errorHandler) { /* This resets BloomFilter by construction, but later we also reset it again. * Optimize this to skip resetting twice (req could be made global) */ @@ -277,7 +277,7 @@ public: // this is exactly the same as below! // todo: refactor this - if (remainingStreamingBytes >= (unsigned int) length) { + if (remainingStreamingBytes >= length) { void *returnedUser = dataHandler(user, std::string_view(data, length), remainingStreamingBytes == (unsigned int) length); remainingStreamingBytes -= length; return returnedUser; @@ -297,14 +297,14 @@ public: } else if (fallback.length()) { int had = (int) fallback.length(); - int maxCopyDistance = (int) std::min(MAX_FALLBACK_SIZE - fallback.length(), (size_t) length); + size_t maxCopyDistance = std::min(MAX_FALLBACK_SIZE - fallback.length(), (size_t) length); /* We don't want fallback to be short string optimized, since we want to move it */ fallback.reserve(fallback.length() + maxCopyDistance + std::max(MINIMUM_HTTP_POST_PADDING, sizeof(std::string))); fallback.append(data, maxCopyDistance); // break here on break - std::pair consumed = fenceAndConsumePostPadded(fallback.data(), (int) fallback.length(), user, reserved, &req, requestHandler, dataHandler); + std::pair consumed = fenceAndConsumePostPadded(fallback.data(), (unsigned int) fallback.length(), user, reserved, &req, requestHandler, dataHandler); if (consumed.second != user) { return consumed.second; } @@ -313,8 +313,8 @@ public: fallback.clear(); - data += consumed.first - had; - length -= consumed.first - had; + data += (unsigned int) (consumed.first - had); + length -= (unsigned int) (consumed.first - had); if (remainingStreamingBytes) { // this is exactly the same as above! @@ -351,8 +351,8 @@ public: return consumed.second; } - data += consumed.first; - length -= consumed.first; + data += (unsigned int) consumed.first; + length -= (unsigned int) consumed.first; if (length) { if ((unsigned int) length < MAX_FALLBACK_SIZE) { diff --git a/src/HttpResponse.h b/src/HttpResponse.h index 436e6e3..eedceba 100644 --- a/src/HttpResponse.h +++ b/src/HttpResponse.h @@ -155,7 +155,7 @@ private: /* uSockets only deals with int sizes, so pass chunks of max signed int size */ auto writtenFailed = Super::write(data.data() + written, (int) std::min(data.length() - written, INT_MAX), optional); - written += writtenFailed.first; + written += (size_t) writtenFailed.first; failed = writtenFailed.second; } @@ -221,7 +221,7 @@ public: if (webSocketContextData->compression != DISABLED) { if (secWebSocketExtensions.length()) { /* We never support client context takeover (the client cannot compress with a sliding window). */ - int wantedOptions = PERMESSAGE_DEFLATE | CLIENT_NO_CONTEXT_TAKEOVER; + unsigned int wantedOptions = PERMESSAGE_DEFLATE | CLIENT_NO_CONTEXT_TAKEOVER; /* Shared compressor is the default */ if (webSocketContextData->compression == SHARED_COMPRESSOR) { diff --git a/src/HttpRouter.h b/src/HttpRouter.h index aba0b21..c8de7c8 100644 --- a/src/HttpRouter.h +++ b/src/HttpRouter.h @@ -158,7 +158,7 @@ private: /* If we are on STOP, return where we may stand */ if (isStop) { /* We have reached accross the entire URL with no stoppage, execute */ - for (int handler : parent->handlers) { + for (uint32_t handler : parent->handlers) { if (handlers[handler & HANDLER_MASK](this)) { return true; } @@ -170,7 +170,7 @@ private: for (auto &p : parent->children) { if (p->name.length() && p->name[0] == '*') { /* Wildcard match (can be seen as a shortcut) */ - for (int handler : p->handlers) { + for (uint32_t handler : p->handlers) { if (handlers[handler & HANDLER_MASK](this)) { return true; } diff --git a/src/LoopData.h b/src/LoopData.h index eba52e2..320c1d2 100644 --- a/src/LoopData.h +++ b/src/LoopData.h @@ -57,11 +57,11 @@ public: bool noMark = false; /* Good 16k for SSL perf. */ - static const int CORK_BUFFER_SIZE = 16 * 1024; + static const unsigned int CORK_BUFFER_SIZE = 16 * 1024; /* Cork data */ char *corkBuffer = new char[CORK_BUFFER_SIZE]; - int corkOffset = 0; + unsigned int corkOffset = 0; void *corkedSocket = nullptr; /* Per message deflate data */ diff --git a/src/QueryParser.h b/src/QueryParser.h index 9c04d37..38afd28 100644 --- a/src/QueryParser.h +++ b/src/QueryParser.h @@ -56,7 +56,7 @@ namespace uWS { char *in = (char *) statementValue.data(); /* Write offset */ - int out = 0; + unsigned int out = 0; /* Walk over all chars until end or null char, decoding in place */ for (int i = 0; i < statementValue.length() && in[i]; i++) { diff --git a/src/WebSocket.h b/src/WebSocket.h index 270fb6c..0d6f208 100644 --- a/src/WebSocket.h +++ b/src/WebSocket.h @@ -138,9 +138,9 @@ public: /* Format and send the close frame */ static const int MAX_CLOSE_PAYLOAD = 123; - int length = (int) std::min(MAX_CLOSE_PAYLOAD, message.length()); + size_t length = std::min(MAX_CLOSE_PAYLOAD, message.length()); char closePayload[MAX_CLOSE_PAYLOAD + 2]; - int closePayloadLength = (int) protocol::formatClosePayload(closePayload, (uint16_t) code, message.data(), length); + size_t closePayloadLength = protocol::formatClosePayload(closePayload, (uint16_t) code, message.data(), length); bool ok = send(std::string_view(closePayload, closePayloadLength), OpCode::CLOSE); /* FIN if we are ok and not corked */ diff --git a/src/WebSocketContext.h b/src/WebSocketContext.h index 894e188..2815fa4 100644 --- a/src/WebSocketContext.h +++ b/src/WebSocketContext.h @@ -188,7 +188,7 @@ private: } else { /* Here we never mind any size optimizations as we are in the worst possible path */ webSocketData->fragmentBuffer.append(data, length); - webSocketData->controlTipLength += (int) length; + webSocketData->controlTipLength += (unsigned int) length; if (!remainingBytes && fin) { char *controlBuffer = (char *) webSocketData->fragmentBuffer.data() + webSocketData->fragmentBuffer.length() - webSocketData->controlTipLength; @@ -282,7 +282,7 @@ private: asyncSocket->cork(); /* This parser has virtually no overhead */ - uWS::WebSocketProtocol>::consume(data, length, (WebSocketState *) webSocketData, s); + uWS::WebSocketProtocol>::consume(data, (unsigned int) length, (WebSocketState *) webSocketData, s); /* Uncorking a closed socekt is fine, in fact it is needed */ asyncSocket->uncork(); diff --git a/src/WebSocketContextData.h b/src/WebSocketContextData.h index e5a9b60..6bf4f77 100644 --- a/src/WebSocketContextData.h +++ b/src/WebSocketContextData.h @@ -53,7 +53,7 @@ public: /* Settings for this context */ size_t maxPayloadLength = 0; - int idleTimeout = 0; + unsigned int idleTimeout = 0; /* We do need these for async upgrade */ int compression; diff --git a/src/WebSocketData.h b/src/WebSocketData.h index a26e953..a2fde91 100644 --- a/src/WebSocketData.h +++ b/src/WebSocketData.h @@ -35,7 +35,7 @@ struct WebSocketData : AsyncSocketData, WebSocketState { template friend struct HttpContext; private: std::string fragmentBuffer; - int controlTipLength = 0; + unsigned int controlTipLength = 0; bool isShuttingDown = 0; enum CompressionStatus : char { DISABLED, diff --git a/src/WebSocketExtensions.h b/src/WebSocketExtensions.h index 79994ee..d445b92 100644 --- a/src/WebSocketExtensions.h +++ b/src/WebSocketExtensions.h @@ -112,10 +112,10 @@ public: template struct ExtensionsNegotiator { protected: - int options; + unsigned int options; public: - ExtensionsNegotiator(int wantedOptions) { + ExtensionsNegotiator(unsigned int wantedOptions) { options = wantedOptions; } @@ -159,7 +159,7 @@ public: } } - int getNegotiatedOptions() { + unsigned int getNegotiatedOptions() { return options; } }; diff --git a/src/WebSocketHandshake.h b/src/WebSocketHandshake.h index f3bb3f5..901cb1d 100644 --- a/src/WebSocketHandshake.h +++ b/src/WebSocketHandshake.h @@ -123,7 +123,7 @@ public: }; for (int i = 0; i < 6; i++) { - b_input[i] = (input[4 * i + 3] & 0xff) | (input[4 * i + 2] & 0xff) << 8 | (input[4 * i + 1] & 0xff) << 16 | (input[4 * i + 0] & 0xff) << 24; + b_input[i] = (uint32_t) ((input[4 * i + 3] & 0xff) | (input[4 * i + 2] & 0xff) << 8 | (input[4 * i + 1] & 0xff) << 16 | (input[4 * i + 0] & 0xff) << 24); } sha1(b_output, b_input); uint32_t last_b[16] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 480}; diff --git a/src/WebSocketProtocol.h b/src/WebSocketProtocol.h index 01171b3..34ec31b 100644 --- a/src/WebSocketProtocol.h +++ b/src/WebSocketProtocol.h @@ -230,7 +230,7 @@ static inline size_t formatMessage(char *dst, const char *src, size_t length, Op char mask[4]; if (!isServer) { dst[1] |= 0x80; - uint32_t random = rand(); + uint32_t random = (uint32_t) rand(); memcpy(mask, &random, 4); memcpy(dst + headerLength, &random, 4); headerLength += 4;