From 159643824b4307597ece03597a44579c5fbcc9ed Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Mon, 27 Mar 2017 09:39:46 +0200 Subject: [PATCH] fixes #3847 (#5609) --- lib/pure/asynchttpserver.nim | 34 +++++++++++++++++++++++++++------- lib/pure/asyncnet.nim | 16 ++++------------ lib/pure/httpcore.nim | 4 ++-- lib/pure/net.nim | 11 +++-------- 4 files changed, 36 insertions(+), 29 deletions(-) diff --git a/lib/pure/asynchttpserver.nim b/lib/pure/asynchttpserver.nim index 80aa8bb2c..8d059dbbc 100644 --- a/lib/pure/asynchttpserver.nim +++ b/lib/pure/asynchttpserver.nim @@ -36,6 +36,9 @@ import httpcore export httpcore except parseHeader +const + maxLine = 8*1024 + # TODO: If it turns out that the decisions that asynchttpserver makes # explicitly, about whether to close the client sockets or upgrade them are # wrong, then add a return value which determines what to do for the callback. @@ -97,6 +100,18 @@ proc respond*(req: Request, code: HttpCode, content: string, if headers != nil: msg.addHeaders(headers) + msg.add("Content-Length: ") + # this particular way saves allocations: + msg.add content.len + msg.add "\c\L\c\L" + msg.add(content) + result = req.client.send(msg) + +proc respondError(req: Request, code: HttpCode): Future[void] = + ## Responds to the request with the specified ``HttpCode``. + let content = $code + var msg = "HTTP/1.1 " & content & "\c\L" + msg.add("Content-Length: " & $content.len & "\c\L\c\L") msg.add(content) result = req.client.send(msg) @@ -139,12 +154,16 @@ proc processClient(client: AsyncSocket, address: string, for i in 0..1: lineFut.mget().setLen(0) lineFut.clean() - await client.recvLineInto(lineFut) # TODO: Timeouts. + await client.recvLineInto(lineFut, maxLength=maxLine) # TODO: Timeouts. if lineFut.mget == "": client.close() return + if lineFut.mget.len > maxLine: + await request.respondError(Http413) + client.close() + return if lineFut.mget != "\c\L": break @@ -157,19 +176,17 @@ proc processClient(client: AsyncSocket, address: string, # TODO: this is likely slow. request.reqMethod = parseEnum[HttpMethod]("http" & linePart) except ValueError: - asyncCheck request.respond(Http400, "Invalid request method. Got: " & - linePart) + asyncCheck request.respondError(Http400) continue of 1: parseUri(linePart, request.url) of 2: try: request.protocol = parseProtocol(linePart) except ValueError: - asyncCheck request.respond(Http400, - "Invalid request protocol. Got: " & linePart) + asyncCheck request.respondError(Http400) continue else: - await request.respond(Http400, "Invalid request. Got: " & lineFut.mget) + await request.respondError(Http400) continue inc i @@ -178,10 +195,13 @@ proc processClient(client: AsyncSocket, address: string, i = 0 lineFut.mget.setLen(0) lineFut.clean() - await client.recvLineInto(lineFut) + await client.recvLineInto(lineFut, maxLength=maxLine) if lineFut.mget == "": client.close(); return + if lineFut.mget.len > maxLine: + await request.respondError(Http413) + client.close(); return if lineFut.mget == "\c\L": break let (key, value) = parseHeader(lineFut.mget) request.headers[key] = value diff --git a/lib/pure/asyncnet.nim b/lib/pure/asyncnet.nim index 2d5c65001..1ec751a64 100644 --- a/lib/pure/asyncnet.nim +++ b/lib/pure/asyncnet.nim @@ -464,8 +464,7 @@ proc recvLineInto*(socket: AsyncSocket, resString: FutureVar[string], ## The partial line **will be lost**. ## ## The ``maxLength`` parameter determines the maximum amount of characters - ## that can be read before a ``ValueError`` is raised. This prevents Denial - ## of Service (DOS) attacks. + ## that can be read. ``resString`` will be truncated after that. ## ## **Warning**: The ``Peek`` flag is not yet implemented. ## @@ -519,10 +518,7 @@ proc recvLineInto*(socket: AsyncSocket, resString: FutureVar[string], socket.currPos.inc() # Verify that this isn't a DOS attack: #3847. - if resString.mget.len > maxLength: - let msg = "recvLine received more than the specified `maxLength` " & - "allowed." - raise newException(ValueError, msg) + if resString.mget.len > maxLength: break else: var c = "" while true: @@ -546,10 +542,7 @@ proc recvLineInto*(socket: AsyncSocket, resString: FutureVar[string], resString.mget.add c # Verify that this isn't a DOS attack: #3847. - if resString.mget.len > maxLength: - let msg = "recvLine received more than the specified `maxLength` " & - "allowed." - raise newException(ValueError, msg) + if resString.mget.len > maxLength: break resString.complete() proc recvLine*(socket: AsyncSocket, @@ -569,8 +562,7 @@ proc recvLine*(socket: AsyncSocket, ## The partial line **will be lost**. ## ## The ``maxLength`` parameter determines the maximum amount of characters - ## that can be read before a ``ValueError`` is raised. This prevents Denial - ## of Service (DOS) attacks. + ## that can be read. The result is truncated after that. ## ## **Warning**: The ``Peek`` flag is not yet implemented. ## diff --git a/lib/pure/httpcore.nim b/lib/pure/httpcore.nim index aa8f1958d..a5ab40ca4 100644 --- a/lib/pure/httpcore.nim +++ b/lib/pure/httpcore.nim @@ -285,7 +285,7 @@ proc `$`*(code: HttpCode): string = proc `==`*(a, b: HttpCode): bool {.borrow.} proc `==`*(rawCode: string, code: HttpCode): bool = - return rawCode.toLower() == ($code).toLower() + return cmpIgnoreCase(rawCode, $code) == 0 proc is2xx*(code: HttpCode): bool = ## Determines whether ``code`` is a 2xx HTTP status code. @@ -304,7 +304,7 @@ proc is5xx*(code: HttpCode): bool = return code.int in {500 .. 599} proc `$`*(httpMethod: HttpMethod): string = - return (system.`$`(httpMethod))[4 .. ^1].toUpper() + return (system.`$`(httpMethod))[4 .. ^1].toUpperAscii() when isMainModule: var test = newHttpHeaders() diff --git a/lib/pure/net.nim b/lib/pure/net.nim index 7f6783358..56f8b9399 100644 --- a/lib/pure/net.nim +++ b/lib/pure/net.nim @@ -1010,8 +1010,7 @@ proc readLine*(socket: Socket, line: var TaintedString, timeout = -1, ## the specified time an ETimeout exception will be raised. ## ## The ``maxLength`` parameter determines the maximum amount of characters - ## that can be read before a ``ValueError`` is raised. This prevents Denial - ## of Service (DOS) attacks. + ## that can be read. The result is truncated after that. ## ## **Warning**: Only the ``SafeDisconn`` flag is currently supported. @@ -1047,10 +1046,7 @@ proc readLine*(socket: Socket, line: var TaintedString, timeout = -1, add(line.string, c) # Verify that this isn't a DOS attack: #3847. - if line.string.len > maxLength: - let msg = "recvLine received more than the specified `maxLength` " & - "allowed." - raise newException(ValueError, msg) + if line.string.len > maxLength: break proc recvLine*(socket: Socket, timeout = -1, flags = {SocketFlag.SafeDisconn}, @@ -1069,8 +1065,7 @@ proc recvLine*(socket: Socket, timeout = -1, ## the specified time an ETimeout exception will be raised. ## ## The ``maxLength`` parameter determines the maximum amount of characters - ## that can be read before a ``ValueError`` is raised. This prevents Denial - ## of Service (DOS) attacks. + ## that can be read. The result is truncated after that. ## ## **Warning**: Only the ``SafeDisconn`` flag is currently supported. result = ""