From 8539cbee5b300df13ebd683e81b75f87912628f1 Mon Sep 17 00:00:00 2001
From: Vangelis Banos <vbanos@gmail.com>
Date: Mon, 20 Jan 2025 11:05:07 +0200
Subject: [PATCH] Improve HTTP Request line validation (#59)

* Improve HTTP Request line validation

Refactor `isLineStartingWithHTTPMethod` to include all the logic that is
necessary to validate an HTTP request (prefix and suffix).

Previously, some of the logic was in `isLineStartingWithHTTPMethod` and
some was in `dialer.go` where the function was used.

Rename it to `isHTTPRequest`. (IMHO `isHTTPRequestLine` is redundant as
`line` is the function param.

Add unit tests.
---
 dialer.go     |  2 +-
 utils.go      | 24 ++++++++++++------------
 utils_test.go | 20 ++++++++++++++++++++
 3 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/dialer.go b/dialer.go
index fa00fd1..cc64cb2 100644
--- a/dialer.go
+++ b/dialer.go
@@ -553,7 +553,7 @@ func (d *customDialer) readRequest(scheme string, reqPipe *io.PipeReader, target
 		n, err := requestRecord.Content.Read(block)
 		if n > 0 {
 			if string(block) == "\n" {
-				if isLineStartingWithHTTPMethod(line) && (strings.HasSuffix(line, "HTTP/1.0\r") || strings.HasSuffix(line, "HTTP/1.1\r")) {
+				if isHTTPRequest(line) {
 					target = strings.Split(line, " ")[1]
 
 					if host != "" && target != "" {
diff --git a/utils.go b/utils.go
index d2922fe..acb194f 100644
--- a/utils.go
+++ b/utils.go
@@ -98,19 +98,19 @@ func splitKeyValue(line string) (string, string) {
 	return parts[0], strings.TrimSpace(parts[1])
 }
 
-func isLineStartingWithHTTPMethod(line string) bool {
-	if strings.HasPrefix(line, "GET ") ||
-		strings.HasPrefix(line, "HEAD ") ||
-		strings.HasPrefix(line, "POST ") ||
-		strings.HasPrefix(line, "PUT ") ||
-		strings.HasPrefix(line, "DELETE ") ||
-		strings.HasPrefix(line, "CONNECT ") ||
-		strings.HasPrefix(line, "OPTIONS ") ||
-		strings.HasPrefix(line, "TRACE ") ||
-		strings.HasPrefix(line, "PATCH ") {
-		return true
+func isHTTPRequest(line string) bool {
+	httpMethods := []string{"GET ", "HEAD ", "POST ", "PUT ", "DELETE ", "CONNECT ", "OPTIONS ", "TRACE ", "PATCH "}
+	protocols := []string{"HTTP/1.0\r", "HTTP/1.1\r"}
+
+	for _, method := range httpMethods {
+		if strings.HasPrefix(line, method) {
+			for _, protocol := range protocols {
+				if strings.HasSuffix(line, protocol) {
+					return true
+				}
+			}
+		}
 	}
-
 	return false
 }
 
diff --git a/utils_test.go b/utils_test.go
index 579617a..fbf955f 100644
--- a/utils_test.go
+++ b/utils_test.go
@@ -44,3 +44,23 @@ func TestNewRotatorSettings(t *testing.T) {
 		t.Error("Failed to set WARC rotator's compression dictionary")
 	}
 }
+
+// Tests for the isLineStartingWithHTTPMethod function
+func TestIsHTTPRequest(t *testing.T) {
+	defer goleak.VerifyNone(t)
+
+	goodHTTPRequestHeaders := []string{
+		"GET /index.html HTTP/1.1\r",
+		"POST /api/login HTTP/1.1\r",
+		"DELETE /api/products/456 HTTP/1.1\r",
+		"HEAD /about HTTP/1.0\r",
+		"OPTIONS / HTTP/1.1\r",
+		"PATCH /api/item/789 HTTP/1.1\r",
+		"GET /images/logo.png HTTP/1.1\r",
+	}
+	for _, header := range goodHTTPRequestHeaders {
+		if !isHTTPRequest(header) {
+			t.Error("Invalid HTTP Method parsing:", header)
+		}
+	}
+}