From 5bf571f061d53d35aab727f420afd9f415987723 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Fri, 14 Feb 2020 20:59:37 +0000 Subject: [PATCH] Fix to asynchttpserver form data/body broken with #13147 (#13394) * Fix to asynchttpserver form data/body broken with #13147 * New implementation that use a interator instance of future streams * asynchttpserver now can handle chunks of data. --- changelog.md | 3 +- lib/pure/asynchttpserver.nim | 116 ++++++++++++++++++++++------------- 2 files changed, 75 insertions(+), 44 deletions(-) diff --git a/changelog.md b/changelog.md index 9bec0c47a8f49..52b202d8c442b 100644 --- a/changelog.md +++ b/changelog.md @@ -65,7 +65,8 @@ ## Library changes -- `asynchttpserver` now the request body is a FutureStream. +- `asynchttpserver` added an iterator that allows the request body to be read in + chunks of data when new server "stream" option is set to true. - `asyncdispatch.drain` now properly takes into account `selector.hasPendingOperations` and only returns once all pending async operations are guaranteed to have completed. - `asyncdispatch.drain` now consistently uses the passed timeout value for all diff --git a/lib/pure/asynchttpserver.nim b/lib/pure/asynchttpserver.nim index 5292ac75ae6bc..9aed0255bbf36 100644 --- a/lib/pure/asynchttpserver.nim +++ b/lib/pure/asynchttpserver.nim @@ -41,13 +41,13 @@ ## import asynchttpserver, asyncdispatch ## import strutils, strformat ## +## const stream = true # for test purposes switch from true to false +## ## proc htmlpage(contentLength, bodyLength: int): string = ## return &""" ## ## -## -## -## +## ## ##
## File:
@@ -65,17 +65,23 @@ ## bodyLength = 0 ## if req.reqMethod == HttpPost: ## contentLength = req.headers["Content-length"].parseInt -## if contentLength < 8*1024: # the default chunkSize -## # read the request body at once -## let body = await req.bodyStream.readAll(); -## bodyLength = body.len +## if stream: +## # Read 8*1024 bytes at a time +## # optional chunkSize parameter. The default is 8*1024 +## for length, data in req.bodyStream(8*1024): +## let content = await data +## if length == content.len: +## bodyLength += content.len +## else: +## # Handle exception +## await req.respond(Http400, +## "Bad Request. Data read has a different length than the expected.") +## return ## else: -## # read 8*1024 bytes at a time -## while (let data = await req.bodyStream.read(); data[0]): -## bodyLength += data[1].len +## bodyLength += req.body.len ## await req.respond(Http200, htmlpage(contentLength, bodyLength)) ## -## let server = newAsyncHttpServer(maxBody = 10485760) # 10 MB +## let server = newAsyncHttpServer(maxBody = 10485760, stream = stream) # 10 MB ## waitFor server.serve(Port(8080), cb) import tables, asyncnet, asyncdispatch, parseutils, uri, strutils @@ -93,9 +99,6 @@ const maxLine = 8*1024 when (NimMajor, NimMinor) >= (1, 1): - const - chunkSize = 8*1024 ## This seems perfectly reasonable for default chunkSize. - type Request* = object client*: AsyncSocket # TODO: Separate this into a Response object? @@ -104,8 +107,16 @@ when (NimMajor, NimMinor) >= (1, 1): protocol*: tuple[orig: string, major, minor: int] url*: Uri hostname*: string ## The hostname of the client that made the request. - body*: string # For future removal - bodyStream*: FutureStream[string] + body*: string + contentLength*: int + + type + AsyncHttpServer* = ref object + socket: AsyncSocket + reuseAddr: bool + reusePort: bool + maxBody: int ## The maximum content-length that will be read for the body. + stream: bool ## By default (stream = false), the body of the request is read immediately else: type Request* = object @@ -117,20 +128,30 @@ else: hostname*: string ## The hostname of the client that made the request. body*: string -type - AsyncHttpServer* = ref object - socket: AsyncSocket - reuseAddr: bool - reusePort: bool - maxBody: int ## The maximum content-length that will be read for the body. + type + AsyncHttpServer* = ref object + socket: AsyncSocket + reuseAddr: bool + reusePort: bool + maxBody: int ## The maximum content-length that will be read for the body. -proc newAsyncHttpServer*(reuseAddr = true, reusePort = false, +when (NimMajor, NimMinor) >= (1, 1): + proc newAsyncHttpServer*(reuseAddr = true, reusePort = false, + maxBody = 8388608, stream = false): AsyncHttpServer = + ## Creates a new ``AsyncHttpServer`` instance. + new result + result.reuseAddr = reuseAddr + result.reusePort = reusePort + result.maxBody = maxBody + result.stream = stream +else: + proc newAsyncHttpServer*(reuseAddr = true, reusePort = false, maxBody = 8388608): AsyncHttpServer = - ## Creates a new ``AsyncHttpServer`` instance. - new result - result.reuseAddr = reuseAddr - result.reusePort = reusePort - result.maxBody = maxBody + ## Creates a new ``AsyncHttpServer`` instance. + new result + result.reuseAddr = reuseAddr + result.reusePort = reusePort + result.maxBody = maxBody proc addHeaders(msg: var string, headers: HttpHeaders) = for k, v in headers: @@ -193,13 +214,30 @@ proc parseProtocol(protocol: string): tuple[orig: string, major, minor: int] = proc sendStatus(client: AsyncSocket, status: string): Future[void] = client.send("HTTP/1.1 " & status & "\c\L\c\L") +when (NimMajor, NimMinor) >= (1, 1): + iterator bodyStream*( + request: Request, + chunkSize: int = 8*1024): (int, Future[string]) = + ## The chunkSize parameter is optional and default value is 8*1024 bytes. + ## + ## This iterator return a tuple with the length of the data that was read + ## and a future. + var remainder = request.contentLength + while remainder > 0: + let readSize = min(remainder, chunkSize) + let data = request.client.recv(readSize) + if data.failed: + raise newException(ValueError, "Error reading POST data from client.") + yield (readSize, data) + remainder -= readSize + proc processRequest( server: AsyncHttpServer, req: FutureVar[Request], client: AsyncSocket, address: string, lineFut: FutureVar[string], - callback: proc (request: Request): Future[void] {.closure, gcsafe.}, + callback: proc (request: Request): Future[void] {.closure, gcsafe.} ): Future[bool] {.async.} = # Alias `request` to `req.mget()` so we don't have to write `mget` everywhere. @@ -213,13 +251,9 @@ proc processRequest( request.hostname.shallowCopy(address) assert client != nil request.client = client + request.body = "" when (NimMajor, NimMinor) >= (1, 1): - request.bodyStream = newFutureStream[string]() - # To uncomment in the future after compatibility issues - # with third parties are solved - # else: - # request.body = "" - request.body = "" # Temporary fix for future removal + request.contentLength = 0 # We should skip at least one empty line before the request # https://tools.ietf.org/html/rfc7230#section-3.5 @@ -316,16 +350,12 @@ proc processRequest( return false when (NimMajor, NimMinor) >= (1, 1): - var remainder = contentLength - while remainder > 0: - let readSize = min(remainder, chunkSize) - let data = await client.recv(read_size) - if data.len != read_size: + request.contentLength = contentLength + if not server.stream: + request.body = await client.recv(contentLength) + if request.body.len != contentLength: await request.respond(Http400, "Bad Request. Content-Length does not match actual.") return true - await request.bodyStream.write(data) - remainder -= data.len - request.bodyStream.complete() else: request.body = await client.recv(contentLength) if request.body.len != contentLength: