-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: zhttp supports more complex content type strings #99
Conversation
WalkthroughThe changes update the request handling in the zhttp package to improve Content-Type header parsing. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RequestFunc as Request()
participant StrCut as strings.Cut
participant Parser
Client->>RequestFunc: Send HTTP request with Content-Type header
RequestFunc->>StrCut: Extract media type from header
StrCut-->>RequestFunc: Return media type (e.g., "application/json")
RequestFunc->>Parser: Route request based on extracted media type
Parser-->>RequestFunc: Process request accordingly
RequestFunc-->>Client: Return parsed response
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
zhttp/zhttp.go (1)
79-81
: Consider enhancing Content-Type parsing robustness.The changes correctly handle complex Content-Type strings by following RFC 2045 format. However, consider these improvements:
- Add error handling for malformed Content-Type headers
- Use constants for content type strings
+const ( + contentTypeJSON = "application/json" + contentTypeForm = "application/x-www-form-urlencoded" +) func Request(r *http.Request) p.DpFactory { // Content-Type follows this format: Content-Type: <media-type> [; parameter=value] - typ, _, _ := strings.Cut(r.Header.Get("Content-Type"), ";") + contentType := r.Header.Get("Content-Type") + if contentType == "" { + return Config.Parsers.Query(r) + } + typ, _, found := strings.Cut(contentType, ";") + if !found { + typ = contentType + } switch typ { - case "application/json": + case contentTypeJSON: return Config.Parsers.JSON(r) - case "application/x-www-form-urlencoded": + case contentTypeForm: return Config.Parsers.Form(r) default: return Config.Parsers.Query(r) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
zhttp/zhttp.go
(2 hunks)zhttp/zhttp_test.go
(1 hunks)
🔇 Additional comments (2)
zhttp/zhttp.go (1)
6-6
: LGTM!The strings import is required for the new Content-Type parsing logic.
zhttp/zhttp_test.go (1)
195-204
: LGTM! Good test coverage for complex Content-Type handling.The test effectively validates that JSON requests with charset specifications are correctly handled. It follows the established test patterns and provides good coverage for the new functionality.
Addresses #95
Summary by CodeRabbit