Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpAddress errors should be not only critical. #446

Merged
merged 3 commits into from
Sep 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions chronos/apps/http/httpclient.nim
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ type
name*: string
data*: string

HttpAddressResult* = Result[HttpAddress, HttpAddressErrorType]

# HttpClientRequestRef valid states are:
# Ready -> Open -> (Finished, Error) -> (Closing, Closed)
#
Expand Down Expand Up @@ -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 =
Expand Down
42 changes: 42 additions & 0 deletions chronos/apps/http/httpcommon.nim
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,48 @@ type
HttpState* {.pure.} = enum
Alive, Closing, Closed

HttpAddressErrorType* {.pure.} = enum
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or list specific error kinds we interpret then have a set of critical ones, ie Critical = {ParseError, XxxErro}, Recoverable = {LookupError, ...}

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)
Expand Down
83 changes: 83 additions & 0 deletions tests/testhttpclient.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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()