feat(http): respect annotations in unsafe mode#7044
Open
dwisiswant0 wants to merge 1 commit intodevfrom
Open
Conversation
Apply request annotations during `unsafe` raw request generation so @Host/@timeout/etc affect the effective target and execution context. Strip leading annotation directives from `unsafe` wire bytes before sending, since `rawhttp` transmits raw payload verbatim and annotation lines can produce malformed HTTP requests. Close #6747 Signed-off-by: Dwi Siswanto <git@dw1.io>
Contributor
WalkthroughThe pull request adds support for request annotations in unsafe/raw HTTP mode. Changes include fetching annotation overrides, applying host and header adjustments, stripping leading annotation lines from raw requests, and merging annotation-provided interactsh URLs into the final unsafe request object. Changes
Sequence DiagramsequenceDiagram
participant Client
participant RawParser as Raw Parser
participant AnnotationFetcher as Annotation Handler
participant RequestBuilder as Request Builder
participant Server
Client->>RawParser: Parse unsafe raw request with annotations
RawParser->>RawParser: Strip leading `@annotations`
RawParser-->>Client: rawRequest (without annotations)
Client->>RequestBuilder: Generate unsafe request
RequestBuilder->>AnnotationFetcher: Fetch annotation overrides<br/>(if annotationURL available)
AnnotationFetcher->>AnnotationFetcher: Apply Host header<br/>from original request
AnnotationFetcher->>Server: Context-bound request
Server-->>AnnotationFetcher: Annotation metadata
AnnotationFetcher->>AnnotationFetcher: Extract cancelFunc<br/>& interactsh URLs
AnnotationFetcher-->>RequestBuilder: Overrides
RequestBuilder->>RequestBuilder: Apply host overrides
RequestBuilder->>RequestBuilder: Merge interactsh URLs
RequestBuilder->>RequestBuilder: Attach cancelFunc
RequestBuilder-->>Client: Final unsafe request
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/protocols/http/raw/raw.go`:
- Around line 268-283: stripLeadingAnnotations fails to detect annotation
directives that are indented (e.g., " `@Host`: ..."); update the annotation check
in stripLeadingAnnotations to ignore leading whitespace before testing for "@"
so indented annotation lines are treated the same as non-indented ones.
Specifically, change the condition that calls stringsutil.HasPrefixAny on line
to instead call it on a left-trimmed version of line (trim spaces and tabs) so
lines starting with optional indentation followed by "@" are recognized and
skipped; keep the rest of the logic (requestLineFound, buffer writes, and
bufferPool handling) unchanged and only alter the annotation-detection
expression in stripLeadingAnnotations.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
pkg/protocols/http/build_request.gopkg/protocols/http/build_request_test.gopkg/protocols/http/raw/raw.gopkg/protocols/http/raw/raw_test.gopkg/protocols/http/request.go
Comment on lines
+268
to
+283
| func stripLeadingAnnotations(request string) []byte { | ||
| reader := bufio.NewReader(strings.NewReader(request)) | ||
| buffer := bufferPool.Get().(*bytes.Buffer) | ||
| buffer.Reset() | ||
| defer func() { | ||
| buffer.Reset() | ||
| bufferPool.Put(buffer) | ||
| }() | ||
|
|
||
| requestLineFound := false | ||
| for { | ||
| line, err := reader.ReadString('\n') | ||
| if len(line) > 0 { | ||
| if requestLineFound || !stringsutil.HasPrefixAny(line, "@") { | ||
| requestLineFound = true | ||
| _, _ = buffer.WriteString(line) |
Contributor
There was a problem hiding this comment.
Handle indented annotation directives when stripping.
Annotation detection is currently prefix-only on the raw line. Inputs like " @host: ..." are retained, so unsafe raw bytes can still carry annotation text.
🔧 Proposed fix
diff --git a/pkg/protocols/http/raw/raw.go b/pkg/protocols/http/raw/raw.go
@@
- if stringsutil.HasPrefixAny(s, "@") {
+ if stringsutil.HasPrefixAny(strings.TrimLeft(s, " \t"), "@") {
goto read_line
}
@@
- if requestLineFound || !stringsutil.HasPrefixAny(line, "@") {
+ trimmed := strings.TrimLeft(line, " \t")
+ if requestLineFound || !stringsutil.HasPrefixAny(trimmed, "@") {
requestLineFound = true
_, _ = buffer.WriteString(line)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/protocols/http/raw/raw.go` around lines 268 - 283,
stripLeadingAnnotations fails to detect annotation directives that are indented
(e.g., " `@Host`: ..."); update the annotation check in stripLeadingAnnotations
to ignore leading whitespace before testing for "@" so indented annotation lines
are treated the same as non-indented ones. Specifically, change the condition
that calls stringsutil.HasPrefixAny on line to instead call it on a left-trimmed
version of line (trim spaces and tabs) so lines starting with optional
indentation followed by "@" are recognized and skipped; keep the rest of the
logic (requestLineFound, buffer writes, and bufferPool handling) unchanged and
only alter the annotation-detection expression in stripLeadingAnnotations.
Neo - PR Security ReviewNo security issues found Highlights
Hardening Notes
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Apply request annotations during
unsaferawrequest generation so @Host/@timeout/etc affect
the effective target and execution context.
Strip leading annotation directives from
unsafewire bytes before sending, since
rawhttptransmits raw payload verbatim and annotation
lines can produce malformed HTTP requests.
Close #6747
Proof
Checklist
Summary by CodeRabbit
Release Notes