Merge branch 'master' into dev/etan/ae-returnstrain

Author: etan-status

chronos.nimble

@@ -1,7 +1,8 @@
 mode = ScriptMode.Verbose
 
 packageName   = "chronos"
-version       = "4.2.2"
+# keep in sync: chronos/apps/http/httpagent.nim
+version       = "4.4.0"
 author        = "Status Research & Development GmbH"
 description   = "Networking framework with async/await support"
 license       = "MIT or Apache License 2.0"
@@ -31,7 +32,7 @@ let testArguments =
     [
       "-d:debug -d:chronosDebug -d:useSysAssert -d:useGcAssert",
       "-d:debug -d:chronosDebug -d:chronosEventEngine=poll -d:useSysAssert -d:useGcAssert",
-      "-d:release",
+      "-d:release -d:chronosPreviewV5",
     ]
 
 let cfg =

chronos/apps/http/httpagent.nim

@@ -14,11 +14,11 @@ import strutils
 const
   ChronosName* = "nim-chronos"
     ## Project name string
-  ChronosMajor* {.intdefine.}: int = 3
+  ChronosMajor* {.intdefine.}: int = 4
     ## Major number of Chronos' version.
-  ChronosMinor* {.intdefine.}: int = 0
+  ChronosMinor* {.intdefine.}: int = 4
     ## Minor number of Chronos' version.
-  ChronosPatch* {.intdefine.}: int = 2
+  ChronosPatch* {.intdefine.}: int = 0
     ## Patch number of Chronos' version.
   ChronosVersion* = $ChronosMajor & "." & $ChronosMinor & "." & $ChronosPatch
     ## Version of Chronos as a string.

chronos/apps/http/httpclient.nim

@@ -188,8 +188,10 @@ type
     reader*: HttpBodyReader
     error*: ref HttpError
     bodyFlag*: HttpClientBodyFlag
-    contentEncoding*: set[ContentEncodingFlags]
-    transferEncoding*: set[TransferEncodingFlags]
+    contentEncoding*{.deprecated.}: set[ContentEncodingFlags]
+    transferEncoding*{.deprecated.}: set[TransferEncodingFlags]
+    contentEncodings*: seq[ContentEncodingFlags]
+    transferEncodings*: seq[TransferEncodingFlags]
     contentLength*: uint64
     contentType*: Opt[ContentTypeData]
     timestamp*: Moment
@@ -268,8 +270,9 @@ template isIdle(conn: HttpClientConnectionRef, timestamp: Moment,
 
 proc sessionWatcher(session: HttpSessionRef) {.async: (raises: []).}
 proc directProvider*(): HttpConnectionProvider
+
 proc new*(t: typedesc[HttpSessionRef],
-          flags: HttpClientFlags = {NewConnectionAlways},
+          flags: HttpClientFlags = {},
           maxRedirections = HttpMaxRedirections,
           connectTimeout = HttpConnectTimeout,
           headersTimeout = HttpHeadersTimeout,
@@ -288,6 +291,31 @@ proc new*(t: typedesc[HttpSessionRef],
   ## ``idleTimeout`` - timeout to consider HTTP connection as idle
   ## ``idlePeriod`` - period of time to check HTTP connections for inactivity
   doAssert(maxRedirections >= 0, "maxRedirections should not be negative")
+
+  # TODO chronos v4.2 and earlier enabled persistent connections only
+  #      together with the pipelining flag (persistent connections are
+  #      necessary but not sufficient for pipelining).
+  #
+  #      Since users of chronos (like presto and json-rpc) didn't include
+  #      pipelining in their defaults, this had the practical effect of disabling
+  #      persistent connections by default which is a good thing because the
+  #      current implementation of connection reuse in the http client is
+  #      practically unusable due to the lack of EOF monitoring (without EOF
+  #      monitoring, connections that get closed are not detected leading to
+  #      frequent request failures in most usage scenarios).
+  #
+  #      Although connection reuse is useful even without pipelining, we'll
+  #      keep using the deprecated pipelining flag as a gatekeeper so that
+  #      practically, it remains disabled in most cases - this gatekeeper
+  #      will be removed once reuse is feature complete.
+  {.push warning[Deprecated]: off.}
+  let flags =
+    if HttpClientFlag.Http11Pipeline notin flags:
+      flags + {HttpClientFlag.NewConnectionAlways}
+    else:
+      flags
+  {.pop.}
+
   let res = HttpSessionRef(
     flags: flags,
     maxRedirections: maxRedirections,
@@ -357,17 +385,20 @@ proc getHttpAddress*(
                  path: url.path, query: url.query, anchor: url.anchor,
                  username: url.username, password: url.password))
 
+{.push warning[Deprecated]: off.}
 proc getHttpAddress*(
        uri: Uri,
        flags: HttpClientFlags
      ): HttpAddressResult {.deprecated: "No DNS resolution in getHttpAddress, no flags needed".} =
   getHttpAddress(uri)
+{.pop.}
 
 proc getHttpAddress*(
        url: string,
      ): HttpAddressResult =
   getHttpAddress(parseUri(url))
 
+{.push warning[Deprecated]: off.}
 proc getHttpAddress*(
        url: string,
        flags: HttpClientFlags
@@ -387,6 +418,8 @@ proc getHttpAddress*(
   ## Create new HTTP address using URL string ``url`` and .
   getHttpAddress(parseUri(url))
 
+{.pop.}
+
 proc getAddress*(session: HttpSessionRef, url: Uri): HttpResult[HttpAddress] {.deprecated: "use getHttpAddress".} =
   let res = getHttpAddress(url).valueOr:
     return err($error)
@@ -721,7 +754,6 @@ proc reuseOrConnect(
         connection.flags.incl(HttpClientConnectionFlag.KeepAlive)
         connection.state = HttpClientConnectionState.Acquired
         return connection
-
   let connection =
     try:
       await session.connect(ha).wait(session.connectTimeout)
@@ -906,22 +938,27 @@ proc prepareResponse(
       res
 
   # Preprocessing "Content-Encoding" header.
-  let contentEncoding =
-    block:
-      let res = getContentEncoding(headers.getList(ContentEncodingHeader))
-      if res.isErr():
-        return err("Invalid headers received, invalid `Content-Encoding`")
+  let
+    contentEncodings = getContentEncodings(headers.getList(ContentEncodingHeader)).valueOr:
+      return err("Invalid `Content-Encoding` in headers")
+    contentEncoding = {
+      if contentEncodings.len() > 0:
+        contentEncodings[^1]
       else:
-        res.get()
+        ContentEncodingFlags.Identity
+    }
 
   # Preprocessing "Transfer-Encoding" header.
-  let transferEncoding =
-    block:
-      let res = getTransferEncoding(headers.getList(TransferEncodingHeader))
-      if res.isErr():
-        return err("Invalid headers received, invalid `Transfer-Encoding`")
+  let
+    transferEncodings = getTransferEncodings(
+        headers.getList(TransferEncodingHeader)).valueOr:
+      return err("Invalid `Transfer-Encoding` in headers")
+    transferEncoding = {
+      if transferEncodings.len() > 0:
+        transferEncodings[^1]
       else:
-        res.get()
+        TransferEncodingFlags.Identity
+    }
 
   # Preprocessing "Content-Length" header.
   let (contentLength, bodyFlag) =
@@ -934,9 +971,9 @@ proc prepareResponse(
       # header
       let length = headers.getInt(ContentLengthHeader)
       (length, HttpClientBodyFlag.NoBody)
-    elif transferEncoding != {TransferEncodingFlags.Identity}:
+    elif transferEncodings.len > 0:
       # "Transfer-Encoding overrides the Content-Length"
-      if TransferEncodingFlags.Chunked in transferEncoding:
+      if transferEncodings[^1] == TransferEncodingFlags.Chunked:
         (0'u64, HttpClientBodyFlag.Chunked)
       else:
         (0'u64, HttpClientBodyFlag.Custom)
@@ -986,6 +1023,7 @@ proc prepareResponse(
     reason: resp.reason(data), version: resp.version, session: request.session,
     connection: connection, headers: headers,
     contentEncoding: contentEncoding, transferEncoding: transferEncoding,
+    contentEncodings: contentEncodings, transferEncodings: transferEncodings,
     contentLength: contentLength, contentType: contentType, bodyFlag: bodyFlag
   )
 

chronos/apps/http/httpcommon.nim

@@ -150,9 +150,11 @@ proc raiseHttpRequestBodyTooLargeError*() {.
   raise (ref HttpRequestBodyTooLargeError)(
     code: Http413, msg: MaximumBodySizeError)
 
+{.push warning[Deprecated]: off.}
 proc raiseHttpCriticalError*(msg: string, code = Http400) {.
      noinline, noreturn, raises: [HttpCriticalError], deprecated.} =
   raise (ref HttpCriticalError)(code: code, msg: msg)
+{.pop.}
 
 proc raiseHttpDisconnectError*() {.
      noinline, noreturn, raises: [HttpDisconnectError].} =
@@ -237,69 +239,106 @@ iterator queryParams*(query: string,
       else:
         yield (decodeUrl(k), decodeUrl(v))
 
+func getTransferEncodings*(
+       ch: openArray[string]
+     ): HttpResult[seq[TransferEncodingFlags]] =
+  ## Parse value of multiple Transfer-Encoding headers and return the list.
+  ##
+  ## `identity` is a no-op encoding that is accepted and ignored for backwards
+  ## compatibility.
+  ##
+  ## See also:
+  ##   * https://www.rfc-editor.org/rfc/rfc9112#section-6.1
+  ##   * https://www.rfc-editor.org/rfc/rfc9110#section-8.4
+  ##   * https://www.iana.org/assignments/http-parameters/http-parameters.xhtml#transfer-coding
+  var
+    res: seq[TransferEncodingFlags]
+    chunked = 0
+
+  for header in ch:
+    for item in header.split(","):
+      case strip(item.toLowerAscii())
+      of "identity": # deprecated in HTTP/1.1+; accepted for compat
+        discard
+      of "chunked":
+        res.add(TransferEncodingFlags.Chunked)
+        inc chunked
+      of "compress", "x-compress":
+        res.add(TransferEncodingFlags.Compress)
+      of "deflate":
+        res.add(TransferEncodingFlags.Deflate)
+      of "gzip", "x-gzip":
+        res.add(TransferEncodingFlags.Gzip)
+      else:
+        return err("Unsupported Transfer-Encoding value")
+
+  # Validate RFC 9112 Section 6.1 rules:
+  # 1) chunked MUST NOT appear more than once
+  # 2) if chunked is present, it MUST be the final (last) encoding
+  if chunked > 1:
+    return err("Transfer-Encoding contains duplicate chunked encoding")
+  if chunked == 1 and res[^1] != TransferEncodingFlags.Chunked:
+    return err("chunked transfer coding must be the final encoding")
+
+  ok(res)
+
 func getTransferEncoding*(
        ch: openArray[string]
-     ): HttpResult[set[TransferEncodingFlags]] =
+     ): HttpResult[set[TransferEncodingFlags]] {.deprecated: "getTransferEncodings".} =
   ## Parse value of multiple HTTP headers ``Transfer-Encoding`` and return
-  ## it as set of ``TransferEncodingFlags``.
-  # TODO Transfer-Encoding is ordered, thus using a set here is wrong.
-  #      https://www.rfc-editor.org/info/rfc9112/#section-6.1
-  # https://www.iana.org/assignments/http-parameters/http-parameters.xhtml#transfer-coding
-  var res: set[TransferEncodingFlags] = {}
-  if len(ch) == 0:
-    res.incl(TransferEncodingFlags.Identity)
-    ok(res)
-  else:
-    for header in ch:
-      for item in header.split(","):
-        case strip(item.toLowerAscii())
-        of "identity":
-          res.incl(TransferEncodingFlags.Identity)
-        of "chunked":
-          res.incl(TransferEncodingFlags.Chunked)
-        of "compress", "x-compress":
-          res.incl(TransferEncodingFlags.Compress)
-        of "deflate":
-          res.incl(TransferEncodingFlags.Deflate)
-        of "gzip", "x-gzip":
-          res.incl(TransferEncodingFlags.Gzip)
-        of "":
-          res.incl(TransferEncodingFlags.Identity)
-        else:
-          return err("Unsupported Transfer-Encoding value")
-    ok(res)
+  ## the last entry, ie the first encoding that must be decoded.
+  let encodings = ?getTransferEncodings(ch)
+  ok {
+    if encodings.len() > 0:
+      encodings[^1]
+    else:
+      TransferEncodingFlags.Identity
+  }
+
+func getContentEncodings*(
+       ch: openArray[string]
+     ): HttpResult[seq[ContentEncodingFlags]] =
+  ## Parse value of multiple ``Content-Encoding`` headers and return the
+  ## list of ``ContentEncodingFlags``.
+  ##
+  ## If no encodings are given, returns an empty seq.
+  ##
+  ## `identity` is a no-op encoding that is accepted and ignored for backwards
+  ## compatibility.
+  ##
+  ## See also:
+  ##   * https://www.rfc-editor.org/rfc/rfc9110#section-8.4
+  ##   * https://www.iana.org/assignments/http-parameters/http-parameters.xhtml#content-coding
+  var res: seq[ContentEncodingFlags]
+  for header in ch:
+    for item in header.split(","):
+      case strip(item.toLowerAscii()):
+      of "identity": # valid in Accept-Encoding; accepted here for completeness
+        discard
+      of "br":
+        res.add(ContentEncodingFlags.Br)
+      of "compress", "x-compress":
+        res.add(ContentEncodingFlags.Compress)
+      of "deflate":
+        res.add(ContentEncodingFlags.Deflate)
+      of "gzip", "x-gzip":
+        res.add(ContentEncodingFlags.Gzip)
+      else:
+        return err("Unsupported Content-Encoding value")
+  ok(res)
 
 func getContentEncoding*(
        ch: openArray[string]
-     ): HttpResult[set[ContentEncodingFlags]] =
+     ): HttpResult[set[ContentEncodingFlags]] {.deprecated: "getContentEncodings".} =
   ## Parse value of multiple HTTP headers ``Content-Encoding`` and return
-  ## it as set of ``ContentEncodingFlags``.
-  # TODO Content-Encoding is ordered, thus using a set here is wrong.
-  #      https://www.rfc-editor.org/info/rfc9110/#section-8.4
-  # https://www.iana.org/assignments/http-parameters/http-parameters.xhtml#content-coding
-  var res: set[ContentEncodingFlags] = {}
-  if len(ch) == 0:
-    res.incl(ContentEncodingFlags.Identity)
-    ok(res)
-  else:
-    for header in ch:
-      for item in header.split(","):
-        case strip(item.toLowerAscii()):
-        of "identity":
-          res.incl(ContentEncodingFlags.Identity)
-        of "br":
-          res.incl(ContentEncodingFlags.Br)
-        of "compress", "x-compress":
-          res.incl(ContentEncodingFlags.Compress)
-        of "deflate":
-          res.incl(ContentEncodingFlags.Deflate)
-        of "gzip", "x-gzip":
-          res.incl(ContentEncodingFlags.Gzip)
-        of "":
-          res.incl(ContentEncodingFlags.Identity)
-        else:
-          return err("Unsupported Content-Encoding value")
-    ok(res)
+  ## the last entry, ie the first encoding that must be decoded.
+  let encodings = ?getContentEncodings(ch)
+  ok {
+    if encodings.len() > 0:
+      encodings[^1]
+    else:
+      ContentEncodingFlags.Identity
+  }
 
 func isPersistent*(version: HttpVersion, headers: HttpTable): bool =
   if version >= HttpVersion20: