improvements for httpcore (#12228)

* improvements for httpcore
* further improvements, now stable API but needs extensions later on

4 files changed, 46 insertions, 44 deletions

diff --git a/changelog.md b/changelog.md
index bbcd6d970..e062403d0 100644
--- a/changelog.md
+++ b/changelog.md
@@ -83,7 +83,6 @@ type
   - concurrency/threadpool
   - coro
   - endians
-  - httpcore
   - parsesql
   - pathnorm
   - reservedmem
diff --git a/lib/pure/asynchttpserver.nim b/lib/pure/asynchttpserver.nim
index 876a3a40b..9117f4d7b 100644
--- a/lib/pure/asynchttpserver.nim
+++ b/lib/pure/asynchttpserver.nim
@@ -78,7 +78,7 @@ proc sendHeaders*(req: Request, headers: HttpHeaders): Future[void] =
   return req.client.send(msg)
 
 proc respond*(req: Request, code: HttpCode, content: string,
-              headers: HttpHeaders = nil): Future[void] =
+              headers = EmptyHttpHeaders): Future[void] =
   ## Responds to the request with the specified ``HttpCode``, headers and
   ## content.
   ##
@@ -97,7 +97,7 @@ proc respond*(req: Request, code: HttpCode, content: string,
   ##        await req.respond(Http404, "Not Found")
   var msg = "HTTP/1.1 " & $code & "\c\L"
 
-  if headers != nil:
+  if isEmpty(headers):
     msg.addHeaders(headers)
   msg.add("Content-Length: ")
   # this particular way saves allocations:
diff --git a/lib/pure/httpclient.nim b/lib/pure/httpclient.nim
index f89a928ab..0f5b4b604 100644
--- a/lib/pure/httpclient.nim
+++ b/lib/pure/httpclient.nim
@@ -847,7 +847,7 @@ proc newConnection(client: HttpClient | AsyncHttpClient,
 
 proc override(fallback, override: HttpHeaders): HttpHeaders =
   # Right-biased map union for `HttpHeaders`
-  if override.isNil:
+  if override.isEmpty:
     return fallback
 
   result = newHttpHeaders()
@@ -858,7 +858,7 @@ proc override(fallback, override: HttpHeaders): HttpHeaders =
 
 proc requestAux(client: HttpClient | AsyncHttpClient, url: string,
                 httpMethod: string, body = "",
-                headers: HttpHeaders = nil): Future[Response | AsyncResponse]
+                headers = EmptyHttpHeaders): Future[Response | AsyncResponse]
                 {.multisync.} =
   # Helper that actually makes the request. Does not handle redirects.
   let requestUrl = parseUri(url)
@@ -892,7 +892,7 @@ proc requestAux(client: HttpClient | AsyncHttpClient, url: string,
 
 proc request*(client: HttpClient | AsyncHttpClient, url: string,
               httpMethod: string, body = "",
-              headers: HttpHeaders = nil): Future[Response | AsyncResponse]
+              headers = EmptyHttpHeaders): Future[Response | AsyncResponse]
               {.multisync.} =
   ## Connects to the hostname specified by the URL and performs a request
   ## using the custom method string specified by ``httpMethod``.
@@ -917,7 +917,7 @@ proc request*(client: HttpClient | AsyncHttpClient, url: string,
 
 proc request*(client: HttpClient | AsyncHttpClient, url: string,
               httpMethod = HttpGet, body = "",
-              headers: HttpHeaders = nil): Future[Response | AsyncResponse]
+              headers = EmptyHttpHeaders): Future[Response | AsyncResponse]
               {.multisync.} =
   ## Connects to the hostname specified by the URL and performs a request
   ## using the method specified.
diff --git a/lib/pure/httpcore.nim b/lib/pure/httpcore.nim
index 9397deb34..ca55fe56a 100644
--- a/lib/pure/httpcore.nim
+++ b/lib/pure/httpcore.nim
@@ -9,14 +9,12 @@
 
 ## Contains functionality shared between the ``httpclient`` and
 ## ``asynchttpserver`` modules.
-##
-## Unstable API.
 
 import tables, strutils, parseutils
 
 type
-  HttpHeaders* = ref object
-    table*: TableRef[string, seq[string]]
+  HeadersImpl = TableRef[string, seq[string]]
+  HttpHeaders* = distinct HeadersImpl
 
   HttpHeaderValues* = distinct seq[string]
 
@@ -97,25 +95,32 @@ const
   Http504* = HttpCode(504)
   Http505* = HttpCode(505)
 
-const headerLimit* = 10_000
+const
+  headerLimit* = 10_000 ## The limit of HTTP headers in bytes. This limit
+                        ## is not enforced by httpcore but by modules using
+                        ## httpcore.
+  EmptyHttpHeaders* = HttpHeaders(nil) ## Constant that represents empty
+                                       ## http headers.
+
+template table*(x: HttpHeaders): TableRef[string, seq[string]] {.
+    deprecated: "use the other accessor procs instead".} =
+  TableRef[string, seq[string]](x)
 
 proc newHttpHeaders*(): HttpHeaders =
-  new result
-  result.table = newTable[string, seq[string]]()
+  result = HttpHeaders newTable[string, seq[string]]()
 
 proc newHttpHeaders*(keyValuePairs:
     openArray[tuple[key: string, val: string]]): HttpHeaders =
-  var pairs: seq[tuple[key: string, val: seq[string]]] = @[]
+  result = HttpHeaders newTable[string, seq[string]]()
   for pair in keyValuePairs:
-    pairs.add((pair.key.toLowerAscii(), @[pair.val]))
-  new result
-  result.table = newTable[string, seq[string]](pairs)
+    HeadersImpl(result)[pair.key.toLowerAscii()] = @[pair.val]
 
-proc `$`*(headers: HttpHeaders): string =
-  return $headers.table
+proc `$`*(headers: HttpHeaders): string = $(HeadersImpl(headers))
+
+proc isEmpty*(a: HttpHeaders): bool = HeadersImpl(a) == nil
 
 proc clear*(headers: HttpHeaders) =
-  headers.table.clear()
+  HeadersImpl(headers).clear()
 
 proc `[]`*(headers: HttpHeaders, key: string): HttpHeaderValues =
   ## Returns the values associated with the given ``key``. If the returned
@@ -125,43 +130,44 @@ proc `[]`*(headers: HttpHeaders, key: string): HttpHeaderValues =
   ##
   ## To access multiple values of a key, use the overloaded ``[]`` below or
   ## to get all of them access the ``table`` field directly.
-  return headers.table[key.toLowerAscii].HttpHeaderValues
+  result = HeadersImpl(headers)[key.toLowerAscii].HttpHeaderValues
 
 converter toString*(values: HttpHeaderValues): string =
-  return seq[string](values)[0]
+  result = seq[string](values)[0]
 
 proc `[]`*(headers: HttpHeaders, key: string, i: int): string =
   ## Returns the ``i``'th value associated with the given key. If there are
   ## no values associated with the key or the ``i``'th value doesn't exist,
   ## an exception is raised.
-  return headers.table[key.toLowerAscii][i]
+  result = HeadersImpl(headers)[key.toLowerAscii][i]
 
 proc `[]=`*(headers: HttpHeaders, key, value: string) =
   ## Sets the header entries associated with ``key`` to the specified value.
   ## Replaces any existing values.
-  headers.table[key.toLowerAscii] = @[value]
+  HeadersImpl(headers)[key.toLowerAscii] = @[value]
 
 proc `[]=`*(headers: HttpHeaders, key: string, value: seq[string]) =
   ## Sets the header entries associated with ``key`` to the specified list of
   ## values.
   ## Replaces any existing values.
-  headers.table[key.toLowerAscii] = value
+  HeadersImpl(headers)[key.toLowerAscii] = value
 
 proc add*(headers: HttpHeaders, key, value: string) =
   ## Adds the specified value to the specified key. Appends to any existing
   ## values associated with the key.
-  if not headers.table.hasKey(key.toLowerAscii):
-    headers.table[key.toLowerAscii] = @[value]
+  let k = key.toLowerAscii
+  if not HeadersImpl(headers).hasKey(k):
+    HeadersImpl(headers)[k] = @[value]
   else:
-    headers.table[key.toLowerAscii].add(value)
+    HeadersImpl(headers)[k].add(value)
 
 proc del*(headers: HttpHeaders, key: string) =
   ## Delete the header entries associated with ``key``
-  headers.table.del(key.toLowerAscii)
+  HeadersImpl(headers).del(key.toLowerAscii)
 
 iterator pairs*(headers: HttpHeaders): tuple[key, value: string] =
   ## Yields each key, value pair.
-  for k, v in headers.table:
+  for k, v in HeadersImpl(headers):
     for value in v:
       yield (k, value)
 
@@ -172,18 +178,15 @@ proc contains*(values: HttpHeaderValues, value: string): bool =
     if val.toLowerAscii == value.toLowerAscii: return true
 
 proc hasKey*(headers: HttpHeaders, key: string): bool =
-  return headers.table.hasKey(key.toLowerAscii())
+  result = HeadersImpl(headers).hasKey(key.toLowerAscii())
 
 proc getOrDefault*(headers: HttpHeaders, key: string,
     default = @[""].HttpHeaderValues): HttpHeaderValues =
   ## Returns the values associated with the given ``key``. If there are no
   ## values associated with the key, then ``default`` is returned.
-  if headers.hasKey(key):
-    return headers[key]
-  else:
-    return default
+  result = HttpHeaderValues(HeadersImpl(headers).getOrDefault(key, seq[string](default)))
 
-proc len*(headers: HttpHeaders): int = return headers.table.len
+proc len*(headers: HttpHeaders): int = result = HeadersImpl(headers).len
 
 proc parseList(line: string, list: var seq[string], start: int): int =
   var i = 0
@@ -224,7 +227,7 @@ proc `==`*(protocol: tuple[orig: string, major, minor: int],
   result = protocol.major == major and protocol.minor == minor
 
 proc contains*(methods: set[HttpMethod], x: string): bool =
-  return parseEnum[HttpMethod](x) in methods
+  result = parseEnum[HttpMethod](x) in methods
 
 proc `$`*(code: HttpCode): string =
   ## Converts the specified ``HttpCode`` into a HTTP status.
@@ -286,26 +289,26 @@ proc `$`*(code: HttpCode): string =
 proc `==`*(a, b: HttpCode): bool {.borrow.}
 
 proc `==`*(rawCode: string, code: HttpCode): bool =
-  return cmpIgnoreCase(rawCode, $code) == 0
+  result = cmpIgnoreCase(rawCode, $code) == 0
 
 proc is2xx*(code: HttpCode): bool =
   ## Determines whether ``code`` is a 2xx HTTP status code.
-  return code.int in {200 .. 299}
+  result = code.int in {200 .. 299}
 
 proc is3xx*(code: HttpCode): bool =
   ## Determines whether ``code`` is a 3xx HTTP status code.
-  return code.int in {300 .. 399}
+  result = code.int in {300 .. 399}
 
 proc is4xx*(code: HttpCode): bool =
   ## Determines whether ``code`` is a 4xx HTTP status code.
-  return code.int in {400 .. 499}
+  result = code.int in {400 .. 499}
 
 proc is5xx*(code: HttpCode): bool =
   ## Determines whether ``code`` is a 5xx HTTP status code.
-  return code.int in {500 .. 599}
+  result = code.int in {500 .. 599}
 
 proc `$`*(httpMethod: HttpMethod): string =
-  return (system.`$`(httpMethod))[4 .. ^1].toUpperAscii()
+  result = (system.`$`(httpMethod))[4 .. ^1].toUpperAscii()
 
 when isMainModule:
   var test = newHttpHeaders()