From 300fbaaf09cf8cc8d3798daa328d90d015906623 Mon Sep 17 00:00:00 2001 From: Eugene Kabanov Date: Mon, 4 Sep 2023 21:49:45 +0300 Subject: [PATCH] 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 | 85 ++++++++++++++++++++++++++++++++ chronos/apps/http/httpcommon.nim | 42 ++++++++++++++++ tests/testhttpclient.nim | 83 +++++++++++++++++++++++++++++++ 3 files changed, 210 insertions(+) diff --git a/chronos/apps/http/httpclient.nim b/chronos/apps/http/httpclient.nim index 63ffc37..b4b3202 100644 --- a/chronos/apps/http/httpclient.nim +++ b/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 = diff --git a/chronos/apps/http/httpcommon.nim b/chronos/apps/http/httpcommon.nim index 5a4a628..c01c1c3 100644 --- a/chronos/apps/http/httpcommon.nim +++ b/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) diff --git a/tests/testhttpclient.nim b/tests/testhttpclient.nim index 1eacc21..4daaf87 100644 --- a/tests/testhttpclient.nim +++ b/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()