From 3495122867f7b31be92212c5b1199dbb3b3738ac Mon Sep 17 00:00:00 2001 From: cheatfate Date: Wed, 10 Feb 2021 12:37:56 +0200 Subject: [PATCH] Fix getMultipartBoundary() issues and add tests for it. --- chronos/apps/http/multipart.nim | 45 +++++++++++++------- tests/testhttpserver.nim | 74 +++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 14 deletions(-) diff --git a/chronos/apps/http/multipart.nim b/chronos/apps/http/multipart.nim index 347ae2ad..0181f74a 100644 --- a/chronos/apps/http/multipart.nim +++ b/chronos/apps/http/multipart.nim @@ -423,19 +423,36 @@ func getMultipartBoundary*(ch: openarray[string]): HttpResult[string] = return err("Content-Type is not multipart") if len(mparts) < 2: return err("Content-Type missing boundary value") - let stripped = strip(mparts[1]) - if not(stripped.toLowerAscii().startsWith("boundary")): - return err("Incorrect Content-Type boundary format") - let bparts = stripped.split("=") - if len(bparts) < 2: - err("Missing Content-Type boundary") + + 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 candidate = strip(bparts[1]) - if len(candidate) > 70: - err("Content-Type boundary must be less then 70 characters") + let stripped = strip(mparts[index]) + let bparts = stripped.split("=", 1) + if len(bparts) < 2: + err("Missing Content-Type boundary") else: - for ch in candidate: - if ch notin {'a'..'z', 'A' .. 'Z', '0' .. '9', - '\'' .. ')', '+' .. '/', ':', '=', '?', '_'}: - return err("Content-Type boundary alphabat incorrect") - ok(candidate) + if bparts[0].toLowerAscii() != "boundary": + err("Missing boundary key") + else: + let candidate = strip(bparts[1]) + if len(candidate) == 0: + err("Content-Type boundary must be at least 1 character size") + elif len(candidate) > 70: + err("Content-Type boundary must be less then 70 characters") + else: + for ch in candidate: + if ch notin {'a' .. 'z', 'A' .. 'Z', '0' .. '9', + '\'' .. ')', '+' .. '/', ':', '=', '?', '_'}: + return err("Content-Type boundary alphabet incorrect") + ok(candidate) diff --git a/tests/testhttpserver.nim b/tests/testhttpserver.nim index 0fb0b60b..149489fd 100644 --- a/tests/testhttpserver.nim +++ b/tests/testhttpserver.nim @@ -680,6 +680,80 @@ suite "HTTP server testing suite": check waitFor(testHTTPS2(initTAddress("127.0.0.1:30080"))) == true + + test "Content-Type multipart boundary test": + const AllowedCharacters = { + 'a' .. 'z', 'A' .. 'Z', '0' .. '9', + '\'', '(', ')', '+', '_', ',', '-', '.' ,'/', ':', '=', '?' + } + + const FailureVectors = [ + "", + "multipart/byteranges; boundary=A", + "multipart/form-data;", + "multipart/form-data; boundary", + "multipart/form-data; boundary=", + "multipart/form-data; boundaryMore=A", + "multipart/form-data; charset=UTF-8; boundary", + "multipart/form-data; charset=UTF-8; boundary=", + "multipart/form-data; charset=UTF-8; boundary =", + "multipart/form-data; charset=UTF-8; boundary= ", + "multipart/form-data; charset=UTF-8; boundaryMore=", + "multipart/form-data; charset=UTF-8; boundaryMore=A", + "multipart/form-data; charset=UTF-8; boundaryMore=AAAAAAAAAAAAAAAAAAAA" & + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + ] + + const SuccessVectors = [ + ("multipart/form-data; boundary=A", "A"), + ("multipart/form-data; charset=UTF-8; boundary=B", "B"), + ("multipart/form-data; charset=UTF-8; boundary=--------------------" & + "--------------------------------------------------", "-----------" & + "-----------------------------------------------------------"), + ("multipart/form-data; boundary=--------------------" & + "--------------------------------------------------", "-----------" & + "-----------------------------------------------------------"), + ("multipart/form-data; boundary=--------------------" & + "--------------------------------------------------; charset=UTF-8", + "-----------------------------------------------------------------" & + "-----"), + ("multipart/form-data; boundary=ABCDEFGHIJKLMNOPQRST" & + "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+_,-.; charset=UTF-8", + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()" & + "+_,-."), + ("multipart/form-data; boundary=ABCDEFGHIJKLMNOPQRST" & + "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+?=:/; charset=UTF-8", + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()" & + "+?=:/"), + ("multipart/form-data; charset=UTF-8; boundary=ABCDEFGHIJKLMNOPQRST" & + "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+_,-.", + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()" & + "+_,-."), + ("multipart/form-data; charset=UTF-8; boundary=ABCDEFGHIJKLMNOPQRST" & + "UVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()+?=:/", + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'()" & + "+?=:/") + ] + + for i in 0 ..< 256: + let boundary = "multipart/form-data; boundary=" & $char(i) + if char(i) in AllowedCharacters: + check getMultipartBoundary([boundary]).isOk() + else: + check getMultipartBoundary([boundary]).isErr() + + check: + getMultipartBoundary([]).isErr() + getMultipartBoundary(["multipart/form-data; boundary=A", + "multipart/form-data; boundary=B"]).isErr() + for item in FailureVectors: + check getMultipartBoundary([item]).isErr() + for item in SuccessVectors: + let res = getMultipartBoundary([item[0]]) + check: + res.isOk() + item[1] == res.get() + test "Leaks test": check: getTracker("async.stream.reader").isLeaked() == false