Skip to content

Commit

Permalink
Improve HTTP Request line validation (#59)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
vbanos authored Jan 20, 2025
1 parent 79e5fb7 commit 8539cbe
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 13 deletions.
2 changes: 1 addition & 1 deletion dialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down
24 changes: 12 additions & 12 deletions utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
20 changes: 20 additions & 0 deletions utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

0 comments on commit 8539cbe

Please sign in to comment.