-
Notifications
You must be signed in to change notification settings - Fork 5.1k
fetch, audit: write the verbose curl line and the audit fix --ignore line as shell words #41726
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
base: main
Are you sure you want to change the base?
Changes from all commits
7449b56
32c4499
8a2841e
e66d9b7
bfa137a
60bc0e0
934c9cf
40225c1
3fbe99c
27a3ac5
29d45e1
d57cddc
8388540
0567221
1d2213f
f6db1bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ use core::fmt; | |
|
|
||
| use bstr::BStr; | ||
|
|
||
| use bun_core::fmt::shell_word; | ||
| use bun_core::output::enable_ansi_colors_stderr; | ||
| use bun_core::pretty_fmt; | ||
|
|
||
|
|
@@ -205,12 +206,11 @@ impl fmt::Display for HeaderCurlFormatter<'_> { | |
| if header.value_len > 0 { | ||
| write!( | ||
| f, | ||
| "-H \"{}: {}\"", | ||
| BStr::new(header.name()), | ||
| BStr::new(header.value()) | ||
| "-H {}", | ||
| shell_word(&[header.name(), b": ", header.value()]) | ||
| ) | ||
| } else { | ||
| write!(f, "-H \"{}\"", BStr::new(header.name())) | ||
| write!(f, "-H {}", shell_word(&[header.name()])) | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -330,8 +330,9 @@ pub struct RequestCurlFormatter<'a> { | |
| } | ||
|
|
||
| impl<'a> RequestCurlFormatter<'a> { | ||
| fn is_printable_body(content_type: &[u8]) -> bool { | ||
| if content_type.is_empty() { | ||
| fn is_printable_body(content_type: &[u8], body: &[u8]) -> bool { | ||
| // No argument of a command can hold a NUL. | ||
| if content_type.is_empty() || body.is_empty() || strings::contains_char(body, 0) { | ||
| return false; | ||
| } | ||
|
|
||
|
|
@@ -345,20 +346,27 @@ impl<'a> RequestCurlFormatter<'a> { | |
| impl fmt::Display for RequestCurlFormatter<'_> { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| let request = self.request; | ||
| let url = [request.path]; | ||
| let url = shell_word(&url); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Windows users who paste the printed curl line into cmd.exe can now run server-chosen text, which the base's Why this was flaggedTrigger: a Windows user runs with BUN_CONFIG_VERBOSE_FETCH=curl, a server answers a redirect whose Location query holds Verification: src/http/lib.rs:1376 runs on every platform; src/picohttp/lib.rs:350 |
||
| if enable_ansi_colors_stderr() { | ||
| f.write_str(pretty_fmt!("<r><d>[fetch] $<r> ", true))?; | ||
|
|
||
| write!( | ||
| f, | ||
| pretty_fmt!("<b><cyan>curl<r> <d>--http1.1<r> <b>\"{}\"<r>", true), | ||
| BStr::new(request.path), | ||
| pretty_fmt!("<b><cyan>curl<r> <d>--http1.1<r> <b>{}<r>", true), | ||
| url, | ||
| )?; | ||
| } else { | ||
| write!(f, "curl --http1.1 \"{}\"", BStr::new(request.path))?; | ||
| write!(f, "curl --http1.1 {}", url)?; | ||
| } | ||
|
|
||
| // curl expands `[1-3]` and `{a,b}` in a URL unless globbing is off. | ||
| if strings::index_of_any(request.path, b"[]{}").is_some() { | ||
| f.write_str(" --globoff")?; | ||
| } | ||
|
|
||
| if request.method != b"GET" { | ||
| write!(f, " -X {}", BStr::new(request.method))?; | ||
| write!(f, " -X {}", shell_word(&[request.method]))?; | ||
| } | ||
|
|
||
| if self.ignore_insecure { | ||
|
|
@@ -382,13 +390,8 @@ impl fmt::Display for RequestCurlFormatter<'_> { | |
| } | ||
| } | ||
|
|
||
| if !self.body.is_empty() && Self::is_printable_body(content_type) { | ||
| f.write_str(" --data-raw ")?; | ||
| bun_core::js_printer::write_json_string( | ||
| self.body, | ||
| f, | ||
| bun_core::strings::Encoding::Utf8, | ||
| )?; | ||
| if Self::is_printable_body(content_type, self.body) { | ||
| write!(f, " --data-raw {}", shell_word(&[self.body]))?; | ||
| } | ||
|
|
||
| Ok(()) | ||
|
|
||
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.
🟣 pre-existing, not blocking: Users who paste the curl line into an interactive bash whose history scanner does not track double quotes (the 3.2 and 4.x series) get
event not foundor history text spliced into the argument when a value holds'and a later!. A'is written as a bare"'"run at src/bun_core/fmt.rs:3509; that scanner takes the'"'as a single-quoted string, so the following'...'run is read unquoted and!xinside it is expanded. Fix: keep every!inside quoting that interactive bash of every version honours, e.g. write'as\'outside quotes ('it'\''s', also right for zsh, dash and fish) or document the PowerShell trade-off; the paste test runs bash with --norc non-interactively, so history expansion is never exercised.A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
Trigger: a value the server or request chooses holds a
'and, later in the same word, a!(URL path, header value or body), printed under BUN_CONFIG_VERBOSE_FETCH=curl via print_request at src/http/lib.rs:1376 and shell_word. ShellWordQuoted::part at src/bun_core/fmt.rs:3505-3511 emits the'as"'"between two'...'runs, giving'a'"'"'b!x'. In interactive bash releases where histexpand only skips single-quoted strings and does not toggle a double-quote state before doing so, the scanner treats'"'as the quoted string and then sees!xoutside any quote:!xfails withbash: !x: event not foundor splices the previous command text into the argument before the shell parses the line. On the base branch every value sat in"...", so any!was expanded in all bash versions; the change narrows but does not close the gap. The test at test/js/web/fetch/fetch.test.ts:4345-4359 runsbash --norc --noprofileon a script, where history expansion is off, so it cannot observe this.Verification: Triggers only in an interactive bash whose history scanner skips single-quoted strings but does not track double quotes, with a word holding
'then!. ShellWordQuoted::part in src/bun_core/fmt.rs writes'as"'", soa'b!xbecomes'a'"'"'b!x'and!xis expanded. On the base branch"a'b!x"also expanded!x. The paste test spawns bash non-interactively, where histexpand is off.