HttpAddress errors should be not only critical. (#446)

* Distinguish between resolve errors and check errors.

* Fix issues and add test for getHttpAddress() procedure.

* Address review comments.

chronos/apps/http/httpclient.nim

@@ -195,6 +195,8 @@ type
     name*: string
     data*: string
 
+  HttpAddressResult* = Result[HttpAddress, HttpAddressErrorType]
+
 # HttpClientRequestRef valid states are:
 # Ready -> Open -> (Finished, Error) -> (Closing, Closed)
 #
@@ -298,6 +300,89 @@ proc getTLSFlags(flags: HttpClientFlags): set[TLSFlags] {.raises: [] .} =
     res.incl(TLSFlags.NoVerifyServerName)
   res
 
+proc getHttpAddress*(
+       url: Uri,
+       flags: HttpClientFlags = {}
+     ): HttpAddressResult {.raises: [].} =
+  let
+    scheme =
+      if len(url.scheme) == 0:
+        HttpClientScheme.NonSecure
+      else:
+        case toLowerAscii(url.scheme)
+        of "http":
+          HttpClientScheme.NonSecure
+        of "https":
+          HttpClientScheme.Secure
+        else:
+          return err(HttpAddressErrorType.InvalidUrlScheme)
+    port =
+      if len(url.port) == 0:
+        case scheme
+        of HttpClientScheme.NonSecure:
+          80'u16
+        of HttpClientScheme.Secure:
+          443'u16
+      else:
+        Base10.decode(uint16, url.port).valueOr:
+          return err(HttpAddressErrorType.InvalidPortNumber)
+    hostname =
+      block:
+        if len(url.hostname) == 0:
+          return err(HttpAddressErrorType.MissingHostname)
+        url.hostname
+    id = hostname & ":" & Base10.toString(port)
+    addresses =
+      if (HttpClientFlag.NoInet4Resolution in flags) and
+         (HttpClientFlag.NoInet6Resolution in flags):
+        # DNS resolution is disabled.
+        try:
+          @[initTAddress(hostname, Port(port))]
+        except TransportAddressError:
+          return err(HttpAddressErrorType.InvalidIpHostname)
+      else:
+        try:
+          if (HttpClientFlag.NoInet4Resolution notin flags) and
+             (HttpClientFlag.NoInet6Resolution notin flags):
+            # DNS resolution for both IPv4 and IPv6 addresses.
+            resolveTAddress(hostname, Port(port))
+          else:
+            if HttpClientFlag.NoInet6Resolution in flags:
+              # DNS resolution only for IPv4 addresses.
+              resolveTAddress(hostname, Port(port), AddressFamily.IPv4)
+            else:
+              # DNS resolution only for IPv6 addresses
+              resolveTAddress(hostname, Port(port), AddressFamily.IPv6)
+        except TransportAddressError:
+          return err(HttpAddressErrorType.NameLookupFailed)
+
+  if len(addresses) == 0:
+    return err(HttpAddressErrorType.NoAddressResolved)
+
+  ok(HttpAddress(id: id, scheme: scheme, hostname: hostname, port: port,
+                 path: url.path, query: url.query, anchor: url.anchor,
+                 username: url.username, password: url.password,
+                 addresses: addresses))
+
+proc getHttpAddress*(
+       url: string,
+       flags: HttpClientFlags = {}
+     ): HttpAddressResult {.raises: [].} =
+  getHttpAddress(parseUri(url), flags)
+
+proc getHttpAddress*(
+       session: HttpSessionRef,
+       url: Uri
+     ): HttpAddressResult {.raises: [].} =
+  getHttpAddress(url, session.flags)
+
+proc getHttpAddress*(
+       session: HttpSessionRef,
+       url: string
+     ): HttpAddressResult {.raises: [].} =
+  ## Create new HTTP address using URL string ``url`` and .
+  getHttpAddress(parseUri(url), session.flags)
+
 proc getAddress*(session: HttpSessionRef, url: Uri): HttpResult[HttpAddress] {.
      raises: [] .} =
   let scheme =

chronos/apps/http/httpcommon.nim

@@ -82,6 +82,48 @@ type
   HttpState* {.pure.} = enum
     Alive, Closing, Closed
 
+  HttpAddressErrorType* {.pure.} = enum
+    InvalidUrlScheme,
+    InvalidPortNumber,
+    MissingHostname,
+    InvalidIpHostname,
+    NameLookupFailed,
+    NoAddressResolved
+
+const
+  CriticalHttpAddressError* = {
+    HttpAddressErrorType.InvalidUrlScheme,
+    HttpAddressErrorType.InvalidPortNumber,
+    HttpAddressErrorType.MissingHostname,
+    HttpAddressErrorType.InvalidIpHostname
+  }
+
+  RecoverableHttpAddressError* = {
+    HttpAddressErrorType.NameLookupFailed,
+    HttpAddressErrorType.NoAddressResolved
+  }
+
+func isCriticalError*(error: HttpAddressErrorType): bool =
+  error in CriticalHttpAddressError
+
+func isRecoverableError*(error: HttpAddressErrorType): bool =
+  error in RecoverableHttpAddressError
+
+func toString*(error: HttpAddressErrorType): string =
+  case error
+  of HttpAddressErrorType.InvalidUrlScheme:
+    "URL scheme not supported"
+  of HttpAddressErrorType.InvalidPortNumber:
+    "Invalid URL port number"
+  of HttpAddressErrorType.MissingHostname:
+    "Missing URL hostname"
+  of HttpAddressErrorType.InvalidIpHostname:
+    "Invalid IPv4/IPv6 address in hostname"
+  of HttpAddressErrorType.NameLookupFailed:
+    "Could not resolve remote address"
+  of HttpAddressErrorType.NoAddressResolved:
+    "No address has been resolved"
+
 proc raiseHttpCriticalError*(msg: string,
                              code = Http400) {.noinline, noreturn.} =
   raise (ref HttpCriticalError)(code: code, msg: msg)

tests/testhttpclient.nim

@@ -1262,5 +1262,88 @@ suite "HTTP client testing suite":
   test "HTTP client server-sent events test":
     check waitFor(testServerSentEvents(false)) == true
 
+  test "HTTP getHttpAddress() test":
+    block:
+      # HTTP client supports only `http` and `https` schemes in URL.
+      let res = getHttpAddress("ftp://ftp.scene.org")
+      check:
+        res.isErr()
+        res.error == HttpAddressErrorType.InvalidUrlScheme
+        res.error.isCriticalError()
+    block:
+      # HTTP URL default ports and custom ports test
+      let
+        res1 = getHttpAddress("http://www.google.com")
+        res2 = getHttpAddress("https://www.google.com")
+        res3 = getHttpAddress("http://www.google.com:35000")
+        res4 = getHttpAddress("https://www.google.com:25000")
+      check:
+        res1.isOk()
+        res2.isOk()
+        res3.isOk()
+        res4.isOk()
+        res1.get().port == 80
+        res2.get().port == 443
+        res3.get().port == 35000
+        res4.get().port == 25000
+    block:
+      # HTTP URL invalid port values test
+      let
+        res1 = getHttpAddress("http://www.google.com:-80")
+        res2 = getHttpAddress("http://www.google.com:0")
+        res3 = getHttpAddress("http://www.google.com:65536")
+        res4 = getHttpAddress("http://www.google.com:65537")
+        res5 = getHttpAddress("https://www.google.com:-443")
+        res6 = getHttpAddress("https://www.google.com:0")
+        res7 = getHttpAddress("https://www.google.com:65536")
+        res8 = getHttpAddress("https://www.google.com:65537")
+      check:
+        res1.isErr() and res1.error == HttpAddressErrorType.InvalidPortNumber
+        res1.error.isCriticalError()
+        res2.isOk()
+        res2.get().port == 0
+        res3.isErr() and res3.error == HttpAddressErrorType.InvalidPortNumber
+        res3.error.isCriticalError()
+        res4.isErr() and res4.error == HttpAddressErrorType.InvalidPortNumber
+        res4.error.isCriticalError()
+        res5.isErr() and res5.error == HttpAddressErrorType.InvalidPortNumber
+        res5.error.isCriticalError()
+        res6.isOk()
+        res6.get().port == 0
+        res7.isErr() and res7.error == HttpAddressErrorType.InvalidPortNumber
+        res7.error.isCriticalError()
+        res8.isErr() and res8.error == HttpAddressErrorType.InvalidPortNumber
+        res8.error.isCriticalError()
+    block:
+      # HTTP URL missing hostname
+      let
+        res1 = getHttpAddress("http://")
+        res2 = getHttpAddress("https://")
+      check:
+        res1.isErr() and res1.error == HttpAddressErrorType.MissingHostname
+        res1.error.isCriticalError()
+        res2.isErr() and res2.error == HttpAddressErrorType.MissingHostname
+        res2.error.isCriticalError()
+    block:
+      # No resolution flags and incorrect URL
+      let
+        flags = {HttpClientFlag.NoInet4Resolution,
+                 HttpClientFlag.NoInet6Resolution}
+        res1 = getHttpAddress("http://256.256.256.256", flags)
+        res2 = getHttpAddress(
+          "http://[FFFFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF]", flags)
+      check:
+        res1.isErr() and res1.error == HttpAddressErrorType.InvalidIpHostname
+        res1.error.isCriticalError()
+        res2.isErr() and res2.error == HttpAddressErrorType.InvalidIpHostname
+        res2.error.isCriticalError()
+    block:
+      # Resolution of non-existent hostname
+      let res = getHttpAddress("http://eYr6bdBo.com")
+      check:
+        res.isErr() and res.error == HttpAddressErrorType.NameLookupFailed
+        res.error.isRecoverableError()
+        not(res.error.isCriticalError())
+
   test "Leaks test":
     checkLeaks()