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.
nim-lang · Feb 14, 2020 · 5bf571f · 5bf571f
1 parent 3300e31
commit 5bf571f
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 44 deletions.
diff --git 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
@@ -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 &"""
 ##    <!Doctype html>
 ##    <html lang="en">
-##      <head>
-##        <meta charset="utf-8"/>
-##      </head>
+##      <head><meta charset="utf-8"/></head>
 ##      <body>
 ##        <form action="/" method="post" enctype="multipart/form-data">
 ##          File: <input type="file" name="testfile" accept="text/*"><br />
@@ -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: