From 939195626f371048622936e50bf1b1c18979653d Mon Sep 17 00:00:00 2001 From: Eugene Kabanov Date: Fri, 5 Aug 2022 19:59:26 +0300 Subject: [PATCH] Use new Content-Type header value parser. (#302) --- chronos/apps/http/httpclient.nim | 15 +++++++- chronos/apps/http/httpcommon.nim | 12 ++++--- chronos/apps/http/httpserver.nim | 28 ++++++++------- chronos/apps/http/multipart.nim | 60 ++++++++------------------------ tests/testhttpserver.nim | 45 +++++++++++++++--------- 5 files changed, 79 insertions(+), 81 deletions(-) diff --git a/chronos/apps/http/httpclient.nim b/chronos/apps/http/httpclient.nim index d75e6f2a..6e261e89 100644 --- a/chronos/apps/http/httpclient.nim +++ b/chronos/apps/http/httpclient.nim @@ -157,6 +157,7 @@ type contentEncoding*: set[ContentEncodingFlags] transferEncoding*: set[TransferEncodingFlags] contentLength*: uint64 + contentType*: Opt[ContentTypeData] HttpClientResponseRef* = ref HttpClientResponse @@ -783,13 +784,25 @@ proc prepareResponse(request: HttpClientRequestRef, data: openArray[byte] else: false + let contentType = + block: + let list = headers.getList(ContentTypeHeader) + if len(list) > 0: + let res = getContentType(list) + if res.isErr(): + return err("Invalid headers received, invalid `Content-Type`") + else: + Opt.some(res.get()) + else: + Opt.none(ContentTypeData) + let res = HttpClientResponseRef( state: HttpReqRespState.Open, status: resp.code, address: request.address, requestMethod: request.meth, reason: resp.reason(data), version: resp.version, session: request.session, connection: request.connection, headers: headers, contentEncoding: contentEncoding, transferEncoding: transferEncoding, - contentLength: contentLength, bodyFlag: bodyFlag + contentLength: contentLength, contentType: contentType, bodyFlag: bodyFlag ) res.connection.state = HttpClientConnectionState.ResponseHeadersReceived if nobodyFlag: diff --git a/chronos/apps/http/httpcommon.nim b/chronos/apps/http/httpcommon.nim index 3f01e674..0423f222 100644 --- a/chronos/apps/http/httpcommon.nim +++ b/chronos/apps/http/httpcommon.nim @@ -32,8 +32,8 @@ const LocationHeader* = "location" AuthorizationHeader* = "authorization" - UrlEncodedContentType* = "application/x-www-form-urlencoded" - MultipartContentType* = "multipart/form-data" + UrlEncodedContentType* = MediaType.init("application/x-www-form-urlencoded") + MultipartContentType* = MediaType.init("multipart/form-data") type HttpResult*[T] = Result[T, string] @@ -193,7 +193,7 @@ func getContentEncoding*(ch: openArray[string]): HttpResult[ return err("Incorrect Content-Encoding value") ok(res) -func getContentType*(ch: openArray[string]): HttpResult[string] {. +func getContentType*(ch: openArray[string]): HttpResult[ContentTypeData] {. raises: [Defect].} = ## Check and prepare value of ``Content-Type`` header. if len(ch) == 0: @@ -201,8 +201,10 @@ func getContentType*(ch: openArray[string]): HttpResult[string] {. elif len(ch) > 1: err("Multiple Content-Type values found") else: - let mparts = ch[0].split(";") - ok(strip(mparts[0]).toLowerAscii()) + let res = getContentType(ch[0]) + if res.isErr(): + return err($res.error()) + ok(res.get()) proc bytesToString*(src: openArray[byte], dst: var openArray[char]) = ## Convert array of bytes to array of characters. diff --git a/chronos/apps/http/httpserver.nim b/chronos/apps/http/httpserver.nim index 8535b107..f9a87a93 100644 --- a/chronos/apps/http/httpserver.nim +++ b/chronos/apps/http/httpserver.nim @@ -98,6 +98,7 @@ type transferEncoding*: set[TransferEncodingFlags] requestFlags*: set[HttpRequestFlags] contentLength: int + contentTypeData*: Option[ContentTypeData] connection*: HttpConnectionRef response*: Option[HttpResponseRef] @@ -324,9 +325,10 @@ proc prepareRequest(conn: HttpConnectionRef, # steps to reveal information about body. if ContentLengthHeader in request.headers: let length = request.headers.getInt(ContentLengthHeader) - if length > 0: + if length >= 0: if request.meth == MethodTrace: return err(Http400) + # Because of coversion to `int` we should avoid unexpected OverflowError. if length > uint64(high(int)): return err(Http413) if length > uint64(conn.server.maxRequestBodySize): @@ -342,12 +344,14 @@ proc prepareRequest(conn: HttpConnectionRef, if request.hasBody(): # If request has body, we going to understand how its encoded. if ContentTypeHeader in request.headers: - let contentType = request.headers.getString(ContentTypeHeader) - let tmp = strip(contentType).toLowerAscii() - if tmp.startsWith(UrlEncodedContentType): + let contentType = + getContentType(request.headers.getList(ContentTypeHeader)).valueOr: + return err(Http415) + if contentType == UrlEncodedContentType: request.requestFlags.incl(HttpRequestFlags.UrlencodedForm) - elif tmp.startsWith(MultipartContentType): + elif contentType == MultipartContentType: request.requestFlags.incl(HttpRequestFlags.MultipartForm) + request.contentTypeData = some(contentType) if ExpectHeader in request.headers: let expectHeader = request.headers.getString(ExpectHeader) @@ -899,19 +903,17 @@ proc join*(server: HttpServerRef): Future[void] = retFuture -proc getMultipartReader*(req: HttpRequestRef): HttpResult[MultiPartReaderRef] = +proc getMultipartReader*(req: HttpRequestRef): HttpResult[MultiPartReaderRef] {. + raises: [Defect].} = ## Create new MultiPartReader interface for specific request. if req.meth in PostMethods: if MultipartForm in req.requestFlags: - let ctype = ? getContentType(req.headers.getList(ContentTypeHeader)) - if ctype != MultipartContentType: - err("Content type is not supported") - else: - let boundary = ? getMultipartBoundary( - req.headers.getList(ContentTypeHeader) - ) + if req.contentTypeData.isSome(): + let boundary = ? getMultipartBoundary(req.contentTypeData.get()) var stream = ? req.getBodyReader() ok(MultiPartReaderRef.new(stream, boundary)) + else: + err("Content type is missing or invalid") else: err("Request's data is not multipart encoded") else: diff --git a/chronos/apps/http/multipart.nim b/chronos/apps/http/multipart.nim index 5c45a6b7..971c0009 100644 --- a/chronos/apps/http/multipart.nim +++ b/chronos/apps/http/multipart.nim @@ -8,11 +8,11 @@ # Apache License, version 2.0, (LICENSE-APACHEv2) # MIT license (LICENSE-MIT) import std/[monotimes, strutils] -import stew/results +import stew/results, httputils import ../../asyncloop import ../../streams/[asyncstream, boundstream, chunkstream] import httptable, httpcommon, httpbodyrw -export asyncloop, httptable, httpcommon, httpbodyrw, asyncstream +export asyncloop, httptable, httpcommon, httpbodyrw, asyncstream, httputils const UnableToReadMultipartBody = "Unable to read multipart message body" @@ -439,55 +439,25 @@ func validateBoundary[B: BChar](boundary: openArray[B]): HttpResult[void] = return err("Content-Type boundary alphabet incorrect") ok() -func getMultipartBoundary*(ch: openArray[string]): HttpResult[string] {. +func getMultipartBoundary*(contentData: ContentTypeData): HttpResult[string] {. raises: [Defect].} = ## Returns ``multipart/form-data`` boundary value from ``Content-Type`` ## header. ## ## The procedure carries out all the necessary checks: - ## 1) There should be single `Content-Type` header value in headers. - ## 2) `Content-Type` must be ``multipart/form-data``. - ## 3) `boundary` value must be present - ## 4) `boundary` value must be less then 70 characters length and + ## 1) `boundary` value must be present. + ## 2) `boundary` value must be less then 70 characters length and ## all characters should be part of specific alphabet. - if len(ch) > 1: - err("Multiple Content-Type headers found") - else: - if len(ch) == 0: - err("Content-Type header is missing") - else: - if len(ch[0]) == 0: - return err("Content-Type header has empty value") - let mparts = ch[0].split(";") - if strip(mparts[0]).toLowerAscii() != "multipart/form-data": - return err("Content-Type is not multipart") - if len(mparts) < 2: - return err("Content-Type missing boundary value") - - let index = - block: - var idx = 0 - for i in 1 ..< len(mparts): - let stripped = strip(mparts[i]) - if stripped.toLowerAscii().startsWith("boundary="): - idx = i - break - idx - - if index == 0: - err("Missing Content-Type boundary key") - else: - let stripped = strip(mparts[index]) - let bparts = stripped.split("=", 1) - if len(bparts) < 2: - err("Missing Content-Type boundary") - else: - let candidate = strip(bparts[1]) - let res = validateBoundary(candidate) - if res.isErr(): - err($res.error()) - else: - ok(candidate) + let candidate = + block: + var res: string + for item in contentData.params: + if cmpIgnoreCase(item.name, "boundary") == 0: + res = item.value + break + res + ? validateBoundary(candidate) + ok(candidate) proc quoteCheck(name: string): HttpResult[string] = if len(name) > 0: diff --git a/tests/testhttpserver.nim b/tests/testhttpserver.nim index eb3ccce0..1b311dcc 100644 --- a/tests/testhttpserver.nim +++ b/tests/testhttpserver.nim @@ -610,39 +610,50 @@ suite "HTTP server testing suite": "--------------------------------------------------; charset=UTF-8", "-----------------------------------------------------------------" & "-----"), - ("multipart/form-data; boundary=ABCDEFGHIJKLMNOPQRST" & - "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+_,-.; charset=UTF-8", + ("multipart/form-data; boundary=\"ABCDEFGHIJKLMNOPQRST" & + "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+_,-.\"; charset=UTF-8", "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()" & "+_,-."), - ("multipart/form-data; boundary=ABCDEFGHIJKLMNOPQRST" & - "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+?=:/; charset=UTF-8", + ("multipart/form-data; boundary=\"ABCDEFGHIJKLMNOPQRST" & + "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+?=:/\"; charset=UTF-8", "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()" & "+?=:/"), - ("multipart/form-data; charset=UTF-8; boundary=ABCDEFGHIJKLMNOPQRST" & - "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+_,-.", + ("multipart/form-data; charset=UTF-8; boundary=\"ABCDEFGHIJKLMNOPQRST" & + "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+_,-.\"", "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()" & "+_,-."), - ("multipart/form-data; charset=UTF-8; boundary=ABCDEFGHIJKLMNOPQRST" & - "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+?=:/", + ("multipart/form-data; charset=UTF-8; boundary=\"ABCDEFGHIJKLMNOPQRST" & + "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+?=:/\"", "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()" & - "+?=:/") + "+?=:/"), + ("multipart/form-data; charset=UTF-8; boundary=0123456789ABCDEFGHIJKL" & + "MNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz+-", + "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz+-"), + ("multipart/form-data; boundary=0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZa" & + "bcdefghijklmnopqrstuvwxyz+-; charset=UTF-8", + "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz+-") ] + proc performCheck(ch: openArray[string]): HttpResult[string] = + let cdata = ? getContentType(ch) + if cdata.mediaType != MediaType.init("multipart/form-data"): + return err("Invalid media type") + getMultipartBoundary(cdata) for i in 0 ..< 256: - let boundary = "multipart/form-data; boundary=" & $char(i) + let boundary = "multipart/form-data; boundary=\"" & $char(i) & "\"" if char(i) in AllowedCharacters: - check getMultipartBoundary([boundary]).isOk() + check performCheck([boundary]).isOk() else: - check getMultipartBoundary([boundary]).isErr() + check performCheck([boundary]).isErr() check: - getMultipartBoundary([]).isErr() - getMultipartBoundary(["multipart/form-data; boundary=A", - "multipart/form-data; boundary=B"]).isErr() + performCheck([]).isErr() + performCheck(["multipart/form-data; boundary=A", + "multipart/form-data; boundary=B"]).isErr() for item in FailureVectors: - check getMultipartBoundary([item]).isErr() + check performCheck([item]).isErr() for item in SuccessVectors: - let res = getMultipartBoundary([item[0]]) + let res = performCheck([item[0]]) check: res.isOk() item[1] == res.get()