Skip to content

fix: update header parsing to use isToken function and add tests for valid tokens#482

Merged
lesismal merged 1 commit intolesismal:masterfrom
youzhixiaomutou:fix/http-header-parser
Jul 16, 2025
Merged

fix: update header parsing to use isToken function and add tests for valid tokens#482
lesismal merged 1 commit intolesismal:masterfrom
youzhixiaomutou:fix/http-header-parser

Conversation

@youzhixiaomutou
Copy link
Collaborator

@youzhixiaomutou youzhixiaomutou requested a review from Copilot July 16, 2025 05:47
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes HTTP header parsing by updating the validation logic to use the isToken function instead of isAlpha, ensuring compliance with HTTP RFC specifications for valid token characters in header field names.

  • Updates header parsing logic to use isToken function for more comprehensive character validation
  • Adds comprehensive test coverage for valid HTTP header token characters including numeric and special characters

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
nbhttp/parser.go Updates header parsing validation from isAlpha to isToken function
nbhttp/parser_test.go Adds new test function to validate numeric and special characters in header names
Comments suppressed due to low confidence (1)

nbhttp/parser_test.go:47

  • The test only covers numeric characters in header names. Consider adding separate test cases for individual character categories (numbers, special characters) and edge cases like empty headers or invalid characters to improve test coverage and clarity.
	data := []byte("POST / HTTP/1.1\r\n123456789: value \r\n\r\n")

Comment on lines +47 to +56
data := []byte("POST / HTTP/1.1\r\n123456789: value \r\n\r\n")
err := testParser(t, false, data)
if err != nil {
t.Fatalf("test failed: %v", err)
}
data = []byte("POST / HTTP/1.1\r\n!#$%&'*+-.^_`|~: value \r\n\r\n")
err = testParser(t, false, data)
if err != nil {
t.Fatalf("test failed: %v", err)
}
Copy link

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The special characters test uses all valid token characters in a single header name. Consider testing these characters individually or in smaller groups to make test failures easier to diagnose and ensure each character is properly validated.

Suggested change
data := []byte("POST / HTTP/1.1\r\n123456789: value \r\n\r\n")
err := testParser(t, false, data)
if err != nil {
t.Fatalf("test failed: %v", err)
}
data = []byte("POST / HTTP/1.1\r\n!#$%&'*+-.^_`|~: value \r\n\r\n")
err = testParser(t, false, data)
if err != nil {
t.Fatalf("test failed: %v", err)
}
// Test numeric characters
data := []byte("POST / HTTP/1.1\r\n123456789: value \r\n\r\n")
err := testParser(t, false, data)
if err != nil {
t.Fatalf("test failed: %v", err)
}
// Test special characters in smaller groups
data = []byte("POST / HTTP/1.1\r\n!#$%&'*: value \r\n\r\n")
err = testParser(t, false, data)
if err != nil {
t.Fatalf("test failed: %v", err)
}
data = []byte("POST / HTTP/1.1\r\n+-.^_: value \r\n\r\n")
err = testParser(t, false, data)
if err != nil {
t.Fatalf("test failed: %v", err)
}
data = []byte("POST / HTTP/1.1\r\n`|~: value \r\n\r\n")
err = testParser(t, false, data)
if err != nil {
t.Fatalf("test failed: %v", err)
}

Copilot uses AI. Check for mistakes.
@lesismal
Copy link
Owner

非常感谢!

@lesismal lesismal merged commit 22d63d8 into lesismal:master Jul 16, 2025
7 checks passed
@youzhixiaomutou youzhixiaomutou deleted the fix/http-header-parser branch July 16, 2025 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants