-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(raw): handle full URLs in unsafe raw requests #6589
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(raw): handle full URLs in unsafe raw requests #6589
Conversation
Previously, when using `unsafe: true` with full URLs (e.g., `GET http://example.com/path HTTP/1.1`), the `Parse` func would treat the full URL as a relative path, resulting in malformed requests like `GET /http://example.com/path HTTP/1.1`. This occurred because the full URL handling logic was only executed for non-unsafe requests, causing unsafe requests with full URLs to fall through to the unsafe case which wasn't designed to handle them. Changes: * Extract full URL handling before mode-specific logic runs. * Convert full URLs to relative paths for both safe and unsafe modes. * Update `UnsafeRawBytes` with the correct relative path when unsafe is true. * Ensure path merging works correctly with `disable-path-automerge`. This fix maintains backward compatibility while properly supporting the previously broken combination of unsafe mode with full URLs. Fixes #6558. Signed-off-by: Dwi Siswanto <[email protected]>
WalkthroughThe Parse function in the HTTP raw protocol handler is refactored to handle full URLs upfront: when a path begins with Changes
Sequence Diagram(s)sequenceDiagram
participant Parse as Parse Function
participant URL as URL Parser
participant Path as Path Handler
Parse->>Parse: Check if Path starts with http:// or https://
alt Full URL detected
Parse->>URL: Parse full URL
URL->>Parse: Return parsed components
Parse->>Path: Extract relative path
alt Unsafe mode
Parse->>Parse: Replace path in UnsafeRawBytes
end
Parse->>Parse: Set Path to relative path
end
Parse->>Parse: Continue with main switch logic (non-URL branches)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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)
pkg/protocols/http/raw/raw.go (1)
41-58: Excellent refactoring that consolidates full URL handling.This upfront preprocessing correctly extracts relative paths from full URLs before the switch logic, fixing issue #6558. The approach ensures both safe and unsafe modes handle full URLs consistently.
The use of
bytes.Replace(..., 1)on line 53 is appropriate since the path appears first in the request line (e.g., "GET http://example.com/path HTTP/1.1"), so only the first occurrence is replaced. This preserves any other occurrences that might appear in headers or body.Consider adding a brief comment on line 53 explaining why
count=1is used:if unsafe { + // Replace only the first occurrence (in the request line), preserving any other occurrences in headers/body rawrequest.UnsafeRawBytes = bytes.Replace(rawrequest.UnsafeRawBytes, []byte(prevPath), []byte(relPath), 1) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/protocols/http/raw/raw.go(1 hunks)pkg/protocols/http/raw/raw_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Format Go code using go fmt
Run static analysis with go vet
Files:
pkg/protocols/http/raw/raw.gopkg/protocols/http/raw/raw_test.go
pkg/protocols/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Each protocol implementation must provide a Request interface with methods Compile(), ExecuteWithResults(), Match(), and Extract()
Files:
pkg/protocols/http/raw/raw.gopkg/protocols/http/raw/raw_test.go
🧠 Learnings (1)
📚 Learning: 2025-06-30T16:33:26.746Z
Learnt from: dwisiswant0
Repo: projectdiscovery/nuclei PR: 6290
File: pkg/fuzz/component/path_test.go:43-44
Timestamp: 2025-06-30T16:33:26.746Z
Learning: The user dwisiswant0 declined adding documentation for internal API changes in pkg/fuzz/component/path when the change was from nested URL field access (rebuilt.URL.Path, rebuilt.URL.String()) to direct field access (rebuilt.Path, rebuilt.String()), indicating they don't consider additional documentation necessary for such internal API modifications.
Applied to files:
pkg/protocols/http/raw/raw.go
🧬 Code graph analysis (1)
pkg/protocols/http/raw/raw_test.go (1)
pkg/protocols/http/raw/raw.go (1)
Parse(35-142)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint
🔇 Additional comments (6)
pkg/protocols/http/raw/raw_test.go (6)
125-135: LGTM! Core fix validated.This test correctly verifies that full URLs in unsafe raw requests are converted to relative paths, addressing the bug described in issue #6558.
137-147: LGTM! Path merging behavior validated.This test correctly verifies that when
disable-path-automergeis false, the extracted relative path is merged with the target URL's path.
149-159: LGTM! Query parameter preservation validated.This test correctly verifies that query parameters are preserved when converting full URLs to relative paths.
161-171: LGTM! HTTPS URL handling validated.This test confirms that HTTPS full URLs are handled identically to HTTP, which is the correct behavior.
173-189: LGTM! Root path edge case validated.This test correctly handles the root path edge case for both automerge modes, confirming the existing behavior is preserved.
191-199: LGTM! Safe mode backward compatibility validated.This test confirms that safe mode continues to work correctly with full URLs, maintaining backward compatibility.
Mzack9999
left a comment
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.
lgtm! Looks like the only way to send a full URL even as unsafe it's set self-contained to true
Proposed changes
fix(raw): handle full URLs in unsafe raw requests
Previously, when using
unsafe: truewith fullURLs (e.g.,
GET http://example.com/path HTTP/1.1),the
Parsefunc would treat the full URL as arelative path, resulting in malformed requests
like
GET /http://example.com/path HTTP/1.1.This occurred because the full URL handling
logic was only executed for non-unsafe requests,
causing unsafe requests with full URLs to fall
through to the unsafe case which wasn't designed
to handle them.
Changes:
logic runs.
safe and unsafe modes.
UnsafeRawByteswith the correctrelative path when unsafe is true.
disable-path-automerge.This fix maintains backward compatibility while
properly supporting the previously broken
combination of unsafe mode with full URLs.
Fixes #6558.
Proof
unsafe-raw-request.yaml:
unsafe-raw-request-merge.yaml:
safe-with-full-url-merge.yaml:
Outputs:
Checklist
Summary by CodeRabbit
Tests
Refactor