From 3cee2249068b404d385dc7f77b04245e7461a86e Mon Sep 17 00:00:00 2001 From: "Danil Yarantsev (Yardanico)" Date: Mon, 30 Oct 2023 06:24:30 +0300 Subject: [PATCH 1/6] Fix `IndexDefect` errors in httpclient on invalid/weird headers --- lib/pure/httpclient.nim | 31 +++++++++++++++++++++---------- lib/pure/httpcore.nim | 2 +- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/pure/httpclient.nim b/lib/pure/httpclient.nim index d2bf925bab3c3..43b2d132d2a5a 100644 --- a/lib/pure/httpclient.nim +++ b/lib/pure/httpclient.nim @@ -856,6 +856,7 @@ proc parseResponse(client: HttpClient | AsyncHttpClient, var parsedStatus = false var linei = 0 var fullyRead = false + var lastHeaderName = "" var line = "" result.headers = newHttpHeaders() while true: @@ -890,16 +891,26 @@ proc parseResponse(client: HttpClient | AsyncHttpClient, parsedStatus = true else: # Parse headers - var name = "" - var le = parseUntil(line, name, ':', linei) - if le <= 0: httpError("invalid headers") - inc(linei, le) - if line[linei] != ':': httpError("invalid headers") - inc(linei) # Skip : - - result.headers.add(name, line[linei .. ^1].strip()) - if result.headers.len > headerLimit: - httpError("too many headers") + # There's at least one char because empty lines are handled above (with client.close) + if line[0] in {' ', '\t'}: + # Check if it's a multiline header value, if so, append to the header we're currently parsing + # This works because a line with a header must start with the header name without any leading space + # See https://datatracker.ietf.org/doc/html/rfc7230, section 3.2 and 3.2.4 + # Multiline headers are deprecated in the spec, but it's better to parse them than crash + result.headers.table[result.headers.toCaseInsensitive(lastHeaderName)][^1].add "\n" & line + else: + var name = "" + var le = parseUntil(line, name, ':', linei) + if le <= 0: httpError("Invalid header value - empty line received") + if line.len == le: httpError("Invalid header value - no colon after the header name") + inc(linei, le) # Skip the parsed header name + inc(linei) # Skip : + if linei >= line.len: httpError("Invalid header value - no header value after the colon") + # Remember the header name for the possible multi-line header + lastHeaderName = name + result.headers.add(name, line[linei .. ^1].strip()) + if result.headers.len > headerLimit: + httpError("too many headers") if not fullyRead: httpError("Connection was closed before full request has been made") diff --git a/lib/pure/httpcore.nim b/lib/pure/httpcore.nim index 3b5cbc3cfc9e5..93ed1590b4c1b 100644 --- a/lib/pure/httpcore.nim +++ b/lib/pure/httpcore.nim @@ -126,7 +126,7 @@ func toTitleCase(s: string): string = result[i] = if upper: toUpperAscii(s[i]) else: toLowerAscii(s[i]) upper = s[i] == '-' -func toCaseInsensitive(headers: HttpHeaders, s: string): string {.inline.} = +func toCaseInsensitive*(headers: HttpHeaders, s: string): string {.inline.} = return if headers.isTitleCase: toTitleCase(s) else: toLowerAscii(s) func newHttpHeaders*(titleCase=false): HttpHeaders = From 832813eb167ee246d6d2d15e0cd14ac8e1c8a3fc Mon Sep 17 00:00:00 2001 From: "Danil Yarantsev (Yardanico)" Date: Mon, 30 Oct 2023 06:59:38 +0300 Subject: [PATCH 2/6] linei can't be larger than line.len --- lib/pure/httpclient.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pure/httpclient.nim b/lib/pure/httpclient.nim index 43b2d132d2a5a..8c8c42cede294 100644 --- a/lib/pure/httpclient.nim +++ b/lib/pure/httpclient.nim @@ -905,7 +905,7 @@ proc parseResponse(client: HttpClient | AsyncHttpClient, if line.len == le: httpError("Invalid header value - no colon after the header name") inc(linei, le) # Skip the parsed header name inc(linei) # Skip : - if linei >= line.len: httpError("Invalid header value - no header value after the colon") + if linei == line.len: httpError("Invalid header value - no header value after the colon") # Remember the header name for the possible multi-line header lastHeaderName = name result.headers.add(name, line[linei .. ^1].strip()) From ac25e375552e63d19366151a57a4325b3332024d Mon Sep 17 00:00:00 2001 From: "Danil Yarantsev (Yardanico)" Date: Mon, 30 Oct 2023 07:15:53 +0300 Subject: [PATCH 3/6] Fix the case of a multiline header value without a header, change error messages --- lib/pure/httpclient.nim | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/pure/httpclient.nim b/lib/pure/httpclient.nim index 8c8c42cede294..5fe617bceb9f8 100644 --- a/lib/pure/httpclient.nim +++ b/lib/pure/httpclient.nim @@ -897,15 +897,21 @@ proc parseResponse(client: HttpClient | AsyncHttpClient, # This works because a line with a header must start with the header name without any leading space # See https://datatracker.ietf.org/doc/html/rfc7230, section 3.2 and 3.2.4 # Multiline headers are deprecated in the spec, but it's better to parse them than crash - result.headers.table[result.headers.toCaseInsensitive(lastHeaderName)][^1].add "\n" & line + if lastHeaderName == "": + # This should ideally throw an error, but old httpclient actually treated lines like this + # as a header with an empty name and didn't crash + #httpError("Invalid headers - received multiline header value without previous header") + discard + else: + result.headers.table[result.headers.toCaseInsensitive(lastHeaderName)][^1].add "\n" & line else: var name = "" var le = parseUntil(line, name, ':', linei) - if le <= 0: httpError("Invalid header value - empty line received") - if line.len == le: httpError("Invalid header value - no colon after the header name") + if le <= 0: httpError("Invalid headers - received empty header name") + if line.len == le: httpError("Invalid headers - no colon after header name") inc(linei, le) # Skip the parsed header name inc(linei) # Skip : - if linei == line.len: httpError("Invalid header value - no header value after the colon") + if linei == line.len: httpError("Invalid headers - no header value after colon") # Remember the header name for the possible multi-line header lastHeaderName = name result.headers.add(name, line[linei .. ^1].strip()) From 74c629d407c4aa6a862fc7553e3899e0e272bd04 Mon Sep 17 00:00:00 2001 From: "Danil Yarantsev (Yardanico)" Date: Mon, 30 Oct 2023 07:34:07 +0300 Subject: [PATCH 4/6] Don't refuse headers with empty value --- lib/pure/httpclient.nim | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pure/httpclient.nim b/lib/pure/httpclient.nim index 5fe617bceb9f8..4cbc8ea3949a3 100644 --- a/lib/pure/httpclient.nim +++ b/lib/pure/httpclient.nim @@ -911,7 +911,9 @@ proc parseResponse(client: HttpClient | AsyncHttpClient, if line.len == le: httpError("Invalid headers - no colon after header name") inc(linei, le) # Skip the parsed header name inc(linei) # Skip : - if linei == line.len: httpError("Invalid headers - no header value after colon") + # Here linei is smaller or equal to line.len, so the slice below should work fine + # If we want to enforce the HTTP spec later, do this: + #if linei == line.len: httpError("Invalid headers - no header value after colon") # Remember the header name for the possible multi-line header lastHeaderName = name result.headers.add(name, line[linei .. ^1].strip()) From bad312ce0645e8a38abaeb24e15cb80889096a19 Mon Sep 17 00:00:00 2001 From: "Danil Yarantsev (Yardanico)" Date: Tue, 31 Oct 2023 07:32:08 +0300 Subject: [PATCH 5/6] Update comments from discussion --- lib/pure/httpclient.nim | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/pure/httpclient.nim b/lib/pure/httpclient.nim index 4cbc8ea3949a3..fc66b96f522f5 100644 --- a/lib/pure/httpclient.nim +++ b/lib/pure/httpclient.nim @@ -898,9 +898,7 @@ proc parseResponse(client: HttpClient | AsyncHttpClient, # See https://datatracker.ietf.org/doc/html/rfc7230, section 3.2 and 3.2.4 # Multiline headers are deprecated in the spec, but it's better to parse them than crash if lastHeaderName == "": - # This should ideally throw an error, but old httpclient actually treated lines like this - # as a header with an empty name and didn't crash - #httpError("Invalid headers - received multiline header value without previous header") + # Some extra unparsable lines in the HTTP output - we ignore them discard else: result.headers.table[result.headers.toCaseInsensitive(lastHeaderName)][^1].add "\n" & line @@ -911,11 +909,8 @@ proc parseResponse(client: HttpClient | AsyncHttpClient, if line.len == le: httpError("Invalid headers - no colon after header name") inc(linei, le) # Skip the parsed header name inc(linei) # Skip : - # Here linei is smaller or equal to line.len, so the slice below should work fine - # If we want to enforce the HTTP spec later, do this: - #if linei == line.len: httpError("Invalid headers - no header value after colon") - # Remember the header name for the possible multi-line header - lastHeaderName = name + # If we want to be HTTP spec compliant later, error on linei == line.len (for empty header value) + lastHeaderName = name # Remember the header name for the possible multi-line header result.headers.add(name, line[linei .. ^1].strip()) if result.headers.len > headerLimit: httpError("too many headers") From f3c582221c10c1b3b127389591d23faa69f09493 Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Wed, 1 Nov 2023 08:01:17 +0100 Subject: [PATCH 6/6] Update lib/pure/httpcore.nim --- lib/pure/httpcore.nim | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/pure/httpcore.nim b/lib/pure/httpcore.nim index 93ed1590b4c1b..5c5104bda7275 100644 --- a/lib/pure/httpcore.nim +++ b/lib/pure/httpcore.nim @@ -127,6 +127,7 @@ func toTitleCase(s: string): string = upper = s[i] == '-' func toCaseInsensitive*(headers: HttpHeaders, s: string): string {.inline.} = + ## For internal usage only. Do not use. return if headers.isTitleCase: toTitleCase(s) else: toLowerAscii(s) func newHttpHeaders*(titleCase=false): HttpHeaders =