diff --git a/Cargo.lock b/Cargo.lock index 4f88b2de6b33..e6b9d7d6933f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -117,6 +117,31 @@ dependencies = [ "cipher", ] +[[package]] +name = "bon" +version = "3.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a602c73c7b0148ec6d12af6fd5cc7a46e2eacc8878271a999abac56eed12f561" +dependencies = [ + "bon-macros", + "rustversion", +] + +[[package]] +name = "bon-macros" +version = "3.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6dee98b0db6a962de883bf5d20362dee4d7ca0d12fe39a7c6c73c844e1cd7c1f" +dependencies = [ + "darling", + "ident_case", + "prettyplease", + "proc-macro2", + "quote", + "rustversion", + "syn", +] + [[package]] name = "bstr" version = "1.12.1" @@ -343,6 +368,7 @@ name = "bun_bundler" version = "0.0.0" dependencies = [ "bitflags", + "bon", "bstr", "bumpalo", "bun_alloc", @@ -773,6 +799,7 @@ name = "bun_glob" version = "0.0.0" dependencies = [ "bitflags", + "bon", "bstr", "bun_alloc", "bun_collections", @@ -813,6 +840,7 @@ name = "bun_http" version = "0.0.0" dependencies = [ "bitflags", + "bon", "bstr", "bun_alloc", "bun_analytics", @@ -934,6 +962,7 @@ name = "bun_install" version = "0.0.0" dependencies = [ "bitflags", + "bon", "bstr", "bun_alloc", "bun_analytics", @@ -1619,6 +1648,7 @@ dependencies = [ "bcrypt", "bitflags", "blake2", + "bon", "bstr", "bun_alloc", "bun_analytics", @@ -1717,6 +1747,7 @@ name = "bun_s3_signing" version = "0.0.0" dependencies = [ "bitflags", + "bon", "bstr", "bun_base64", "bun_boringssl_sys", @@ -2548,6 +2579,7 @@ dependencies = [ "ident_case", "proc-macro2", "quote", + "strsim", "syn", ] @@ -3331,6 +3363,12 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" +[[package]] +name = "strsim" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" + [[package]] name = "strum" version = "0.26.3" diff --git a/Cargo.toml b/Cargo.toml index d6ef20a9a5f5..5753c141fd3f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -343,6 +343,10 @@ libc = "0.2" memchr = "2" rustix = { version = "0.38", default-features = false, features = ["std", "fs", "event", "process", "net"] } bitflags = "2" +# Compile-time-checked (typestate) builders for structs and functions: named +# setters for same-typed positional args, required fields enforced by the type +# system. Zero runtime cost (the required/optional state lives in the types). +bon = "3" thiserror = "2" smallvec = "1" bumpalo = { version = "3", features = ["collections", "boxed"] } diff --git a/src/bundler/Cargo.toml b/src/bundler/Cargo.toml index b568c7ef08d1..75969de123f6 100644 --- a/src/bundler/Cargo.toml +++ b/src/bundler/Cargo.toml @@ -10,6 +10,7 @@ path = "lib.rs" workspace = true [dependencies] +bon.workspace = true bytemuck = "1" bun_opaque.workspace = true bun_dispatch.workspace = true diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index 8ce69747981a..08435f1fff3b 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -728,13 +728,13 @@ impl<'a> LinkerContext<'a> { // When --splitting is enabled, we have to make sure we import the __jsonParse function. self.graph - .generate_symbol_import_and_use( - html_import, - Index::part(1u32).get(), - actual_ref, - 1, - Index::RUNTIME, - ) + .generate_symbol_import_and_use() + .source_index(html_import) + .part_index(Index::part(1u32).get()) + .ref_(actual_ref) + .use_count(1) + .source_index_to_import_from(Index::RUNTIME) + .call() .expect("OOM"); } } @@ -3174,13 +3174,13 @@ impl<'a> LinkerContext<'a> { // Bake uses a wrapping approach that does not use __commonJS if self.options.output_format != Format::InternalBakeDev { self.graph - .generate_symbol_import_and_use( - source_index, - part_index, - self.cjs_runtime_ref, - 1, - crate::Index::RUNTIME, - ) + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(part_index) + .ref_(self.cjs_runtime_ref) + .use_count(1) + .source_index_to_import_from(crate::Index::RUNTIME) + .call() .expect("unreachable"); } } @@ -3277,25 +3277,25 @@ impl<'a> LinkerContext<'a> { *wrapper_part_index = crate::Index::part(part_index); if wrapper_ref.is_valid() && self.options.output_format != Format::InternalBakeDev { self.graph - .generate_symbol_import_and_use( - source_index, - part_index, - self.esm_runtime_ref, - 1, - crate::Index::RUNTIME, - ) + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(part_index) + .ref_(self.esm_runtime_ref) + .use_count(1) + .source_index_to_import_from(crate::Index::RUNTIME) + .call() .expect("OOM"); // Only mark __promiseAll as used if we have multiple async dependencies if needs_promise_all { self.graph - .generate_symbol_import_and_use( - source_index, - part_index, - self.promise_all_runtime_ref, - 1, - crate::Index::RUNTIME, - ) + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(part_index) + .ref_(self.promise_all_runtime_ref) + .use_count(1) + .source_index_to_import_from(crate::Index::RUNTIME) + .call() .expect("OOM"); } } @@ -4037,13 +4037,14 @@ impl<'a> LinkerContext<'a> { }, )?; - self.graph.generate_symbol_import_and_use( - source_index, - part_index, - module_ref, - 1, - crate::Index::init(source_index), - )?; + self.graph + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(part_index) + .ref_(module_ref) + .use_count(1) + .source_index_to_import_from(crate::Index::init(source_index)) + .call()?; let top_level = &mut self .graph .meta diff --git a/src/bundler/LinkerGraph.rs b/src/bundler/LinkerGraph.rs index 329a26df3808..1029b9a7e56b 100644 --- a/src/bundler/LinkerGraph.rs +++ b/src/bundler/LinkerGraph.rs @@ -586,13 +586,13 @@ impl<'a> LinkerGraph<'a> { ); let ref_ = self.runtime_function(name); - self.generate_symbol_import_and_use( - source_index, - entry_point_part_index.get(), - ref_, - count, - Index::RUNTIME, - ) + self.generate_symbol_import_and_use() + .source_index(source_index) + .part_index(entry_point_part_index.get()) + .ref_(ref_) + .use_count(count) + .source_index_to_import_from(Index::RUNTIME) + .call() } pub fn add_part_to_file(&mut self, id: u32, part: Part) -> Result { @@ -605,7 +605,14 @@ impl<'a> LinkerGraph<'a> { part, ) } +} +// Separate impl block so `#[bon::bon]` only re-emits this one method. +#[bon::bon] +impl<'a> LinkerGraph<'a> { + /// Named setters: `source_index`, `part_index`, and `use_count` are all + /// `u32`; positional arguments could transpose any pair of them. + #[builder] pub fn generate_symbol_import_and_use( &mut self, source_index: u32, @@ -631,7 +638,9 @@ impl<'a> LinkerGraph<'a> { source_index_to_import_from, ) } +} +impl<'a> LinkerGraph<'a> { pub fn top_level_symbol_to_parts(&self, id: u32, ref_: Ref) -> &[u32] { top_level_symbol_to_parts( self.meta.items_top_level_symbol_to_parts_overlay(), diff --git a/src/bundler/linker_context/generateCodeForLazyExport.rs b/src/bundler/linker_context/generateCodeForLazyExport.rs index 81e82c5baefd..d602777a8f18 100644 --- a/src/bundler/linker_context/generateCodeForLazyExport.rs +++ b/src/bundler/linker_context/generateCodeForLazyExport.rs @@ -409,13 +409,14 @@ pub fn generate_code_for_lazy_export( ), expr, ); - this.graph.generate_symbol_import_and_use( - source_index, - 0, - module_ref, - 1, - Index::init(source_index), - )?; + this.graph + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(0) + .ref_(module_ref) + .use_count(1) + .source_index_to_import_from(Index::init(source_index)) + .call()?; // If this is a .napi addon and it's not node, we need to generate a require() call to the runtime if matches!(expr.data, ExprData::ECall(ref c) diff --git a/src/bundler/linker_context/scanImportsAndExports.rs b/src/bundler/linker_context/scanImportsAndExports.rs index babceaa56ee7..14e4e5ec5ffe 100644 --- a/src/bundler/linker_context/scanImportsAndExports.rs +++ b/src/bundler/linker_context/scanImportsAndExports.rs @@ -700,13 +700,14 @@ pub fn scan_imports_and_exports( debug_assert!(runtime_export_symbol_ref.is_valid()); - this.graph.generate_symbol_import_and_use( - source_index, - bun_ast::NAMESPACE_EXPORT_PART_INDEX, - runtime_export_symbol_ref, - 1, - Index::RUNTIME, - )?; + this.graph + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(bun_ast::NAMESPACE_EXPORT_PART_INDEX) + .ref_(runtime_export_symbol_ref) + .use_count(1) + .source_index_to_import_from(Index::RUNTIME) + .call()?; } { @@ -989,13 +990,14 @@ pub fn scan_imports_and_exports( // Depend on the automatically-generated require wrapper symbol let wrapper_ref = col_ref!(wrapper_refs)[other_id]; if wrapper_ref.is_valid() { - this.graph.generate_symbol_import_and_use( - source_index, - part_index as u32, - wrapper_ref, - 1, - Index::source(other_source_index), - )?; + this.graph + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(part_index as u32) + .ref_(wrapper_ref) + .use_count(1) + .source_index_to_import_from(Index::source(other_source_index)) + .call()?; } // This is an ES6 import of a CommonJS module, so it needs the @@ -1017,13 +1019,14 @@ pub fn scan_imports_and_exports( // but does not need to be done for "import" statements since // those just cause us to reference the exports directly. if other_flags.wrap == WrapKind::Esm && kind != ImportKind::Stmt { - this.graph.generate_symbol_import_and_use( - source_index, - part_index as u32, - col_ref!(exports_refs)[other_id], - 1, - Index::source(other_source_index), - )?; + this.graph + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(part_index as u32) + .ref_(col_ref!(exports_refs)[other_id]) + .use_count(1) + .source_index_to_import_from(Index::source(other_source_index)) + .call()?; // If this is a "require()" call, then we should add the // "__esModule" marker to behave as if the module was converted @@ -1050,13 +1053,14 @@ pub fn scan_imports_and_exports( // something ends up needing to use it later. This could potentially // be omitted in some cases with more advanced analysis if this // dynamic export fallback object doesn't end up being needed. - this.graph.generate_symbol_import_and_use( - source_index, - part_index as u32, - col_ref!(exports_refs)[other_id], - 1, - Index::source(other_source_index), - )?; + this.graph + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(part_index as u32) + .ref_(col_ref!(exports_refs)[other_id]) + .use_count(1) + .source_index_to_import_from(Index::source(other_source_index)) + .call()?; } } @@ -1087,25 +1091,27 @@ pub fn scan_imports_and_exports( // pull in the "exports_b" symbol into this export star. This matters // in code splitting situations where the "export_b" symbol might live // in a different chunk than this export star. - this.graph.generate_symbol_import_and_use( - source_index, - part_index as u32, - col_ref!(exports_refs)[other_id], - 1, - Index::source(other_source_index), - )?; + this.graph + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(part_index as u32) + .ref_(col_ref!(exports_refs)[other_id]) + .use_count(1) + .source_index_to_import_from(Index::source(other_source_index)) + .call()?; } } if happens_at_runtime { // Depend on this file's "exports" object for the first argument to "__reExport" - this.graph.generate_symbol_import_and_use( - source_index, - part_index as u32, - col_ref!(exports_refs)[id], - 1, - Index::source(source_index), - )?; + this.graph + .generate_symbol_import_and_use() + .source_index(source_index) + .part_index(part_index as u32) + .ref_(col_ref!(exports_refs)[id]) + .use_count(1) + .source_index_to_import_from(Index::source(source_index)) + .call()?; col!(ast_flags_list)[id].insert(AstFlags::USES_EXPORTS_REF); col!(import_records_list)[id].as_mut_slice()[*import_record_index as usize] .flags diff --git a/src/glob/Cargo.toml b/src/glob/Cargo.toml index 450b9d2abd62..703008a2d410 100644 --- a/src/glob/Cargo.toml +++ b/src/glob/Cargo.toml @@ -10,6 +10,7 @@ path = "lib.rs" workspace = true [dependencies] +bon.workspace = true strum.workspace = true bstr.workspace = true scopeguard.workspace = true diff --git a/src/glob/GlobWalker.rs b/src/glob/GlobWalker.rs index 4a018934645a..5ecd06841294 100644 --- a/src/glob/GlobWalker.rs +++ b/src/glob/GlobWalker.rs @@ -1421,66 +1421,29 @@ impl SyntaxHint { // GlobWalker impl // ───────────────────────────────────────────────────────────────────────────── +#[bon::bon] impl GlobWalker { - /// The arena parameter is dereferenced and copied if all allocations go well and nothing goes wrong - // Note: out-param constructor reshaped to return Self. + /// `pattern` is positional; everything else is a named setter so the + /// five bool flags cannot be transposed: + /// `GlobWalker::init(pattern).only_files(true).call()?` + #[builder] pub fn init( + /// Copied into the walker. + #[builder(start_fn)] pattern: &[u8], - dot: bool, - absolute: bool, - follow_symlinks: bool, - error_on_broken_symlinks: bool, - only_files: bool, - ignore_filter_fn: Option, - ) -> Result, Error> { - // `bun_paths::fs::FileSystem` (singleton holds only the cwd string; the - // DirEntry cache stays in `bun_resolver`). - Self::init_with_cwd( - pattern, - bun_paths::fs::FileSystem::instance().top_level_dir(), - dot, - absolute, - follow_symlinks, - error_on_broken_symlinks, - only_files, - ignore_filter_fn, - ) - } - - pub fn debug_pattern_components(&self) { - let pattern = &self.pattern; - let components = &self.pattern_components; - let ptr = std::ptr::from_ref(self) as usize; - log!("GlobWalker(0x{:x}) components:", ptr); - for cmp in components.iter() { - match cmp.syntax_hint { - SyntaxHint::Single => log!(" *"), - SyntaxHint::Double => log!(" **"), - SyntaxHint::Dot => log!(" ."), - SyntaxHint::DotBack => log!(" ../"), - SyntaxHint::Literal | SyntaxHint::WildcardFilepath | SyntaxHint::None => log!( - " hint={} component_str={}", - <&'static str>::from(cmp.syntax_hint), - bstr::BStr::new(cmp.pattern_slice(pattern)) - ), - } - } - } - - /// `cwd` should be allocated with the arena - /// The arena parameter is dereferenced and copied if all allocations go well and nothing goes wrong - // Note: out-param constructor reshaped to return Self. - pub fn init_with_cwd( - pattern: &[u8], + /// Copied into the walker. Defaults to the process's top-level dir + /// (`bun_paths::fs::FileSystem` singleton; the DirEntry cache stays + /// in `bun_resolver`). + #[builder(default = bun_paths::fs::FileSystem::instance().top_level_dir())] cwd: &[u8], - dot: bool, - absolute: bool, - follow_symlinks: bool, - error_on_broken_symlinks: bool, - only_files: bool, + #[builder(default)] dot: bool, + #[builder(default)] absolute: bool, + #[builder(default)] follow_symlinks: bool, + #[builder(default)] error_on_broken_symlinks: bool, + #[builder(default)] only_files: bool, ignore_filter_fn: Option, ) -> Result, Error> { - log!("initWithCwd(cwd={})", bstr::BStr::new(cwd)); + log!("init(cwd={})", bstr::BStr::new(cwd)); let mut this = Self { cwd: Box::from(cwd), pattern: Box::from(pattern), @@ -1510,14 +1473,34 @@ impl GlobWalker { &mut this.basename_excluding_special_syntax_component_idx, )?; - // copy arena after all allocations are successful - if cfg!(debug_assertions) { this.debug_pattern_components(); } Ok(Ok(this)) } +} + +impl GlobWalker { + pub fn debug_pattern_components(&self) { + let pattern = &self.pattern; + let components = &self.pattern_components; + let ptr = std::ptr::from_ref(self) as usize; + log!("GlobWalker(0x{:x}) components:", ptr); + for cmp in components.iter() { + match cmp.syntax_hint { + SyntaxHint::Single => log!(" *"), + SyntaxHint::Double => log!(" **"), + SyntaxHint::Dot => log!(" ."), + SyntaxHint::DotBack => log!(" ../"), + SyntaxHint::Literal | SyntaxHint::WildcardFilepath | SyntaxHint::None => log!( + " hint={} component_str={}", + <&'static str>::from(cmp.syntax_hint), + bstr::BStr::new(cmp.pattern_slice(pattern)) + ), + } + } + } pub fn handle_sys_err_with_path(&mut self, err: &SysError, path_buf: &ZStr) -> SysError { let src = path_buf.as_bytes(); diff --git a/src/http/AsyncHTTP.rs b/src/http/AsyncHTTP.rs index acab6d6cac42..9ec08894a71f 100644 --- a/src/http/AsyncHTTP.rs +++ b/src/http/AsyncHTTP.rs @@ -422,17 +422,21 @@ pub fn preconnect(url: URL<'static>, is_url_owned: bool) { unsafe { let response_buffer: *mut MutableString = core::ptr::addr_of_mut!((*this).response_buffer); let url = (*this).url.clone(); - let async_http = (*this).async_http.insert(AsyncHTTP::init( - Method::GET, - url, - headers::EntryList::default(), - b"", - response_buffer, - b"", - HTTPClientResultCallback::new::(this, Preconnect::on_result), - FetchRedirect::Manual, - Options::default(), - )); + let async_http = (*this).async_http.insert( + AsyncHTTP::init() + .method(Method::GET) + .url(url) + .headers(headers::EntryList::default()) + .headers_buf(b"") + .response_buffer(response_buffer) + .request_body(b"") + .callback(HTTPClientResultCallback::new::( + this, + Preconnect::on_result, + )) + .redirect_type(FetchRedirect::Manual) + .call(), + ); async_http.client.flags.is_preconnect_only = true; crate::HTTPThread::schedule(Batch::from(core::ptr::addr_of_mut!(async_http.task))); @@ -443,16 +447,23 @@ pub fn preconnect(url: URL<'static>, is_url_owned: bool) { // impl AsyncHTTP — init / reset / schedule // ────────────────────────────────────────────────────────────────────────── +#[bon::bon] impl<'a> AsyncHTTP<'a> { + /// Named setters: `headers_buf` and `request_body` are both `&[u8]`, + /// so positional arguments could be transposed and still type-check. + #[builder] pub fn init( method: Method, url: URL<'a>, headers: headers::EntryList, + /// Backing storage that `headers` indexes into. headers_buf: &'a [u8], response_buffer: *mut MutableString, request_body: &'a [u8], callback: HTTPClientResultCallback, redirect_type: FetchRedirect, + /// All-optional extras; `Options::default()` is the neutral element. + #[builder(default)] options: Options<'a>, ) -> AsyncHTTP<'a> { let async_http_id = if options @@ -561,10 +572,12 @@ impl<'a> AsyncHTTP<'a> { /// value — in practice they live on the calling stack frame and the /// request is driven to completion via `send_sync` before that frame /// returns. + #[builder] pub fn init_sync( method: Method, url: URL<'a>, headers: headers::EntryList, + /// Backing storage that `headers` indexes into. headers_buf: &'a [u8], response_buffer: *mut MutableString, request_body: &'a [u8], @@ -572,21 +585,21 @@ impl<'a> AsyncHTTP<'a> { hostname: Option<&'a [u8]>, redirect_type: FetchRedirect, ) -> AsyncHTTP<'a> { - Self::init( - method, - url, - headers, - headers_buf, - response_buffer, - request_body, - noop_callback(), - redirect_type, - Options { + Self::init() + .method(method) + .url(url) + .headers(headers) + .headers_buf(headers_buf) + .response_buffer(response_buffer) + .request_body(request_body) + .callback(noop_callback()) + .redirect_type(redirect_type) + .options(Options { http_proxy, hostname, ..Options::default() - }, - ) + }) + .call() } pub fn schedule(&mut self, batch: &mut Batch) { diff --git a/src/http/Cargo.toml b/src/http/Cargo.toml index 24e3240fe170..2acd5502177c 100644 --- a/src/http/Cargo.toml +++ b/src/http/Cargo.toml @@ -10,6 +10,7 @@ path = "lib.rs" workspace = true [dependencies] +bon.workspace = true thiserror.workspace = true strum.workspace = true bstr.workspace = true diff --git a/src/http/HTTPContext.rs b/src/http/HTTPContext.rs index d5db29f77305..59e03a239271 100644 --- a/src/http/HTTPContext.rs +++ b/src/http/HTTPContext.rs @@ -565,7 +565,12 @@ impl HTTPContext { unsafe { ssl_ctx_setup(self.ssl_ctx()) }; } } +} +// Separate impl block so `#[bon::bon]` only re-emits `release_socket` and +// `existing_socket`, not the rest of the (large) `HTTPContext` impl above. +#[bon::bon] +impl HTTPContext { /// Attempt to keep the socket alive by reusing it for another request. /// If no space is available, close the socket. /// @@ -577,10 +582,13 @@ impl HTTPContext { /// tunnel. The pool takes ownership of one strong ref on the tunnel; /// the caller must NOT deref it afterwards. If pooling fails (pool /// full, hostname too long, socket bad), the tunnel is dereffed here. - #[allow(clippy::too_many_arguments)] + /// + /// Named setters: `hostname`/`target_hostname` and `port`/`target_port` + /// are same-typed pairs that positional arguments could transpose. + #[builder] pub(crate) fn release_socket( &mut self, - socket: HTTPSocket, + #[builder(start_fn)] socket: HTTPSocket, did_have_handshaking_error_while_reject_unauthorized_is_false: bool, established_with_reject_unauthorized: bool, hostname: &[u8], @@ -685,7 +693,9 @@ impl HTTPContext { Self::close_socket(socket); } - #[allow(clippy::too_many_arguments)] + /// Named setters: `hostname`/`target_hostname` and `port`/`target_port` + /// are same-typed pairs that positional arguments could transpose. + #[builder] fn existing_socket( &mut self, reject_unauthorized: bool, @@ -830,7 +840,9 @@ impl HTTPContext { None } +} +impl HTTPContext { pub(crate) fn connect_socket( &mut self, client: &mut HTTPClient, @@ -951,21 +963,23 @@ impl HTTPContext { 0 }; - if let Some(mut found) = self.existing_socket( - client.flags.reject_unauthorized, - hostname, - port, - SSLConfig::raw_ptr(client.tls_props.as_ref()), - want_tunnel, - target_hostname, - target_port, - proxy_auth_hash, - if SSL { + if let Some(mut found) = self + .existing_socket() + .reject_unauthorized(client.flags.reject_unauthorized) + .hostname(hostname) + .port(port) + .maybe_ssl_config(SSLConfig::raw_ptr(client.tls_props.as_ref())) + .want_tunnel(want_tunnel) + .target_hostname(target_hostname) + .target_port(target_port) + .proxy_auth_hash(proxy_auth_hash) + .want_h2(if SSL { client.alpn_offer() } else { AlpnOffer::H1 - }, - ) { + }) + .call() + { let sock = found.socket; Self::set_socket_ext( sock, diff --git a/src/http/h2_client/ClientSession.rs b/src/http/h2_client/ClientSession.rs index d9696a577cf4..03e6bc57c40a 100644 --- a/src/http/h2_client/ClientSession.rs +++ b/src/http/h2_client/ClientSession.rs @@ -945,19 +945,20 @@ impl ClientSession { // ancestor frame holds `&mut NewHTTPContext` here and forming one // from the backref is sound — route through the centralised // [`HTTPClient::ssl_ctx_mut`] accessor (same set-once invariant). - HTTPClient::ssl_ctx_mut(self.ctx).release_socket( - self.socket, - self.did_have_handshaking_error, - self.established_with_reject_unauthorized, - &self.hostname, - self.port, - self.ssl_config.as_ref(), - None, - b"", - 0, - self.host_header_hash, - Some(self_ptr), - ); + HTTPClient::ssl_ctx_mut(self.ctx) + .release_socket(self.socket) + .did_have_handshaking_error_while_reject_unauthorized_is_false( + self.did_have_handshaking_error, + ) + .established_with_reject_unauthorized(self.established_with_reject_unauthorized) + .hostname(&self.hostname) + .port(self.port) + .maybe_ssl_config(self.ssl_config.as_ref()) + .target_hostname(b"") + .target_port(0) + .proxy_auth_hash(self.host_header_hash) + .h2_session(self_ptr) + .call(); } else { NewHTTPContext::::close_socket(self.socket); // SAFETY: `self: &mut Self` carries write provenance to the Box alloc. diff --git a/src/http/lib.rs b/src/http/lib.rs index 3c12b4afdba6..6b16eaab446b 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -2637,19 +2637,19 @@ impl<'a> HTTPClient<'a> { // server is still parsing as the previous chunked body. bun_core::scoped_log!(fetch, "Keep-Alive release in redirect"); debug_assert!(!self.connected_url.hostname.is_empty()); - Self::ssl_ctx_mut(ctx).release_socket( - socket, - self.flags.did_have_handshaking_error && !self.flags.reject_unauthorized, - self.flags.reject_unauthorized, - self.connected_url.hostname, - self.connected_url.get_port_auto(), - self.tls_props.as_ref(), - None, - b"", - 0, - 0, - None, - ); + Self::ssl_ctx_mut(ctx) + .release_socket(socket) + .did_have_handshaking_error_while_reject_unauthorized_is_false( + self.flags.did_have_handshaking_error && !self.flags.reject_unauthorized, + ) + .established_with_reject_unauthorized(self.flags.reject_unauthorized) + .hostname(self.connected_url.hostname) + .port(self.connected_url.get_port_auto()) + .maybe_ssl_config(self.tls_props.as_ref()) + .target_hostname(b"") + .target_port(0) + .proxy_auth_hash(0) + .call(); } else { GenHttpContext::::close_socket(socket); } @@ -4311,21 +4311,23 @@ impl<'a> HTTPClient<'a> { // writeProxyConnect line 346). The SNI override (hostname) is // hashed into proxyAuthHash separately — both must match, but // they're distinct values when a Host header override is set. - Self::ssl_ctx_mut(ctx).release_socket( - socket, - self.flags.did_have_handshaking_error && !self.flags.reject_unauthorized, - self.flags.reject_unauthorized, - self.connected_url.hostname, - self.connected_url.get_port_auto(), - self.tls_props.as_ref(), - tunnel, - if had_tunnel { self.url.hostname } else { b"" }, - if had_tunnel { + Self::ssl_ctx_mut(ctx) + .release_socket(socket) + .did_have_handshaking_error_while_reject_unauthorized_is_false( + self.flags.did_have_handshaking_error && !self.flags.reject_unauthorized, + ) + .established_with_reject_unauthorized(self.flags.reject_unauthorized) + .hostname(self.connected_url.hostname) + .port(self.connected_url.get_port_auto()) + .maybe_ssl_config(self.tls_props.as_ref()) + .maybe_tunnel(tunnel) + .target_hostname(if had_tunnel { self.url.hostname } else { b"" }) + .target_port(if had_tunnel { self.url.get_port_auto() } else { 0 - }, - if had_tunnel || (IS_SSL && self.http_proxy.is_none()) { + }) + .proxy_auth_hash(if had_tunnel || (IS_SSL && self.http_proxy.is_none()) { // Direct TLS: the handshake verified the peer against // the Host-header override (get_tls_hostname), so the // override hash must be part of the pool key. Matches @@ -4333,9 +4335,8 @@ impl<'a> HTTPClient<'a> { self.proxy_auth_hash() } else { 0 - }, - None, - ); + }) + .call(); } else { if self.proxy_tunnel.is_some() { bun_core::scoped_log!(fetch, "close the tunnel"); @@ -4561,19 +4562,19 @@ impl<'a> HTTPClient<'a> { bun_core::scoped_log!(fetch, "onPreconnect({})", BStr::new(self.url.href)); self.unregister_abort_tracker(); let ctx = self.get_ssl_ctx::(); - Self::ssl_ctx_mut(ctx).release_socket( - socket, - self.flags.did_have_handshaking_error && !self.flags.reject_unauthorized, - self.flags.reject_unauthorized, - self.url.hostname, - self.url.get_port_auto(), - self.tls_props.as_ref(), - None, - b"", - 0, - 0, - None, - ); + Self::ssl_ctx_mut(ctx) + .release_socket(socket) + .did_have_handshaking_error_while_reject_unauthorized_is_false( + self.flags.did_have_handshaking_error && !self.flags.reject_unauthorized, + ) + .established_with_reject_unauthorized(self.flags.reject_unauthorized) + .hostname(self.url.hostname) + .port(self.url.get_port_auto()) + .maybe_ssl_config(self.tls_props.as_ref()) + .target_hostname(b"") + .target_port(0) + .proxy_auth_hash(0) + .call(); self.state.reset(); self.state.response_stage = ResponseStage::Done; diff --git a/src/install/Cargo.toml b/src/install/Cargo.toml index f6544158714a..4cce2eaee65f 100644 --- a/src/install/Cargo.toml +++ b/src/install/Cargo.toml @@ -17,6 +17,7 @@ shim_standalone = [] workspace = true [dependencies] +bon.workspace = true bytemuck = "1" strum.workspace = true thiserror.workspace = true diff --git a/src/install/NetworkTask.rs b/src/install/NetworkTask.rs index e89078efa2c4..9f80e5670a8e 100644 --- a/src/install/NetworkTask.rs +++ b/src/install/NetworkTask.rs @@ -632,20 +632,22 @@ impl NetworkTask { // MaybeUninit overwrite — see field doc; old slot value is // either uninitialized (fresh hive slot) or a stale bitwise copy from // `notify`, neither of which is safe/meaningful to drop. - self.unsafe_http_client = MaybeUninit::new(AsyncHTTP::init( - http::Method::GET, - url, - header_builder.entries, - headers_buf, - ptr::addr_of_mut!(self.response_buffer), - b"", - completion_callback, - http::FetchRedirect::Follow, - AsyncHTTPOptions { - http_proxy, - ..Default::default() - }, - )); + self.unsafe_http_client = MaybeUninit::new( + AsyncHTTP::init() + .method(http::Method::GET) + .url(url) + .headers(header_builder.entries) + .headers_buf(headers_buf) + .response_buffer(ptr::addr_of_mut!(self.response_buffer)) + .request_body(b"") + .callback(completion_callback) + .redirect_type(http::FetchRedirect::Follow) + .options(AsyncHTTPOptions { + http_proxy, + ..Default::default() + }) + .call(), + ); self.http_mut().client.flags.reject_unauthorized = pm.tls_reject_unauthorized(); if PackageManager::verbose_install() { @@ -865,17 +867,19 @@ impl NetworkTask { // MaybeUninit overwrite — see field doc; old slot value is // either uninitialized (fresh hive slot) or a stale bitwise copy from // `notify`, neither of which is safe/meaningful to drop. - self.unsafe_http_client = MaybeUninit::new(AsyncHTTP::init( - http::Method::GET, - url, - header_builder.entries, - header_buf, - ptr::addr_of_mut!(self.response_buffer), - b"", - completion_callback, - http::FetchRedirect::Follow, - http_options, - )); + self.unsafe_http_client = MaybeUninit::new( + AsyncHTTP::init() + .method(http::Method::GET) + .url(url) + .headers(header_builder.entries) + .headers_buf(header_buf) + .response_buffer(ptr::addr_of_mut!(self.response_buffer)) + .request_body(b"") + .callback(completion_callback) + .redirect_type(http::FetchRedirect::Follow) + .options(http_options) + .call(), + ); self.http_mut().client.flags.reject_unauthorized = pm.tls_reject_unauthorized(); if PackageManager::verbose_install() { self.http_mut().client.verbose = HTTPVerboseLevel::Headers; diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index aad2a40ddc2c..673d7ba0f6ad 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -1726,12 +1726,20 @@ impl Package { }; self.parse(lockfile, pm, log, source, resolver, features) } +} +// Separate impl block so `#[bon::bon]` only re-emits `parse_dependency`, not +// the rest of the (large) `Package` impl above. +#[bon::bon] +impl Package { // The live `StringBuilder` // (also passed) already holds `&mut lockfile.buffers.string_bytes`. The // body only otherwise touches `workspace_paths` / `workspace_versions`, // so accept those two maps directly and read `string_bytes` via the // builder — caller can then split-borrow at the field level. + // + // Named setters: sixteen positional parameters would be unreviewable. + #[builder] fn parse_dependency( workspace_paths: &mut lockfile::NameHashMap, workspace_versions: &mut lockfile::VersionHashMap, @@ -2134,7 +2142,9 @@ impl Package { Ok(Some(this_dep)) } +} +impl Package { pub fn parse_with_json( &mut self, lockfile: &mut Lockfile, @@ -2858,24 +2868,25 @@ impl Package { None }; - if let Some(dep_) = Self::parse_dependency( - &mut lockfile.workspace_paths, - &mut lockfile.workspace_versions, - &mut lockfile.scratch.duplicate_checker_map, - pm, - log, - source, - group, - &mut string_builder, - FEATURES, - package_dependencies.as_mut_slice(), - total_dependencies_count, - Some(dependency::version::Tag::Workspace), - workspace_version, - external_name, - path_, - bun_ast::Loc::EMPTY, - )? { + if let Some(dep_) = Self::parse_dependency() + .workspace_paths(&mut lockfile.workspace_paths) + .workspace_versions(&mut lockfile.workspace_versions) + .duplicate_checker_map(&mut lockfile.scratch.duplicate_checker_map) + .pm(pm) + .log(log) + .source(source) + .group(group) + .string_builder(&mut string_builder) + .features(FEATURES) + .package_dependencies(package_dependencies.as_mut_slice()) + .dependencies_count(total_dependencies_count) + .tag(dependency::version::Tag::Workspace) + .maybe_workspace_ver(workspace_version) + .external_alias(external_name) + .version(path_) + .key_loc(bun_ast::Loc::EMPTY) + .call()? + { let mut dep = dep_; if group.behavior.is_peer() && optional_peer_dependencies.swap_remove(&external_name.hash) @@ -2905,24 +2916,23 @@ impl Package { let external_name = string_builder.append::(key); let version = version.unwrap_or(b""); - if let Some(dep_) = Self::parse_dependency( - &mut lockfile.workspace_paths, - &mut lockfile.workspace_versions, - &mut lockfile.scratch.duplicate_checker_map, - pm, - log, - source, - group, - &mut string_builder, - FEATURES, - package_dependencies.as_mut_slice(), - total_dependencies_count, - None, - None, - external_name, - version, - key_loc, - )? { + if let Some(dep_) = Self::parse_dependency() + .workspace_paths(&mut lockfile.workspace_paths) + .workspace_versions(&mut lockfile.workspace_versions) + .duplicate_checker_map(&mut lockfile.scratch.duplicate_checker_map) + .pm(pm) + .log(log) + .source(source) + .group(group) + .string_builder(&mut string_builder) + .features(FEATURES) + .package_dependencies(package_dependencies.as_mut_slice()) + .dependencies_count(total_dependencies_count) + .external_alias(external_name) + .version(version) + .key_loc(key_loc) + .call()? + { let mut dep = dep_; if group.behavior.is_peer() && optional_peer_dependencies.swap_remove(&external_name.hash) @@ -2955,24 +2965,23 @@ impl Package { let meta_only = optional_peer_dependencies.iterator(); for entry in meta_only { let external_name = string_builder.append::(*entry.value_ptr); - if let Some(dep_) = Self::parse_dependency( - &mut lockfile.workspace_paths, - &mut lockfile.workspace_versions, - &mut lockfile.scratch.duplicate_checker_map, - pm, - log, - source, - &DependencyGroup::PEER, - &mut string_builder, - FEATURES, - package_dependencies.as_mut_slice(), - total_dependencies_count, - None, - None, - external_name, - b"*", - bun_ast::Loc::EMPTY, - )? { + if let Some(dep_) = Self::parse_dependency() + .workspace_paths(&mut lockfile.workspace_paths) + .workspace_versions(&mut lockfile.workspace_versions) + .duplicate_checker_map(&mut lockfile.scratch.duplicate_checker_map) + .pm(pm) + .log(log) + .source(source) + .group(&DependencyGroup::PEER) + .string_builder(&mut string_builder) + .features(FEATURES) + .package_dependencies(package_dependencies.as_mut_slice()) + .dependencies_count(total_dependencies_count) + .external_alias(external_name) + .version(b"*") + .key_loc(bun_ast::Loc::EMPTY) + .call()? + { let mut dep = dep_; dep.behavior.insert(Behavior::OPTIONAL); package_dependencies.push(dep); diff --git a/src/install/lockfile/Package/WorkspaceMap.rs b/src/install/lockfile/Package/WorkspaceMap.rs index 555dde2baef1..997f0d41e11d 100644 --- a/src/install/lockfile/Package/WorkspaceMap.rs +++ b/src/install/lockfile/Package/WorkspaceMap.rs @@ -343,19 +343,12 @@ impl WorkspaceMap { if cwd.is_empty() { cwd = bun_resolver::fs::FileSystem::instance().top_level_dir(); } - // GlobWalker::init_with_cwd is now an associated constructor - // returning `Result>`; arena param dropped (heap-backed), - // ignore filter supplied as final arg. - let mut walker = match GlobWalker::init_with_cwd( - glob_pattern, - cwd, - false, - false, - false, - false, - true, - Some(ignored_workspace_paths), - )? { + let mut walker = match GlobWalker::init(glob_pattern) + .cwd(cwd) + .only_files(true) + .ignore_filter_fn(ignored_workspace_paths) + .call()? + { Ok(w) => w, Err(e) => { let _ = bun_ast::add_error_pretty!( @@ -573,5 +566,5 @@ fn ignored_workspace_paths(path: &[u8]) -> bool { // The ignore-filter is a runtime fn-pointer field on // `bun_glob::GlobWalker` (const-generic fn ptrs are unstable). Supplied via -// `init_with_cwd(..., Some(ignored_workspace_paths))`. +// `init(..).ignore_filter_fn(ignored_workspace_paths)`. type GlobWalker = glob::GlobWalker; diff --git a/src/install/npm.rs b/src/install/npm.rs index 4116d4fa4fcd..15bb0c841954 100644 --- a/src/install/npm.rs +++ b/src/install/npm.rs @@ -152,17 +152,15 @@ pub fn whoami(manager: &mut PackageManager) -> Result, WhoamiError> { // `&[]` when unallocated, matching the previous `None => b""` arm). let header_buf: &[u8] = headers.content.written_slice(); - let mut req = AsyncHTTP::init_sync( - http::Method::GET, - url, - headers.entries, - header_buf, - &raw mut response_buf, - b"", - None, - None, - http::FetchRedirect::Follow, - ); + let mut req = AsyncHTTP::init_sync() + .method(http::Method::GET) + .url(url) + .headers(headers.entries) + .headers_buf(header_buf) + .response_buffer(&raw mut response_buf) + .request_body(b"") + .redirect_type(http::FetchRedirect::Follow) + .call(); let res = match req.send_sync() { Ok(res) => res, @@ -555,16 +553,19 @@ pub mod registry { new_etag = &new_etag_buf[..new_etag.len()]; } - if let Some(package) = PackageManifest::parse( - scope, - log, - body, - package_name, - newly_last_modified, - new_etag, - (u64::try_from(bun_core::time::timestamp().max(0)).expect("int cast") as u32) + 300, - is_extended_manifest, - )? { + if let Some(package) = PackageManifest::parse() + .scope(scope) + .log(log) + .json_buffer(body) + .expected_name(package_name) + .last_modified(newly_last_modified) + .etag(new_etag) + .public_max_age( + (u64::try_from(bun_core::time::timestamp().max(0)).expect("int cast") as u32) + 300, + ) + .is_extended_manifest(is_extended_manifest) + .call()? + { if package_manager.options.enable.manifest_cache() { package_manifest::Serializer::save_async( &package, @@ -1962,8 +1963,14 @@ const DEPENDENCY_GROUPS: [DependencyGroup; 3] = [ DependencyGroup::PEER, ]; +#[bon::bon] impl PackageManifest { /// This parses [Abbreviated metadata](https://github.com/npm/registry/blob/master/docs/responses/package-metadata.md#abbreviated-metadata-format) + /// + /// Named setters: `json_buffer`/`expected_name`/`last_modified`/`etag` + /// are four consecutive `&[u8]` parameters that positional arguments + /// could transpose. + #[builder] pub fn parse( scope: ®istry::Scope, log: &mut bun_ast::Log, diff --git a/src/runtime/Cargo.toml b/src/runtime/Cargo.toml index fcbdce7f066e..4478eefb6a2f 100644 --- a/src/runtime/Cargo.toml +++ b/src/runtime/Cargo.toml @@ -10,6 +10,7 @@ path = "lib.rs" workspace = true [dependencies] +bon.workspace = true bun_opaque.workspace = true rust-argon2.workspace = true bcrypt.workspace = true diff --git a/src/runtime/api/glob.rs b/src/runtime/api/glob.rs index f0e6bd58a993..7b48de42bce6 100644 --- a/src/runtime/api/glob.rs +++ b/src/runtime/api/glob.rs @@ -291,9 +291,6 @@ fn glob_walk_result_to_js( } impl Glob { - /// The reference to the arena is not used after the scope because it is copied - /// by `GlobWalker.init`/`GlobWalker.initWithCwd` if all allocations work and no - /// errors occur fn make_glob_walker( &self, global_this: &JSGlobalObject, @@ -313,34 +310,18 @@ impl Glob { let _ = arena; // arena ownership is no longer threaded through GlobWalker init. - if let Some(cwd) = cwd { - let glob_walker = match GlobWalker::init_with_cwd( - &self.pattern, - &cwd, - dot, - absolute, - follow_symlinks, - error_on_broken_symlinks, - only_files, - None, - )? { - bun_sys::Result::Err(err) => { - return Err(global_this.throw_value(err.to_js(global_this))); - } - bun_sys::Result::Ok(gw) => Box::new(gw), - }; - return Ok(Some(glob_walker)); - } - - let glob_walker = match GlobWalker::init( - &self.pattern, - dot, - absolute, - follow_symlinks, - error_on_broken_symlinks, - only_files, - None, - )? { + // `maybe_cwd(None)` falls through to the builder's default (the + // process top-level dir), so the "with cwd" / "without cwd" call + // sites collapse into one. + let glob_walker = match GlobWalker::init(&self.pattern) + .maybe_cwd(cwd.as_deref()) + .dot(dot) + .absolute(absolute) + .follow_symlinks(follow_symlinks) + .error_on_broken_symlinks(error_on_broken_symlinks) + .only_files(only_files) + .call()? + { bun_sys::Result::Err(err) => { return Err(global_this.throw_value(err.to_js(global_this))); } @@ -412,8 +393,8 @@ impl Glob { // `arguments` drops at scope exit. let mut arena = Arena::new(); - // GlobWalker::init/init_with_cwd own their allocations (Box); the - // arena here is vestigial. + // GlobWalker::init owns its allocations (Box); the arena here is + // vestigial. let glob_walker = match self.make_glob_walker(global_this, &mut arguments, "scan", &mut arena) { Err(err) => { diff --git a/src/runtime/cli/audit_command.rs b/src/runtime/cli/audit_command.rs index 62c33d490159..61c214fc28ee 100644 --- a/src/runtime/cli/audit_command.rs +++ b/src/runtime/cli/audit_command.rs @@ -476,17 +476,16 @@ fn send_audit_request( let mut response_buf = MutableString::init(1024)?; // `init_sync` erases lifetimes internally (port-erased raw pointers); all // borrowed inputs live on this stack frame past `send_sync()`. - let mut req = http::AsyncHTTP::init_sync( - http::Method::POST, - url, - headers.entries, - headers_buf, - &raw mut response_buf, - &final_compressed_body, - http_proxy, - None, - http::FetchRedirect::Follow, - ); + let mut req = http::AsyncHTTP::init_sync() + .method(http::Method::POST) + .url(url) + .headers(headers.entries) + .headers_buf(headers_buf) + .response_buffer(&raw mut response_buf) + .request_body(&final_compressed_body) + .maybe_http_proxy(http_proxy) + .redirect_type(http::FetchRedirect::Follow) + .call(); let res = match req.send_sync() { Ok(r) => r, Err(err) => { diff --git a/src/runtime/cli/create_command.rs b/src/runtime/cli/create_command.rs index d32f2fc2f56f..08e097e3b0f3 100644 --- a/src/runtime/cli/create_command.rs +++ b/src/runtime/cli/create_command.rs @@ -2301,17 +2301,18 @@ impl Example { crate::cli::cli_arena().alloc(MutableString::init(8192)?); // ensure very stable memory address - let mut async_http = Box::new(HTTP::AsyncHTTP::init_sync( - HTTP::Method::GET, - api_url, - header_entries, - headers_buf, - mutable, - b"", - http_proxy, - None, - HTTP::FetchRedirect::Follow, - )); + let mut async_http = Box::new( + HTTP::AsyncHTTP::init_sync() + .method(HTTP::Method::GET) + .url(api_url) + .headers(header_entries) + .headers_buf(headers_buf) + .response_buffer(mutable) + .request_body(b"") + .maybe_http_proxy(http_proxy) + .redirect_type(HTTP::FetchRedirect::Follow) + .call(), + ); async_http.client.progress_node = Some(core::ptr::NonNull::from(&mut *progress)); async_http.client.flags.reject_unauthorized = env_loader.get_tls_reject_unauthorized(); @@ -2402,19 +2403,19 @@ impl Example { .map(|u| unsafe { u.erase_lifetime() }); // ensure very stable memory address - let async_http: &mut HTTP::AsyncHTTP = - crate::cli::cli_arena().alloc(HTTP::AsyncHTTP::init_sync( - HTTP::Method::GET, + let async_http: &mut HTTP::AsyncHTTP = crate::cli::cli_arena().alloc( + HTTP::AsyncHTTP::init_sync() + .method(HTTP::Method::GET) // SAFETY: single-threaded CLI access to static URL_ (set just above) - unsafe { (*URL_.get()).clone() }.unwrap(), - Default::default(), - b"", - mutable, - b"", - http_proxy, - None, - HTTP::FetchRedirect::Follow, - )); + .url(unsafe { (*URL_.get()).clone() }.unwrap()) + .headers(Default::default()) + .headers_buf(b"") + .response_buffer(mutable) + .request_body(b"") + .maybe_http_proxy(http_proxy) + .redirect_type(HTTP::FetchRedirect::Follow) + .call(), + ); async_http.client.progress_node = Some(core::ptr::NonNull::from(&mut *progress)); async_http.client.flags.reject_unauthorized = env_loader.get_tls_reject_unauthorized(); @@ -2498,17 +2499,16 @@ impl Example { .get_http_proxy_for(&parsed_tarball_url) .map(|u| unsafe { u.erase_lifetime() }); - *async_http = HTTP::AsyncHTTP::init_sync( - HTTP::Method::GET, - parsed_tarball_url, - Default::default(), - b"", - mutable, - b"", - http_proxy, - None, - HTTP::FetchRedirect::Follow, - ); + *async_http = HTTP::AsyncHTTP::init_sync() + .method(HTTP::Method::GET) + .url(parsed_tarball_url) + .headers(Default::default()) + .headers_buf(b"") + .response_buffer(mutable) + .request_body(b"") + .maybe_http_proxy(http_proxy) + .redirect_type(HTTP::FetchRedirect::Follow) + .call(); async_http.client.progress_node = Some(core::ptr::NonNull::from(&mut *progress)); async_http.client.flags.reject_unauthorized = env_loader.get_tls_reject_unauthorized(); @@ -2544,17 +2544,18 @@ impl Example { let mutable: &'static mut MutableString = crate::cli::cli_arena().alloc(MutableString::init(2048)?); - let mut async_http = Box::new(HTTP::AsyncHTTP::init_sync( - HTTP::Method::GET, - url, - Default::default(), - b"", - mutable, - b"", - http_proxy, - None, - HTTP::FetchRedirect::Follow, - )); + let mut async_http = Box::new( + HTTP::AsyncHTTP::init_sync() + .method(HTTP::Method::GET) + .url(url) + .headers(Default::default()) + .headers_buf(b"") + .response_buffer(mutable) + .request_body(b"") + .maybe_http_proxy(http_proxy) + .redirect_type(HTTP::FetchRedirect::Follow) + .call(), + ); async_http.client.flags.reject_unauthorized = env_loader.get_tls_reject_unauthorized(); if Output::enable_ansi_colors_stdout() { diff --git a/src/runtime/cli/filter_arg.rs b/src/runtime/cli/filter_arg.rs index 9f3bc1222598..76546ada91b8 100644 --- a/src/runtime/cli/filter_arg.rs +++ b/src/runtime/cli/filter_arg.rs @@ -31,7 +31,7 @@ fn glob_ignore_fn(val: &[u8]) -> bool { false } -// The ignore filter is a runtime parameter on `init_with_cwd`, and +// The ignore filter is a runtime parameter on `init`, and // `DirEntryAccessor` lives in `bun_resolver` (it depends on the resolver's // DirEntry cache). type GlobWalker = glob::GlobWalker; @@ -299,16 +299,14 @@ impl PackageFilterIterator { // bun_glob copies `pattern`/`cwd` internally. let cwd: &[u8] = self.root_dir.slice(); // outer `?` propagates the error, inner converts `Maybe(Self)` to a Result. - let walker = GlobWalker::init_with_cwd( - pattern, - cwd, - true, - true, - false, - true, - true, - Some(glob_ignore_fn), - )??; + let walker = GlobWalker::init(pattern) + .cwd(cwd) + .dot(true) + .absolute(true) + .error_on_broken_symlinks(true) + .only_files(true) + .ignore_filter_fn(glob_ignore_fn) + .call()??; // Heap-allocate the walker so its address is stable even if `self` moves between // `init_walker` and the iterator's last use. `iter` holds a `'static`-erased `&mut` // into this allocation; `deinit_walker` drops `iter` before freeing the walker. diff --git a/src/runtime/cli/pack_command.rs b/src/runtime/cli/pack_command.rs index 8e5290cb119c..be853b379368 100644 --- a/src/runtime/cli/pack_command.rs +++ b/src/runtime/cli/pack_command.rs @@ -3037,18 +3037,19 @@ fn run_lifecycle_script( // while a lifecycle script runs (single-threaded CLI dispatch). let command_ctx = unsafe { &mut *std::ptr::from_ref(ctx.command_ctx).cast_mut() }; let use_system_shell = command_ctx.debug.use_system_shell; - match RunCommand::run_package_script_foreground( - command_ctx, - script, - name, - abs_workspace_path, + match RunCommand::run_package_script_foreground() + .ctx(command_ctx) + .original_script(script) + .name(name) + .cwd(abs_workspace_path) // SAFETY: `env` is non-null (set by `PackageManager::init` / // `configure_env_for_run`). - unsafe { &mut *env }, - &[], - silent, - use_system_shell, - ) { + .env(unsafe { &mut *env }) + .passthrough(&[]) + .silent(silent) + .use_system_shell(use_system_shell) + .call() + { Ok(_) => Ok(()), Err(err) => { if err == bun_core::err!("MissingShell") { diff --git a/src/runtime/cli/pm_version_command.rs b/src/runtime/cli/pm_version_command.rs index dd784389b430..599cf1a13d4b 100644 --- a/src/runtime/cli/pm_version_command.rs +++ b/src/runtime/cli/pm_version_command.rs @@ -154,16 +154,16 @@ impl PmVersionCommand { if let Some(s) = &scripts_obj { if let Some(script) = s.get(b"preversion") { if let Some(script_command) = script.as_string(&json_bump) { - RunCommand::run_package_script_foreground( - ctx, - script_command, - b"preversion", - &package_json_dir, - pm.env_mut(), - &[], - silent, - use_system_shell, - )?; + RunCommand::run_package_script_foreground() + .ctx(ctx) + .original_script(script_command) + .name(b"preversion") + .cwd(&package_json_dir) + .env(pm.env_mut()) + .passthrough(&[]) + .silent(silent) + .use_system_shell(use_system_shell) + .call()?; } } } @@ -234,16 +234,16 @@ impl PmVersionCommand { if let Some(s) = &scripts_obj { if let Some(script) = s.get(b"version") { if let Some(script_command) = script.as_string(&json_bump) { - RunCommand::run_package_script_foreground( - ctx, - script_command, - b"version", - &package_json_dir, - pm.env_mut(), - &[], - silent, - use_system_shell, - )?; + RunCommand::run_package_script_foreground() + .ctx(ctx) + .original_script(script_command) + .name(b"version") + .cwd(&package_json_dir) + .env(pm.env_mut()) + .passthrough(&[]) + .silent(silent) + .use_system_shell(use_system_shell) + .call()?; } } } @@ -255,16 +255,16 @@ impl PmVersionCommand { if let Some(s) = &scripts_obj { if let Some(script) = s.get(b"postversion") { if let Some(script_command) = script.as_string(&json_bump) { - RunCommand::run_package_script_foreground( - ctx, - script_command, - b"postversion", - &package_json_dir, - pm.env_mut(), - &[], - silent, - use_system_shell, - )?; + RunCommand::run_package_script_foreground() + .ctx(ctx) + .original_script(script_command) + .name(b"postversion") + .cwd(&package_json_dir) + .env(pm.env_mut()) + .passthrough(&[]) + .silent(silent) + .use_system_shell(use_system_shell) + .call()?; } } } diff --git a/src/runtime/cli/pm_view_command.rs b/src/runtime/cli/pm_view_command.rs index 75c011faf43c..5a49049a8c63 100644 --- a/src/runtime/cli/pm_view_command.rs +++ b/src/runtime/cli/pm_view_command.rs @@ -120,17 +120,16 @@ pub(crate) fn view( let mut response_buf = MutableString::init(2048)?; let header_buf: &[u8] = headers.content.written_slice(); let http_proxy = manager.http_proxy(&url); - let mut req = http::AsyncHTTP::init_sync( - http::Method::GET, - url, - headers.entries, - header_buf, - &raw mut response_buf, - b"", - http_proxy, - None, - http::FetchRedirect::Follow, - ); + let mut req = http::AsyncHTTP::init_sync() + .method(http::Method::GET) + .url(url) + .headers(headers.entries) + .headers_buf(header_buf) + .response_buffer(&raw mut response_buf) + .request_body(b"") + .maybe_http_proxy(http_proxy) + .redirect_type(http::FetchRedirect::Follow) + .call(); req.client.flags.reject_unauthorized = manager.tls_reject_unauthorized(); let res = match req.send_sync() { @@ -159,17 +158,20 @@ pub(crate) fn view( Global::crash(); } - // Parse the existing JSON response into a PackageManifest using the now-public parse function - let parsed_manifest = match PackageManifest::parse( - &scope, - &mut log, - response_buf.list.as_slice(), - name, - b"", // last_modified (not needed for view) - b"", // etag (not needed for view) - 0, // public_max_age (not needed for view) - true, // is_extended_manifest (view uses application/json Accept header) - ) { + // `pm view` reads the whole manifest, so the cache metadata members + // (`last_modified`/`etag`/`public_max_age`) are unused here. The + // `application/json` Accept header requests the extended manifest shape. + let parsed_manifest = match PackageManifest::parse() + .scope(&scope) + .log(&mut log) + .json_buffer(response_buf.list.as_slice()) + .expected_name(name) + .last_modified(b"") + .etag(b"") + .public_max_age(0) + .is_extended_manifest(true) + .call() + { Ok(Some(m)) => m, Ok(None) => { Output::err_generic("failed to parse package manifest", ()); diff --git a/src/runtime/cli/publish_command.rs b/src/runtime/cli/publish_command.rs index 715699dbf4bd..709abaa398c5 100644 --- a/src/runtime/cli/publish_command.rs +++ b/src/runtime/cli/publish_command.rs @@ -720,18 +720,19 @@ impl PublishCommand { let cmd_ctx_ptr: *mut crate::cli::command::ContextData = context.command_ctx; if let Some(publish_script) = &context.publish_script { - if let Err(e) = Run::run_package_script_foreground( + if let Err(e) = Run::run_package_script_foreground() // SAFETY: see above. - unsafe { &mut *cmd_ctx_ptr }, - publish_script, - b"publish", - &abs_workspace_path, - script_env, - &[], - context.manager.options.log_level == LogLevel::Silent, + .ctx(unsafe { &mut *cmd_ctx_ptr }) + .original_script(publish_script) + .name(b"publish") + .cwd(&abs_workspace_path) + .env(script_env) + .passthrough(&[]) + .silent(context.manager.options.log_level == LogLevel::Silent) // SAFETY: see above. - unsafe { &*cmd_ctx_ptr }.debug.use_system_shell, - ) { + .use_system_shell(unsafe { &*cmd_ctx_ptr }.debug.use_system_shell) + .call() + { if e == err!("MissingShell") { Output::err_generic( "failed to find shell executable to run publish script", @@ -744,18 +745,19 @@ impl PublishCommand { } if let Some(postpublish_script) = &context.postpublish_script { - if let Err(e) = Run::run_package_script_foreground( + if let Err(e) = Run::run_package_script_foreground() // SAFETY: see above. - unsafe { &mut *cmd_ctx_ptr }, - postpublish_script, - b"postpublish", - &abs_workspace_path, - script_env, - &[], - context.manager.options.log_level == LogLevel::Silent, + .ctx(unsafe { &mut *cmd_ctx_ptr }) + .original_script(postpublish_script) + .name(b"postpublish") + .cwd(&abs_workspace_path) + .env(script_env) + .passthrough(&[]) + .silent(context.manager.options.log_level == LogLevel::Silent) // SAFETY: see above. - unsafe { &*cmd_ctx_ptr }.debug.use_system_shell, - ) { + .use_system_shell(unsafe { &*cmd_ctx_ptr }.debug.use_system_shell) + .call() + { if e == err!("MissingShell") { Output::err_generic( "failed to find shell executable to run postpublish script", @@ -836,17 +838,15 @@ impl PublishCommand { headers.append(b"authorization", &auth_buf); } - let mut req = http::AsyncHTTP::init_sync( - http::Method::GET, - package_url, - headers.entries, - headers.content.written_slice(), - &raw mut response_buf, - b"", - None, - None, - http::FetchRedirect::Follow, - ); + let mut req = http::AsyncHTTP::init_sync() + .method(http::Method::GET) + .url(package_url) + .headers(headers.entries) + .headers_buf(headers.content.written_slice()) + .response_buffer(&raw mut response_buf) + .request_body(b"") + .redirect_type(http::FetchRedirect::Follow) + .call(); let Ok(res) = req.send_sync() else { return false; @@ -961,17 +961,15 @@ impl PublishCommand { let publish_url = URL::parse(crate::cli::cli_dupe(&print_buf)); print_buf.clear(); - let mut req = http::AsyncHTTP::init_sync( - http::Method::PUT, - publish_url.clone(), - publish_headers.entries, - publish_headers.content.written_slice(), - &raw mut response_buf, - publish_req_body, - None, - None, - http::FetchRedirect::Follow, - ); + let mut req = http::AsyncHTTP::init_sync() + .method(http::Method::PUT) + .url(publish_url.clone()) + .headers(publish_headers.entries) + .headers_buf(publish_headers.content.written_slice()) + .response_buffer(&raw mut response_buf) + .request_body(publish_req_body) + .redirect_type(http::FetchRedirect::Follow) + .call(); let res = match req.send_sync() { Ok(r) => r, @@ -1058,17 +1056,15 @@ impl PublishCommand { response_buf.reset(); - let mut otp_req = http::AsyncHTTP::init_sync( - http::Method::PUT, - publish_url, - otp_headers.entries, - otp_headers.content.written_slice(), - &raw mut response_buf, - publish_req_body, - None, - None, - http::FetchRedirect::Follow, - ); + let mut otp_req = http::AsyncHTTP::init_sync() + .method(http::Method::PUT) + .url(publish_url) + .headers(otp_headers.entries) + .headers_buf(otp_headers.content.written_slice()) + .response_buffer(&raw mut response_buf) + .request_body(publish_req_body) + .redirect_type(http::FetchRedirect::Follow) + .call(); let otp_res = match otp_req.send_sync() { Ok(r) => r, @@ -1277,17 +1273,15 @@ impl PublishCommand { // Note: `done_url`/`auth_headers.entries` move into // `init_sync`, so re-clone per iteration. - let mut req = http::AsyncHTTP::init_sync( - http::Method::GET, - done_url.clone(), - auth_headers.entries.clone()?, - auth_headers.content.written_slice(), - response_buf, - b"", - None, - None, - http::FetchRedirect::Follow, - ); + let mut req = http::AsyncHTTP::init_sync() + .method(http::Method::GET) + .url(done_url.clone()) + .headers(auth_headers.entries.clone()?) + .headers_buf(auth_headers.content.written_slice()) + .response_buffer(response_buf) + .request_body(b"") + .redirect_type(http::FetchRedirect::Follow) + .call(); let res = match req.send_sync() { Ok(r) => r, diff --git a/src/runtime/cli/run_command.rs b/src/runtime/cli/run_command.rs index 49b21f9613dc..6e02f891f5bc 100644 --- a/src/runtime/cli/run_command.rs +++ b/src/runtime/cli/run_command.rs @@ -226,11 +226,21 @@ Full documentation is available at https://bun.com/docs/cli/run ) -> Result<(), bun_core::Error> { bun_install::lifecycle_script_runner::replace_package_manager_run(copy_script, script) } +} +// Separate impl block so `#[bon::bon]` only re-emits this one method, not +// the rest of the (large) `RunCommand` impl above. +#[bon::bon] +impl RunCommand { /// Spawns the script body via the bun-shell or system shell and exits on /// non-zero. /// /// `passthrough` is `&[Box<[u8]>]` to match `ctx.passthrough` directly. + /// + /// Named setters: `original_script`/`name`/`cwd` are all `&[u8]` and + /// `silent`/`use_system_shell` are both `bool`; positional arguments + /// could transpose either group. + #[builder] pub fn run_package_script_foreground( ctx: &mut ContextData, original_script: &[u8], @@ -240,31 +250,8 @@ Full documentation is available at https://bun.com/docs/cli/run passthrough: &[Box<[u8]>], silent: bool, use_system_shell: bool, - ) -> Result<(), bun_core::Error> { - Self::run_package_script_foreground_with_shell_path( - ctx, - original_script, - name, - cwd, - env, - passthrough, - silent, - use_system_shell, - None, - ) - } - - /// Like [`Self::run_package_script_foreground`], but resolves the shell - /// interpreter from `shell_path` instead of the loader's `PATH`. - pub fn run_package_script_foreground_with_shell_path( - ctx: &mut ContextData, - original_script: &[u8], - name: &[u8], - cwd: &[u8], - env: &mut DotEnv::Loader<'_>, - passthrough: &[Box<[u8]>], - silent: bool, - use_system_shell: bool, + /// Resolve the shell interpreter from here instead of the loader's + /// `PATH`. shell_path: Option<&[u8]>, ) -> Result<(), bun_core::Error> { let shell_search_path = shell_path.unwrap_or_else(|| env.get(b"PATH").unwrap_or(b"")); @@ -519,7 +506,9 @@ Full documentation is available at https://bun.com/docs/cli/run Ok(()) } +} +impl RunCommand { /// Allocates a /// process-lifetime `Transpiler`, primes its resolver/env, reads the /// top-level `DirInfo`, configures the bundler linker / JSX runtime, and @@ -2516,45 +2505,45 @@ impl RunCommand { let use_system_shell = ctx.debug.use_system_shell; if let Some(&prescript) = scripts.get(&temp_script_buffer[1..]) { - Self::run_package_script_foreground_with_shell_path( - ctx, - prescript, - &temp_script_buffer[1..], - package_json_dir, - env_loader, - &[], - silent, - use_system_shell, - Some(original_path.as_slice()), - )?; + Self::run_package_script_foreground() + .ctx(ctx) + .original_script(prescript) + .name(&temp_script_buffer[1..]) + .cwd(package_json_dir) + .env(env_loader) + .passthrough(&[]) + .silent(silent) + .use_system_shell(use_system_shell) + .shell_path(original_path.as_slice()) + .call()?; } - Self::run_package_script_foreground_with_shell_path( - ctx, - script_content, - target_name, - package_json_dir, - env_loader, - &passthrough, - silent, - use_system_shell, - Some(original_path.as_slice()), - )?; + Self::run_package_script_foreground() + .ctx(ctx) + .original_script(script_content) + .name(target_name) + .cwd(package_json_dir) + .env(env_loader) + .passthrough(&passthrough) + .silent(silent) + .use_system_shell(use_system_shell) + .shell_path(original_path.as_slice()) + .call()?; temp_script_buffer[..b"post".len()].copy_from_slice(b"post"); if let Some(&postscript) = scripts.get(&temp_script_buffer[..]) { - Self::run_package_script_foreground_with_shell_path( - ctx, - postscript, - &temp_script_buffer, - package_json_dir, - env_loader, - &[], - silent, - use_system_shell, - Some(original_path.as_slice()), - )?; + Self::run_package_script_foreground() + .ctx(ctx) + .original_script(postscript) + .name(&temp_script_buffer) + .cwd(package_json_dir) + .env(env_loader) + .passthrough(&[]) + .silent(silent) + .use_system_shell(use_system_shell) + .shell_path(original_path.as_slice()) + .call()?; } return Ok(true); @@ -3310,20 +3299,21 @@ impl RunCommand { let response_buffer_ptr: *mut bun_core::MutableString = unsafe { ::core::ptr::addr_of_mut!((*slot).response_buffer) }; let d_ptr: *mut RemoteImageDownload = slot; - let async_http = bun_http::AsyncHTTP::init( - bun_http::Method::GET, - bun_url::URL::parse(url_static), - Default::default(), - b"", - response_buffer_ptr, - b"", - bun_http::HTTPClientResultCallback::new::( - d_ptr, - RemoteImageDownload::on_done, - ), - bun_http::FetchRedirect::Follow, - Default::default(), - ); + let async_http = bun_http::AsyncHTTP::init() + .method(bun_http::Method::GET) + .url(bun_url::URL::parse(url_static)) + .headers(Default::default()) + .headers_buf(b"") + .response_buffer(response_buffer_ptr) + .request_body(b"") + .callback( + bun_http::HTTPClientResultCallback::new::( + d_ptr, + RemoteImageDownload::on_done, + ), + ) + .redirect_type(bun_http::FetchRedirect::Follow) + .call(); // SAFETY: last field — all four fields are now initialized. unsafe { ::core::ptr::addr_of_mut!((*slot).async_http).write(async_http) }; // SAFETY: every field of `RemoteImageDownload` was `ptr::write`n above. diff --git a/src/runtime/cli/upgrade_command.rs b/src/runtime/cli/upgrade_command.rs index 81d47e4ec1b1..36b683b4bd69 100644 --- a/src/runtime/cli/upgrade_command.rs +++ b/src/runtime/cli/upgrade_command.rs @@ -285,17 +285,18 @@ impl UpgradeCommand { let headers_buf: &'static [u8] = crate::cli::cli_dupe(&headers_buf); // ensure very stable memory address - let mut async_http = Box::new(HTTP::AsyncHTTP::init_sync( - HTTP::Method::GET, - api_url, - header_entries, - headers_buf, - std::ptr::from_mut::(metadata_body), - b"", - http_proxy, - None, - HTTP::FetchRedirect::Follow, - )); + let mut async_http = Box::new( + HTTP::AsyncHTTP::init_sync() + .method(HTTP::Method::GET) + .url(api_url) + .headers(header_entries) + .headers_buf(headers_buf) + .response_buffer(std::ptr::from_mut::(metadata_body)) + .request_body(b"") + .maybe_http_proxy(http_proxy) + .redirect_type(HTTP::FetchRedirect::Follow) + .call(), + ); async_http.client.flags.reject_unauthorized = env_loader.get_tls_reject_unauthorized(); if !SILENT { @@ -683,17 +684,18 @@ impl UpgradeCommand { let zip_file_buffer: &'static mut MutableString = crate::cli::cli_arena() .alloc(MutableString::init(version.size.max(1024) as usize)?); - let mut async_http = Box::new(HTTP::AsyncHTTP::init_sync( - HTTP::Method::GET, - zip_url, - headers::EntryList::default(), - b"", - std::ptr::from_mut::(zip_file_buffer), - b"", - http_proxy, - None, - HTTP::FetchRedirect::Follow, - )); + let mut async_http = Box::new( + HTTP::AsyncHTTP::init_sync() + .method(HTTP::Method::GET) + .url(zip_url) + .headers(headers::EntryList::default()) + .headers_buf(b"") + .response_buffer(std::ptr::from_mut::(zip_file_buffer)) + .request_body(b"") + .maybe_http_proxy(http_proxy) + .redirect_type(HTTP::FetchRedirect::Follow) + .call(), + ); // `progress` is intentionally leaked (process-lifetime), so the // untracked NonNull stored in `progress_node` can never dangle. async_http.client.progress_node = diff --git a/src/runtime/server/ServerWebSocket.rs b/src/runtime/server/ServerWebSocket.rs index d1858f15e7ba..976f1dac2fe3 100644 --- a/src/runtime/server/ServerWebSocket.rs +++ b/src/runtime/server/ServerWebSocket.rs @@ -263,10 +263,19 @@ impl ServerWebSocket { } Ok(args_len > 1 && compress_value.to_boolean()) } +} +// Separate impl block so `#[bon::bon]` only re-emits `do_publish`, not the +// `#[bun_jsc::host_fn]` methods in the block above. +#[bon::bon] +impl ServerWebSocket { /// Route a publish through either the per-socket uWS handle (when /// `!publish_to_self && !closed`) or the app-wide broadcast, then map the /// aggregated `SendStatus` to the JS number contract shared with `send()`. + /// + /// Named setters: `ssl`/`publish_to_self`/`compress` are all `bool` and + /// `topic`/`buffer` are both `&[u8]`; positional args could transpose them. + #[builder] #[inline] fn do_publish( &self, @@ -285,7 +294,9 @@ impl ServerWebSocket { }; send_status_to_js(status, buffer.len(), "publish", "bytes") } +} +impl ServerWebSocket { /// Shared body for `subscribe` / `unsubscribe` / `isSubscribed`: identical /// arg-count guard, closed short-circuit, string-type guard, UTF-8 slice, /// non-empty guard, then dispatch to the uWS topic op. Only the JS-visible @@ -792,15 +803,16 @@ impl ServerWebSocket { if let Some(array_buffer) = message_value.as_array_buffer(global_this) { let buffer = array_buffer.slice(); - return Ok(self.do_publish( - ssl, - app, - publish_to_self, - topic_slice.slice(), - buffer, - Opcode::Binary, - compress, - )); + return Ok(self + .do_publish() + .ssl(ssl) + .app(app) + .publish_to_self(publish_to_self) + .topic(topic_slice.slice()) + .buffer(buffer) + .opcode(Opcode::Binary) + .compress(compress) + .call()); } { @@ -808,15 +820,16 @@ impl ServerWebSocket { let view = js_string.view(global_this); let slice = view.to_slice(); - let ret = self.do_publish( - ssl, - app, - publish_to_self, - topic_slice.slice(), - slice.slice(), - Opcode::Text, - compress, - ); + let ret = self + .do_publish() + .ssl(ssl) + .app(app) + .publish_to_self(publish_to_self) + .topic(topic_slice.slice()) + .buffer(slice.slice()) + .opcode(Opcode::Text) + .compress(compress) + .call(); js_string.ensure_still_alive(); Ok(ret) } @@ -862,15 +875,16 @@ impl ServerWebSocket { let view = js_string.view(global_this); let slice = view.to_slice(); - let ret = self.do_publish( - ssl, - app, - publish_to_self, - topic_slice.slice(), - slice.slice(), - Opcode::Text, - compress, - ); + let ret = self + .do_publish() + .ssl(ssl) + .app(app) + .publish_to_self(publish_to_self) + .topic(topic_slice.slice()) + .buffer(slice.slice()) + .opcode(Opcode::Text) + .compress(compress) + .call(); js_string.ensure_still_alive(); Ok(ret) } @@ -921,15 +935,16 @@ impl ServerWebSocket { return Err(global_this.throw(format_args!("publishBinary expects an ArrayBufferView"))); }; - Ok(self.do_publish( - ssl, - app, - publish_to_self, - topic_slice.slice(), - array_buffer.slice(), - Opcode::Binary, - compress, - )) + Ok(self + .do_publish() + .ssl(ssl) + .app(app) + .publish_to_self(publish_to_self) + .topic(topic_slice.slice()) + .buffer(array_buffer.slice()) + .opcode(Opcode::Binary) + .compress(compress) + .call()) } pub fn publish_binary_without_type_checks( @@ -953,15 +968,16 @@ impl ServerWebSocket { return Ok(JSValue::js_number(0.0)); } - Ok(self.do_publish( - ssl, - app, - publish_to_self, - topic_slice.slice(), - buffer, - Opcode::Binary, - true, - )) + Ok(self + .do_publish() + .ssl(ssl) + .app(app) + .publish_to_self(publish_to_self) + .topic(topic_slice.slice()) + .buffer(buffer) + .opcode(Opcode::Binary) + .compress(true) + .call()) } pub fn publish_text_without_type_checks( @@ -987,15 +1003,16 @@ impl ServerWebSocket { return Ok(JSValue::js_number(0.0)); } - Ok(self.do_publish( - ssl, - app, - publish_to_self, - topic_slice.slice(), - buffer, - Opcode::Text, - true, - )) + Ok(self + .do_publish() + .ssl(ssl) + .app(app) + .publish_to_self(publish_to_self) + .topic(topic_slice.slice()) + .buffer(buffer) + .opcode(Opcode::Text) + .compress(true) + .call()) } // `passThis: true` in server.classes.ts — wrapper is emitted by diff --git a/src/runtime/shell/builtin/cp.rs b/src/runtime/shell/builtin/cp.rs index 854e2bb415fb..fcfa2d9595f8 100644 --- a/src/runtime/shell/builtin/cp.rs +++ b/src/runtime/shell/builtin/cp.rs @@ -175,16 +175,16 @@ impl Cp { let interp_ptr = interp.as_ctx_ptr(); for i in start..target { let src = Builtin::of(interp, cmd).arg_bytes(i).to_vec(); - let task = ShellCpTask::create( - cmd, - evtloop, - opts, - operands, - src, - tgt.clone(), - cwd.clone(), - interp_ptr, - ); + let task = ShellCpTask::create() + .cmd(cmd) + .evtloop(evtloop) + .opts(opts) + .operands(operands) + .src(src) + .tgt(tgt.clone()) + .cwd_path(cwd.clone()) + .interp(interp_ptr) + .call(); // SAFETY: freshly heap-allocated. unsafe { ShellCpTask::schedule(task) }; } @@ -402,7 +402,13 @@ pub struct ShellCpTask { pub task: ShellTask, } +// Separate impl block so `#[bon::bon]` only re-emits `create`, not the rest +// of the (large) `ShellCpTask` impl below. +#[bon::bon] impl ShellCpTask { + /// Named setters: `src`/`tgt`/`cwd_path` are three consecutive `Vec` + /// parameters; transposing `src` and `tgt` in a `cp` is data loss. + #[builder] pub(crate) fn create( cmd: NodeId, evtloop: EventLoopHandle, @@ -431,7 +437,9 @@ impl ShellCpTask { task.task.interp = interp; bun_core::heap::into_raw(task) } +} +impl ShellCpTask { /// Appends `"{src} -> {dest}\n"` to the verbose /// buffer (printed to stdout once the cp finishes). Called from work-pool /// threads; serialised via `verbose_output`'s mutex. diff --git a/src/runtime/shell/states/Expansion.rs b/src/runtime/shell/states/Expansion.rs index 26181fd97115..2c6462bca303 100644 --- a/src/runtime/shell/states/Expansion.rs +++ b/src/runtime/shell/states/Expansion.rs @@ -432,9 +432,7 @@ impl Expansion { pattern = Self::neutralize_glob_metachars(&me.current_out, &me.meta_offsets); cwd = me.base.shell().cwd().to_vec(); } - let walker = match bun_glob::BunGlobWalkerZ::init_with_cwd( - &pattern, &cwd, false, false, false, false, false, None, - ) { + let walker = match bun_glob::BunGlobWalkerZ::init(&pattern).cwd(&cwd).call() { Ok(Ok(w)) => w, Ok(Err(e)) => { interp.as_expansion_mut(this).state = diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index fc5508d0c533..244f1cec8dbf 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -1466,28 +1466,26 @@ impl BlobExt for Blob { // credentials to the upload (`upload_stream` consumes an // `IntrusiveRc` by value, so the else-arm heap-dupes from the // store's `Arc` instead of from the `aws_options` clone). - return crate::webcore::__s3_client::upload_stream( - if extra_options.is_some() { - aws_options.credentials.dupe() - } else { - s3.get_credentials().dupe() - }, - path, - readable_stream, - global_this, - aws_options.options, - aws_options.acl, - aws_options.storage_class, - self.content_type_or_mime_type(), - // SAFETY: option-wrapped raw `*const [u8]` borrowed back; the - // backing storage is owned by `aws_options` which outlives this call. - aws_options.content_disposition.as_deref(), - aws_options.content_encoding.as_deref(), - proxy_url, - aws_options.request_payer, - None, - core::ptr::null_mut(), - ); + return crate::webcore::__s3_client::upload_stream(if extra_options.is_some() { + aws_options.credentials.dupe() + } else { + s3.get_credentials().dupe() + }) + .path(path) + .readable_stream(readable_stream) + .global_this(global_this) + .options(aws_options.options) + .maybe_acl(aws_options.acl) + .maybe_storage_class(aws_options.storage_class) + .maybe_content_type(self.content_type_or_mime_type()) + // SAFETY: option-wrapped raw `*const [u8]` borrowed back; the + // backing storage is owned by `aws_options` which outlives this call. + .maybe_content_disposition(aws_options.content_disposition.as_deref()) + .maybe_content_encoding(aws_options.content_encoding.as_deref()) + .maybe_proxy(proxy_url) + .request_payer(aws_options.request_payer) + .callback_context(core::ptr::null_mut()) + .call(); } if !matches!(store.data, store::Data::File(_)) { @@ -1832,30 +1830,27 @@ impl BlobExt for Blob { // MultiPartUpload derefs on done. return crate::webcore::s3::client::writable_stream( credentials_with_options.credentials.dupe(), - path, - global_this, - credentials_with_options.options, - self.content_type_or_mime_type(), - content_disposition_str.as_ref().map(|s| s.slice()), - content_encoding_str.as_ref().map(|s| s.slice()), - proxy, - credentials_with_options.storage_class, - credentials_with_options.request_payer, - ); - } - - return crate::webcore::s3::client::writable_stream( - s3.get_credentials().dupe(), - path, - global_this, - Default::default(), - self.content_type_or_mime_type(), - None, - None, - proxy, - None, - s3.request_payer, - ); + ) + .path(path) + .global_this(global_this) + .options(credentials_with_options.options) + .maybe_content_type(self.content_type_or_mime_type()) + .maybe_content_disposition(content_disposition_str.as_ref().map(|s| s.slice())) + .maybe_content_encoding(content_encoding_str.as_ref().map(|s| s.slice())) + .maybe_proxy(proxy) + .maybe_storage_class(credentials_with_options.storage_class) + .request_payer(credentials_with_options.request_payer) + .call(); + } + + return crate::webcore::s3::client::writable_stream(s3.get_credentials().dupe()) + .path(path) + .global_this(global_this) + .options(Default::default()) + .maybe_content_type(self.content_type_or_mime_type()) + .maybe_proxy(proxy) + .request_payer(s3.request_payer) + .call(); } #[cfg(windows)] @@ -4676,27 +4671,28 @@ fn write_file_with_empty_source_to_destination( let promise_value = promise.value(); let proxy_owned = http_proxy_href(ctx); let proxy_url = proxy_owned.as_deref(); - s3_client::upload( - &aws_options.credentials, - s3.path(), - b"", - destination_blob.content_type_or_mime_type(), + s3_client::upload(&aws_options.credentials) + .path(s3.path()) + .content(b"") + .maybe_content_type(destination_blob.content_type_or_mime_type()) // SAFETY: `*const [u8]` borrows from sibling `_*_slice` fields // on `aws_options`, which outlives this call. - aws_options.content_disposition.as_deref(), - aws_options.content_encoding.as_deref(), - aws_options.acl, - proxy_url, - aws_options.storage_class, - aws_options.request_payer, - Wrapper::resolve, - bun_core::heap::into_raw(Box::new(Wrapper { - promise, - store: destination_store.clone(), - global: bun_ptr::BackRef::new(ctx), - })) - .cast::(), - )?; + .maybe_content_disposition(aws_options.content_disposition.as_deref()) + .maybe_content_encoding(aws_options.content_encoding.as_deref()) + .maybe_acl(aws_options.acl) + .maybe_proxy_url(proxy_url) + .maybe_storage_class(aws_options.storage_class) + .request_payer(aws_options.request_payer) + .callback(Wrapper::resolve) + .callback_context( + bun_core::heap::into_raw(Box::new(Wrapper { + promise, + store: destination_store.clone(), + global: bun_ptr::BackRef::new(ctx), + })) + .cast::(), + ) + .call()?; return Ok(promise_value); } // Writing to a buffer-backed blob should be a type error, @@ -4891,28 +4887,26 @@ pub fn write_file_with_source_destination( )?, ctx, )? { - return s3_client::upload_stream( - if options.extra_options.is_some() { - aws_options.credentials.dupe() - } else { - s3.get_credentials().dupe() - }, - s3.path(), - stream, - ctx, - aws_options.options, - aws_options.acl, - aws_options.storage_class, - destination_blob.content_type_or_mime_type(), - // SAFETY: `*const [u8]` borrows from sibling `_*_slice` - // fields on `aws_options`, which outlives this call. - aws_options.content_disposition.as_deref(), - aws_options.content_encoding.as_deref(), - proxy_url, - aws_options.request_payer, - None, - core::ptr::null_mut(), - ); + return s3_client::upload_stream(if options.extra_options.is_some() { + aws_options.credentials.dupe() + } else { + s3.get_credentials().dupe() + }) + .path(s3.path()) + .readable_stream(stream) + .global_this(ctx) + .options(aws_options.options) + .maybe_acl(aws_options.acl) + .maybe_storage_class(aws_options.storage_class) + .maybe_content_type(destination_blob.content_type_or_mime_type()) + // SAFETY: `*const [u8]` borrows from sibling `_*_slice` + // fields on `aws_options`, which outlives this call. + .maybe_content_disposition(aws_options.content_disposition.as_deref()) + .maybe_content_encoding(aws_options.content_encoding.as_deref()) + .maybe_proxy(proxy_url) + .request_payer(aws_options.request_payer) + .callback_context(core::ptr::null_mut()) + .call(); } else { return Ok(JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( ctx, @@ -4957,27 +4951,28 @@ pub fn write_file_with_source_destination( } let promise = jsc::JSPromiseStrong::init(ctx); let promise_value = promise.value(); - s3_client::upload( - &aws_options.credentials, - s3.path(), - bytes.slice(), - destination_blob.content_type_or_mime_type(), + s3_client::upload(&aws_options.credentials) + .path(s3.path()) + .content(bytes.slice()) + .maybe_content_type(destination_blob.content_type_or_mime_type()) // SAFETY: `*const [u8]` borrows from sibling `_*_slice` fields // on `aws_options`, which outlives this call. - aws_options.content_disposition.as_deref(), - aws_options.content_encoding.as_deref(), - aws_options.acl, - proxy_url, - aws_options.storage_class, - aws_options.request_payer, - Wrapper::resolve, - bun_core::heap::into_raw(Box::new(Wrapper { - store: source_store.clone(), - promise, - global: bun_ptr::BackRef::new(ctx), - })) - .cast::(), - )?; + .maybe_content_disposition(aws_options.content_disposition.as_deref()) + .maybe_content_encoding(aws_options.content_encoding.as_deref()) + .maybe_acl(aws_options.acl) + .maybe_proxy_url(proxy_url) + .maybe_storage_class(aws_options.storage_class) + .request_payer(aws_options.request_payer) + .callback(Wrapper::resolve) + .callback_context( + bun_core::heap::into_raw(Box::new(Wrapper { + store: source_store.clone(), + promise, + global: bun_ptr::BackRef::new(ctx), + })) + .cast::(), + ) + .call()?; return Ok(promise_value); } } @@ -4991,28 +4986,26 @@ pub fn write_file_with_source_destination( )?, ctx, )? { - return s3_client::upload_stream( - if options.extra_options.is_some() { - aws_options.credentials.dupe() - } else { - s3.get_credentials().dupe() - }, - s3.path(), - stream, - ctx, - s3.options, - aws_options.acl, - aws_options.storage_class, - destination_blob.content_type_or_mime_type(), - // SAFETY: `*const [u8]` borrows from sibling `_*_slice` fields - // on `aws_options`, which outlives this call. - aws_options.content_disposition.as_deref(), - aws_options.content_encoding.as_deref(), - proxy_url, - aws_options.request_payer, - None, - core::ptr::null_mut(), - ); + return s3_client::upload_stream(if options.extra_options.is_some() { + aws_options.credentials.dupe() + } else { + s3.get_credentials().dupe() + }) + .path(s3.path()) + .readable_stream(stream) + .global_this(ctx) + .options(s3.options) + .maybe_acl(aws_options.acl) + .maybe_storage_class(aws_options.storage_class) + .maybe_content_type(destination_blob.content_type_or_mime_type()) + // SAFETY: `*const [u8]` borrows from sibling `_*_slice` fields + // on `aws_options`, which outlives this call. + .maybe_content_disposition(aws_options.content_disposition.as_deref()) + .maybe_content_encoding(aws_options.content_encoding.as_deref()) + .maybe_proxy(proxy_url) + .request_payer(aws_options.request_payer) + .callback_context(core::ptr::null_mut()) + .call(); } else { return Ok( JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( @@ -5189,112 +5182,115 @@ pub fn write_file_internal( // `Response` and `Request` both expose `get_body_value()` / // `get_body_readable_stream()`; one helper takes the // body-value pointer and a `get_stream` closure. - let mut body_dispatch = - |body_value: *mut webcore::body::Value, - get_stream: &mut dyn FnMut(&JSGlobalObject) -> Option| - -> JsResult> { - use core::ops::ControlFlow; - use webcore::body::Value as BodyValue; - // SAFETY: `body_value` is `&mut Body::Value` from a live JS heap - // Response/Request `m_ctx`; raw to allow re-borrow after `use_()`. - let body_value_ref = unsafe { &mut *body_value }; - match body_value_ref { - BodyValue::WTFStringImpl(_) - | BodyValue::InternalBlob(_) - | BodyValue::Used - | BodyValue::Empty - | BodyValue::Blob(_) - | BodyValue::Null => Ok(ControlFlow::Continue(body_value_ref.use_())), - BodyValue::Error(err_ref) => { - let err_js = err_ref.to_js(global_this); - destination_blob.detach(); - // SAFETY: `body_value` points into a live JS-heap Body; re-borrowed - // after `err_ref` is consumed so no `&mut` alias remains active. - let _ = unsafe { &mut *body_value }.use_(); - Ok(ControlFlow::Break( + let mut body_dispatch = |body_value: *mut webcore::body::Value, + get_stream: &mut dyn FnMut( + &JSGlobalObject, + ) -> Option| + -> JsResult> { + use core::ops::ControlFlow; + use webcore::body::Value as BodyValue; + // SAFETY: `body_value` is `&mut Body::Value` from a live JS heap + // Response/Request `m_ctx`; raw to allow re-borrow after `use_()`. + let body_value_ref = unsafe { &mut *body_value }; + match body_value_ref { + BodyValue::WTFStringImpl(_) + | BodyValue::InternalBlob(_) + | BodyValue::Used + | BodyValue::Empty + | BodyValue::Blob(_) + | BodyValue::Null => Ok(ControlFlow::Continue(body_value_ref.use_())), + BodyValue::Error(err_ref) => { + let err_js = err_ref.to_js(global_this); + destination_blob.detach(); + // SAFETY: `body_value` points into a live JS-heap Body; re-borrowed + // after `err_ref` is consumed so no `&mut` alias remains active. + let _ = unsafe { &mut *body_value }.use_(); + Ok(ControlFlow::Break( JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, err_js, + global_this, + err_js, ), )) - } - BodyValue::Locked(_) => { - if destination_blob.is_s3() { - let dest_store = destination_blob - .store() - .expect("infallible: store present") - .clone(); - let s3 = dest_store.data.as_s3(); - let aws_options = s3 - .get_credentials_with_options(options.extra_options, global_this)?; - let _ = body_value_ref.to_readable_stream(global_this)?; - let readable_opt = get_stream(global_this).or_else(|| { - // SAFETY: re-borrow after `to_readable_stream`. - let BodyValue::Locked(locked) = (unsafe { &mut *body_value }) - else { - return None; - }; - locked.readable.get(global_this) - }); - if let Some(readable) = readable_opt { - if readable.is_disturbed(global_this) { - destination_blob.detach(); - return Err(global_this.throw_invalid_arguments(format_args!( - "ReadableStream has already been used" - ))); - } - let proxy_owned = http_proxy_href(global_this); - let proxy_url = proxy_owned.as_deref(); - return Ok(ControlFlow::Break(s3_client::upload_stream( - if options.extra_options.is_some() { - aws_options.credentials.dupe() - } else { - s3.get_credentials().dupe() - }, - s3.path(), - readable, - global_this, - aws_options.options, - aws_options.acl, - aws_options.storage_class, - destination_blob.content_type_or_mime_type(), - // SAFETY: `*const [u8]` borrows from sibling - // `_*_slice` fields on `aws_options`, which - // outlives this call. - aws_options.content_disposition.as_deref(), - aws_options.content_encoding.as_deref(), - proxy_url, - aws_options.request_payer, - None, - core::ptr::null_mut(), - )?)); + } + BodyValue::Locked(_) => { + if destination_blob.is_s3() { + let dest_store = destination_blob + .store() + .expect("infallible: store present") + .clone(); + let s3 = dest_store.data.as_s3(); + let aws_options = + s3.get_credentials_with_options(options.extra_options, global_this)?; + let _ = body_value_ref.to_readable_stream(global_this)?; + let readable_opt = get_stream(global_this).or_else(|| { + // SAFETY: re-borrow after `to_readable_stream`. + let BodyValue::Locked(locked) = (unsafe { &mut *body_value }) else { + return None; + }; + locked.readable.get(global_this) + }); + if let Some(readable) = readable_opt { + if readable.is_disturbed(global_this) { + destination_blob.detach(); + return Err(global_this.throw_invalid_arguments(format_args!( + "ReadableStream has already been used" + ))); } - destination_blob.detach(); - return Err(global_this.throw_invalid_arguments(format_args!( - "ReadableStream has already been used" - ))); + let proxy_owned = http_proxy_href(global_this); + let proxy_url = proxy_owned.as_deref(); + return Ok(ControlFlow::Break( + s3_client::upload_stream(if options.extra_options.is_some() { + aws_options.credentials.dupe() + } else { + s3.get_credentials().dupe() + }) + .path(s3.path()) + .readable_stream(readable) + .global_this(global_this) + .options(aws_options.options) + .maybe_acl(aws_options.acl) + .maybe_storage_class(aws_options.storage_class) + .maybe_content_type(destination_blob.content_type_or_mime_type()) + // SAFETY: `*const [u8]` borrows from sibling + // `_*_slice` fields on `aws_options`, which + // outlives this call. + .maybe_content_disposition( + aws_options.content_disposition.as_deref(), + ) + .maybe_content_encoding(aws_options.content_encoding.as_deref()) + .maybe_proxy(proxy_url) + .request_payer(aws_options.request_payer) + .callback_context(core::ptr::null_mut()) + .call()?, + )); } - let task = - bun_core::heap::into_raw(Box::new(WriteFileWaitFromLockedValueTask { - global_this: bun_ptr::BackRef::new(global_this), - // Move `destination_blob` by value into the task. - file_blob: core::mem::replace( - &mut destination_blob, - Blob::init_empty(global_this), - ), - promise: jsc::JSPromiseStrong::init(global_this), - mkdirp_if_not_exists: options.mkdirp_if_not_exists.unwrap_or(true), - })); - // SAFETY: re-borrow after the early-return paths. - let BodyValue::Locked(locked) = (unsafe { &mut *body_value }) else { - unreachable!() - }; - locked.task = Some(task.cast::()); - locked.on_receive_value = Some(WriteFileWaitFromLockedValueTask::then_wrap); - // SAFETY: `task` was just heap-allocated; consumed in `then_wrap`. - Ok(ControlFlow::Break(unsafe { (*task).promise.value() })) + destination_blob.detach(); + return Err(global_this.throw_invalid_arguments(format_args!( + "ReadableStream has already been used" + ))); } + let task = + bun_core::heap::into_raw(Box::new(WriteFileWaitFromLockedValueTask { + global_this: bun_ptr::BackRef::new(global_this), + // Move `destination_blob` by value into the task. + file_blob: core::mem::replace( + &mut destination_blob, + Blob::init_empty(global_this), + ), + promise: jsc::JSPromiseStrong::init(global_this), + mkdirp_if_not_exists: options.mkdirp_if_not_exists.unwrap_or(true), + })); + // SAFETY: re-borrow after the early-return paths. + let BodyValue::Locked(locked) = (unsafe { &mut *body_value }) else { + unreachable!() + }; + locked.task = Some(task.cast::()); + locked.on_receive_value = Some(WriteFileWaitFromLockedValueTask::then_wrap); + // SAFETY: `task` was just heap-allocated; consumed in `then_wrap`. + Ok(ControlFlow::Break(unsafe { (*task).promise.value() })) } - }; + } + }; // `as_class_ref` is the safe shared-borrow downcast (one audited unsafe // in `JSValue`); `get_body_value` / `get_body_readable_stream` both diff --git a/src/runtime/webcore/fetch.rs b/src/runtime/webcore/fetch.rs index 7bda0212bc0f..8f7d138b9e46 100644 --- a/src/runtime/webcore/fetch.rs +++ b/src/runtime/webcore/fetch.rs @@ -107,15 +107,15 @@ fn ssl_config_intern_for_http(config: SSLConfig) -> http::ssl_config::SharedPtr pub(crate) fn s3_credentials_from_env( env: &bun_dotenv::S3Credentials, ) -> bun_s3_signing::S3Credentials { - bun_s3_signing::S3Credentials::new_value( - env.access_key_id.clone(), - env.secret_access_key.clone(), - env.region.clone(), - env.endpoint.clone(), - env.bucket.clone(), - env.session_token.clone(), - env.insecure_http, - ) + bun_s3_signing::S3Credentials::new_value() + .access_key_id(env.access_key_id.clone()) + .secret_access_key(env.secret_access_key.clone()) + .region(env.region.clone()) + .endpoint(env.endpoint.clone()) + .bucket(env.bucket.clone()) + .session_token(env.session_token.clone()) + .insecure_http(env.insecure_http) + .call() } /// RAII guard for the `+1` `AbortSignal` ref taken in `extract_signal`, @@ -1934,22 +1934,23 @@ fn fetch_impl( // `dupe()` heap-allocates a fresh intrusive-refcounted copy. // `upload_stream` adopts the ref by value (no extra bump) and the // MultiPartUpload derefs on completion. - let _ = s3::upload_stream( - credentials_with_options.credentials.dupe(), - s3_path, - readable_stream.get(global_this).unwrap(), - global_this, - credentials_with_options.options, - credentials_with_options.acl, - credentials_with_options.storage_class, - headers.as_ref().and_then(|h| h.get_content_type()), - headers.as_ref().and_then(|h| h.get_content_disposition()), - headers.as_ref().and_then(|h| h.get_content_encoding()), - proxy_url, - credentials_with_options.request_payer, - Some(s3_stream_wrapper_resolve), - bun_core::heap::into_raw(s3_stream).cast::(), - )?; + let _ = s3::upload_stream(credentials_with_options.credentials.dupe()) + .path(s3_path) + .readable_stream(readable_stream.get(global_this).unwrap()) + .global_this(global_this) + .options(credentials_with_options.options) + .maybe_acl(credentials_with_options.acl) + .maybe_storage_class(credentials_with_options.storage_class) + .maybe_content_type(headers.as_ref().and_then(|h| h.get_content_type())) + .maybe_content_disposition( + headers.as_ref().and_then(|h| h.get_content_disposition()), + ) + .maybe_content_encoding(headers.as_ref().and_then(|h| h.get_content_encoding())) + .maybe_proxy(proxy_url) + .request_payer(credentials_with_options.request_payer) + .callback(s3_stream_wrapper_resolve) + .callback_context(bun_core::heap::into_raw(s3_stream).cast::()) + .call()?; // url/url_proxy_buffer ownership moved into s3_stream above. return Ok(promise_value); } @@ -2053,39 +2054,41 @@ fn fetch_impl( } else { None }; - let fetch_options = FetchOptions { - method, - url: url_static, - headers: headers.take().unwrap_or_default(), - body, - disable_keepalive, - disable_timeout, - idle_timeout_seconds, - disable_decompression, - max_redirects, - reject_unauthorized, - redirect_type, - verbose, - proxy: proxy_static, - proxy_headers: proxy_headers.take(), - url_proxy_buffer: url_proxy_boxed, - signal: signal.take(), - global_this: Some(global_this.into()), - ssl_config: ssl_config.take(), - hostname: hostname.take(), - upgraded_connection, - force_http2, - force_http3, - force_http1, - is_node_http_client: ALLOW_GET_BODY, - compress, - check_server_identity: if check_server_identity.is_empty_or_undefined_or_null() { + let fetch_options = FetchOptions::builder() + .method(method) + .url(url_static) + .headers(headers.take().unwrap_or_default()) + .body(body) + .disable_keepalive(disable_keepalive) + .disable_timeout(disable_timeout) + .maybe_idle_timeout_seconds(idle_timeout_seconds) + .disable_decompression(disable_decompression) + .maybe_max_redirects(max_redirects) + .reject_unauthorized(reject_unauthorized) + .redirect_type(redirect_type) + .verbose(verbose) + .maybe_proxy(proxy_static) + .maybe_proxy_headers(proxy_headers.take()) + .url_proxy_buffer(url_proxy_boxed) + .maybe_signal(signal.take()) + .maybe_ssl_config(ssl_config.take()) + .maybe_hostname(hostname.take()) + .upgraded_connection(upgraded_connection) + .force_http2(force_http2) + .force_http3(force_http3) + .force_http1(force_http1) + .is_node_http_client(ALLOW_GET_BODY) + .maybe_compress(compress) + .check_server_identity(if check_server_identity.is_empty_or_undefined_or_null() { jsc::strong::Optional::empty() } else { jsc::strong::Optional::create(check_server_identity, global_this) - }, - unix_socket_path: core::mem::replace(&mut unix_socket_path, ZigStringSlice::empty()), - }; + }) + .unix_socket_path(core::mem::replace( + &mut unix_socket_path, + ZigStringSlice::empty(), + )) + .build(); let _ = FetchTasklet::queue( global_this, diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index c97257ce9993..94eeb63cfcff 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -1975,40 +1975,44 @@ impl FetchTasklet { // post-move query (`is_http()`, debug-assert only) up front. let url_is_http = url.is_http(); - fetch_tasklet.http = Some(Box::new(AsyncHTTP::init( - fetch_options.method, - url, - header_entries, - headers_buf, - response_buffer, - request_body_slice, - // handles response events (on headers, on body, etc.) - http::HTTPClientResultCallback::new_with_release::( - fetch_tasklet_ptr, - // SAFETY: `new_with_release` guarantees the pointer/lifetime - // contract `callback` documents. - FetchTasklet::callback, - FetchTasklet::release_at_shutdown, - ), - fetch_options.redirect_type, - http::async_http::Options { - http_proxy: proxy, - proxy_settings, - proxy_headers: fetch_options.proxy_headers, - hostname, - signals: Some(fetch_tasklet.signals), - unix_socket_path: Some(fetch_options.unix_socket_path), - disable_timeout: Some(fetch_options.disable_timeout), - idle_timeout_seconds: fetch_options.idle_timeout_seconds, - disable_keepalive: Some(fetch_options.disable_keepalive), - disable_decompression: Some(fetch_options.disable_decompression), - max_redirects: fetch_options.max_redirects, - reject_unauthorized: Some(fetch_options.reject_unauthorized), - verbose: Some(fetch_options.verbose), - tls_props: fetch_options.ssl_config, - compress: fetch_options.compress, - }, - ))); + fetch_tasklet.http = Some(Box::new( + AsyncHTTP::init() + .method(fetch_options.method) + .url(url) + .headers(header_entries) + .headers_buf(headers_buf) + .response_buffer(response_buffer) + .request_body(request_body_slice) + // handles response events (on headers, on body, etc.) + .callback(http::HTTPClientResultCallback::new_with_release::< + FetchTasklet, + >( + fetch_tasklet_ptr, + // SAFETY: `new_with_release` guarantees the pointer/lifetime + // contract `callback` documents. + FetchTasklet::callback, + FetchTasklet::release_at_shutdown, + )) + .redirect_type(fetch_options.redirect_type) + .options(http::async_http::Options { + http_proxy: proxy, + proxy_settings, + proxy_headers: fetch_options.proxy_headers, + hostname, + signals: Some(fetch_tasklet.signals), + unix_socket_path: Some(fetch_options.unix_socket_path), + disable_timeout: Some(fetch_options.disable_timeout), + idle_timeout_seconds: fetch_options.idle_timeout_seconds, + disable_keepalive: Some(fetch_options.disable_keepalive), + disable_decompression: Some(fetch_options.disable_decompression), + max_redirects: fetch_options.max_redirects, + reject_unauthorized: Some(fetch_options.reject_unauthorized), + verbose: Some(fetch_options.verbose), + tls_props: fetch_options.ssl_config, + compress: fetch_options.compress, + }) + .call(), + )); // enable streaming the write side let is_stream = matches!( fetch_tasklet.request_body, @@ -2516,6 +2520,10 @@ impl FetchTasklet { } } +/// Every non-`Option` field without a `#[builder(default)]` must be supplied +/// at the construction site; forgetting one (an empty `url`, a missing +/// `body`) is a compile error rather than an inert placeholder. +#[derive(bon::Builder)] pub struct FetchOptions { pub method: Method, pub headers: Headers, @@ -2528,15 +2536,18 @@ pub struct FetchOptions { pub max_redirects: Option, pub reject_unauthorized: bool, pub url: ZigURL<'static>, + #[builder(default = http::HTTPVerboseLevel::None)] pub verbose: http::HTTPVerboseLevel, + #[builder(default = FetchRedirect::Follow)] pub redirect_type: FetchRedirect, pub proxy: Option>, pub proxy_headers: Option, + #[builder(default)] pub url_proxy_buffer: Box<[u8]>, pub signal: Option<*mut AbortSignal>, - pub global_this: Option, - // Custom Hostname + /// Custom Hostname pub hostname: Option>, + #[builder(default = StrongOptional::empty())] pub check_server_identity: StrongOptional, pub unix_socket_path: ZigStringSlice, pub ssl_config: Option, @@ -2547,41 +2558,3 @@ pub struct FetchOptions { pub is_node_http_client: bool, pub compress: Option, } - -impl Default for FetchOptions { - fn default() -> Self { - // Zero-values for the required fields - // (method/headers/body/url/bools/unix_socket_path/globalThis) so - // callers can use `..Default::default()` struct-update syntax while - // still overriding the required fields explicitly. - Self { - method: Method::GET, - headers: Headers::default(), - body: HTTPRequestBody::default(), - disable_timeout: false, - idle_timeout_seconds: None, - disable_keepalive: false, - disable_decompression: false, - max_redirects: None, - reject_unauthorized: true, - url: ZigURL::default(), - verbose: http::HTTPVerboseLevel::None, - redirect_type: FetchRedirect::Follow, - proxy: None, - proxy_headers: None, - url_proxy_buffer: Box::default(), - signal: None, - global_this: None, - hostname: None, - check_server_identity: StrongOptional::empty(), - unix_socket_path: ZigStringSlice::EMPTY, - ssl_config: None, - upgraded_connection: false, - force_http2: false, - force_http3: false, - force_http1: false, - is_node_http_client: false, - compress: None, - } - } -} diff --git a/src/runtime/webcore/s3/client.rs b/src/runtime/webcore/s3/client.rs index 26bfaa497f63..56fd5f5d9af2 100644 --- a/src/runtime/webcore/s3/client.rs +++ b/src/runtime/webcore/s3/client.rs @@ -295,22 +295,23 @@ pub(crate) fn list_objects( let headers = bun_http::Headers::from_pico_http_headers(result.headers()); - let task_ptr = bun_core::heap::into_raw(Box::new(S3HttpSimpleTask { - // Written below via `MaybeUninit::write` before any read. - http: core::mem::MaybeUninit::uninit(), - range: None, - sign_result: result, - callback_context, - callback: s3_simple_request::Callback::ListObjects(callback), - headers, - vm: Some(bun_ptr::BackRef::new(VirtualMachine::get())), - response_buffer: MutableString::default(), - result: bun_http::HTTPClientResult::default(), - concurrent_task: Default::default(), - proxy_url: Box::default(), - body: Box::default(), - poll_ref: bun_io::KeepAlive::init(), - })); + let task_ptr = bun_core::heap::into_raw(Box::new( + S3HttpSimpleTask::builder() + // Written below via `MaybeUninit::write` before any read. + .http(core::mem::MaybeUninit::uninit()) + .sign_result(result) + .callback_context(callback_context) + .callback(s3_simple_request::Callback::ListObjects(callback)) + .headers(headers) + .vm(bun_ptr::BackRef::new(VirtualMachine::get())) + .response_buffer(MutableString::default()) + .result(bun_http::HTTPClientResult::default()) + .concurrent_task(Default::default()) + .proxy_url(Box::default()) + .body(Box::default()) + .poll_ref(bun_io::KeepAlive::init()) + .build(), + )); // SAFETY: just allocated, non-null let task = unsafe { &mut *task_ptr }; @@ -341,34 +342,36 @@ pub(crate) fn list_objects( } else { None }; - let mut vm_ref = task.vm.expect("vm set at task creation"); + let mut vm_ref = task.vm; // SAFETY: `task.vm` is the live per-thread VM BackRef from // `VirtualMachine::get()`; `get_mut` exclusivity holds — single-threaded // dispatch on the JS thread, no other `&`/`&mut VirtualMachine` is live for // this call's duration. let vm = unsafe { vm_ref.get_mut() }; - task.http.write(bun_http::AsyncHTTP::init( - bun_http::Method::GET, - url, - task.headers.entries.clone().expect("OOM"), - headers_buf, - &raw mut task.response_buffer, - b"", - bun_http::HTTPClientResultCallback::new::( - task_ptr, - // SAFETY: `task_ptr` is the heap-allocated task registered above; the - // HTTP thread invokes this with that exact pointer. - S3HttpSimpleTask::http_callback, - ), - bun_http::FetchRedirect::Follow, - bun_http::async_http::Options { - http_proxy, - verbose: Some(vm.get_verbose_fetch()), - reject_unauthorized: Some(vm.get_tls_reject_unauthorized()), - ..Default::default() - }, - )); + task.http.write( + bun_http::AsyncHTTP::init() + .method(bun_http::Method::GET) + .url(url) + .headers(task.headers.entries.clone().expect("OOM")) + .headers_buf(headers_buf) + .response_buffer(&raw mut task.response_buffer) + .request_body(b"") + .callback(bun_http::HTTPClientResultCallback::new::( + task_ptr, + // SAFETY: `task_ptr` is the heap-allocated task registered above; the + // HTTP thread invokes this with that exact pointer. + S3HttpSimpleTask::http_callback, + )) + .redirect_type(bun_http::FetchRedirect::Follow) + .options(bun_http::async_http::Options { + http_proxy, + verbose: Some(vm.get_verbose_fetch()), + reject_unauthorized: Some(vm.get_tls_reject_unauthorized()), + ..Default::default() + }) + .call(), + ); // queue http request bun_http::http_thread::init(&Default::default()); @@ -379,8 +382,11 @@ pub(crate) fn list_objects( Ok(()) } +/// Named setters: `path`/`content` are both `&[u8]` and four of the options +/// are `Option<&[u8]>`, so positional arguments could transpose them. +#[bon::builder] pub fn upload( - this: &S3Credentials, + #[builder(start_fn)] this: &S3Credentials, path: &[u8], content: &[u8], content_type: Option<&[u8]>, @@ -417,8 +423,11 @@ pub fn upload( /// /// Takes ownership of one `credentials` ref (adopted directly into the /// `MultiPartUpload`; not bumped). Callers pass `creds.dupe()`. +/// Named setters: four of the parameters are `Option<&[u8]>`, so positional +/// arguments could transpose any pair of them. +#[bon::builder] pub(crate) fn writable_stream( - credentials: bun_ptr::IntrusiveRc, + #[builder(start_fn)] credentials: bun_ptr::IntrusiveRc, path: &[u8], global_this: &JSGlobalObject, options: MultiPartUploadOptions, @@ -737,8 +746,12 @@ impl Drop for S3UploadStreamWrapper { /// Takes ownership of one `credentials` ref (adopted directly into the /// `MultiPartUpload`; not bumped). Callers pass `creds.dupe()`. On every /// early-return path the ref is explicitly released. +/// Named setters: `content_type`/`content_disposition`/`content_encoding`/ +/// `proxy` are four consecutive `Option<&[u8]>` parameters that positional +/// arguments could transpose. +#[bon::builder] pub fn upload_stream( - credentials: bun_ptr::IntrusiveRc, + #[builder(start_fn)] credentials: bun_ptr::IntrusiveRc, path: &[u8], readable_stream: ReadableStream, global_this: &JSGlobalObject, @@ -1004,35 +1017,37 @@ pub(crate) fn download_stream( Box::<[u8]>::default() }; let task_ptr = bun_core::heap::into_raw(S3HttpDownloadStreamingTask::new( - S3HttpDownloadStreamingTask { + S3HttpDownloadStreamingTask::builder() // `http: undefined` — fully overwritten by `task.http.write(AsyncHTTP::init(...))` below. - http: core::mem::MaybeUninit::uninit(), - sign_result: result, - proxy_url: owned_proxy, - callback_context: NonNull::new(callback_context.cast::<()>()) - .expect("callers always pass a non-null Box-allocated context"), - callback, - range: range.map(Vec::into_boxed_slice), - headers, + .http(core::mem::MaybeUninit::uninit()) + .sign_result(result) + .proxy_url(owned_proxy) + .callback_context( + NonNull::new(callback_context.cast::<()>()) + .expect("callers always pass a non-null Box-allocated context"), + ) + .callback(callback) + .maybe_range(range.map(Vec::into_boxed_slice)) + .headers(headers) // `VirtualMachine::get()` returns the live per-thread VM singleton. - vm: Some(bun_ptr::BackRef::new(VirtualMachine::get())), - has_schedule_callback: core::sync::atomic::AtomicBool::new(false), - signal_store: Default::default(), - signals: Default::default(), - poll_ref: bun_io::KeepAlive::init(), - response_buffer: MutableString::default(), - mutex: Default::default(), - reported_response_buffer: MutableString::default(), + .vm(bun_ptr::BackRef::new(VirtualMachine::get())) + .has_schedule_callback(core::sync::atomic::AtomicBool::new(false)) + .signal_store(Default::default()) + .signals(Default::default()) + .poll_ref(bun_io::KeepAlive::init()) + .response_buffer(MutableString::default()) + .mutex(Default::default()) + .reported_response_buffer(MutableString::default()) // `State::default()` sets // `has_more = true` (bit 48). Passing 0 here would start the task with // `has_more == false`, tripping the `assert(state.has_more)` in // `process_http_callback` on the very first HTTP-thread callback. - state: core::sync::atomic::AtomicU64::new( + .state(core::sync::atomic::AtomicU64::new( crate::webcore::s3::download_stream::State::default().0, - ), - concurrent_task: Default::default(), - async_http_id: 0, - }, + )) + .concurrent_task(Default::default()) + .async_http_id(0) + .build(), )); // SAFETY: just allocated via heap::alloc, non-null; lifetime owned by HTTP callback // (freed via heap::take in S3HttpDownloadStreamingTask::http_callback). @@ -1064,28 +1079,32 @@ pub(crate) fn download_stream( let verbose = vm_mut.get_verbose_fetch(); let reject_unauthorized = vm_mut.get_tls_reject_unauthorized(); - task.http.write(bun_http::AsyncHTTP::init( - bun_http::Method::GET, - url, - task.headers.entries.clone().expect("OOM"), - headers_buf, - &raw mut task.response_buffer, - b"", - bun_http::HTTPClientResultCallback::new::( - task_ptr, - // SAFETY: `task_ptr` is the heap-allocated task registered above; the - // HTTP thread invokes this with that exact pointer. - S3HttpDownloadStreamingTask::http_callback, - ), - bun_http::FetchRedirect::Follow, - bun_http::async_http::Options { - http_proxy, - verbose: Some(verbose), - signals: Some(task.signals), - reject_unauthorized: Some(reject_unauthorized), - ..Default::default() - }, - )); + task.http.write( + bun_http::AsyncHTTP::init() + .method(bun_http::Method::GET) + .url(url) + .headers(task.headers.entries.clone().expect("OOM")) + .headers_buf(headers_buf) + .response_buffer(&raw mut task.response_buffer) + .request_body(b"") + .callback(bun_http::HTTPClientResultCallback::new::< + S3HttpDownloadStreamingTask, + >( + task_ptr, + // SAFETY: `task_ptr` is the heap-allocated task registered above; the + // HTTP thread invokes this with that exact pointer. + S3HttpDownloadStreamingTask::http_callback, + )) + .redirect_type(bun_http::FetchRedirect::Follow) + .options(bun_http::async_http::Options { + http_proxy, + verbose: Some(verbose), + signals: Some(task.signals), + reject_unauthorized: Some(reject_unauthorized), + ..Default::default() + }) + .call(), + ); // SAFETY: `http` was initialised by `task.http.write(...)` immediately above. let http = unsafe { task.http.assume_init_mut() }; task.async_http_id = http.async_http_id; diff --git a/src/runtime/webcore/s3/download_stream.rs b/src/runtime/webcore/s3/download_stream.rs index 7b7f2a0a0c77..5895b0fd3503 100644 --- a/src/runtime/webcore/s3/download_stream.rs +++ b/src/runtime/webcore/s3/download_stream.rs @@ -15,13 +15,16 @@ use bun_threading::Mutex; bun_core::declare_scope!(S3, hidden); +/// Every non-`Option` field must be supplied at the construction site; +/// forgetting one is a compile error. In particular `callback` and +/// `callback_context` have no safe placeholder. +#[derive(bon::Builder)] pub struct S3HttpDownloadStreamingTask { // `MaybeUninit` because `AsyncHTTP` contains non-null references, so // `mem::zeroed()` can't be used here (mirrors `S3HttpSimpleTask`). pub http: core::mem::MaybeUninit>, - /// JSC_BORROW: per-thread VM singleton, outlives every task. `None` only in - /// the inert `Default` placeholder (overwritten before the task escapes). - pub vm: Option>, + /// JSC_BORROW: per-thread VM singleton, outlives every task. + pub vm: bun_ptr::BackRef, pub sign_result: SignResult, pub headers: Headers, pub callback_context: NonNull<()>, @@ -50,36 +53,6 @@ impl Taskable for S3HttpDownloadStreamingTask { const TAG: TaskTag = task_tag::S3HttpDownloadStreamingTask; } -impl Default for S3HttpDownloadStreamingTask { - fn default() -> Self { - // only the fields `has_schedule_callback` .. `concurrent_task` - // are observed via this path; the rest are placeholders that the caller (client.rs - // `..Default::default()`) overwrites before the task pointer escapes - // (see S3HttpSimpleTask in simple_request.rs). - Self { - // never read — fully overwritten by `AsyncHTTP::init` before first use. - http: core::mem::MaybeUninit::uninit(), - vm: None, - sign_result: SignResult::default(), - headers: Headers::default(), - callback_context: NonNull::dangling(), - callback: |_, _, _, _| {}, - range: None, - proxy_url: Box::default(), - has_schedule_callback: AtomicBool::new(false), - signal_store: bun_http::signals::Store::default(), - signals: Signals::default(), - poll_ref: KeepAlive::default(), - response_buffer: MutableString::default(), - mutex: Mutex::default(), - reported_response_buffer: MutableString::default(), - state: AtomicU64::new(State::default().0), - concurrent_task: ConcurrentTask::default(), - async_http_id: 0, - } - } -} - impl S3HttpDownloadStreamingTask { pub fn new(init: Self) -> Box { Box::new(init) @@ -350,11 +323,7 @@ impl S3HttpDownloadStreamingTask { // is initialized for the request's lifetime and enqueue is thread-safe (`&self`). // `task` is the inline `concurrent_task` field of this heap request; // the queue takes ownership of its `next` link. - self_ - .vm - .expect("vm set at task creation") - .event_loop_shared() - .enqueue_task_concurrent(task); + self_.vm.event_loop_shared().enqueue_task_concurrent(task); } } } diff --git a/src/runtime/webcore/s3/simple_request.rs b/src/runtime/webcore/s3/simple_request.rs index 986715dbe109..b838babd2828 100644 --- a/src/runtime/webcore/s3/simple_request.rs +++ b/src/runtime/webcore/s3/simple_request.rs @@ -107,6 +107,10 @@ pub enum S3PartResult<'a> { Failure(S3Error<'a>), } +/// Every non-`Option` field must be supplied at the construction site; +/// forgetting one is a compile error. `Drop` calls `assume_init` on `http`, +/// so a half-built value is never allowed to exist. +#[derive(bon::Builder)] pub struct S3HttpSimpleTask { // `http` is `MaybeUninit` because (a) it is initialised late — // `AsyncHTTP` contains `&'static [u8]` and `fn(...)` fields, so a @@ -117,9 +121,8 @@ pub struct S3HttpSimpleTask { // `execute_simple_s3_request` before the task pointer escapes, so every later access (in // `http_callback` / `Drop`) may `assume_init`. pub http: core::mem::MaybeUninit>, - /// JSC_BORROW: per-thread VM singleton, outlives every task. `None` only in - /// the inert `Default` placeholder (overwritten before the task escapes). - pub vm: Option>, + /// JSC_BORROW: per-thread VM singleton, outlives every task. + pub vm: bun_ptr::BackRef, pub sign_result: SignResult, pub headers: Headers, pub callback_context: *mut c_void, @@ -145,32 +148,6 @@ impl Taskable for S3HttpSimpleTask { const TAG: TaskTag = task_tag::S3HttpSimpleTask; } -// `..Default::default()` requires the whole struct to be Default, so beyond -// `response_buffer`/`result`/`concurrent_task` the remaining fields get -// inert placeholders that callers always overwrite (see client.rs / execute_simple_s3_request). -impl Default for S3HttpSimpleTask { - fn default() -> Self { - fn unset_callback(_: S3UploadResult<'_>, _: *mut c_void) -> JsTerminatedResult<()> { - unreachable!("S3HttpSimpleTask.callback used before being set") - } - Self { - http: core::mem::MaybeUninit::uninit(), - vm: None, - sign_result: SignResult::default(), - headers: Headers::default(), - callback_context: core::ptr::null_mut(), - callback: Callback::Upload(unset_callback), - response_buffer: MutableString::default(), - result: HTTPClientResult::default(), - concurrent_task: ConcurrentTask::default(), - range: None, - proxy_url: Box::default(), - body: Box::default(), - poll_ref: KeepAlive::default(), - } - } -} - // Re-export the canonical alias so sibling modules that imported it from here keep compiling. pub use bun_jsc::JsTerminatedResult; @@ -484,10 +461,7 @@ impl S3HttpSimpleTask { // is set during VM init and outlives this task. `enqueue_task_concurrent` is `&self`. // `task` is the inline `concurrent_task` field of this heap request; // the queue takes ownership of its `next` link. - this.vm - .expect("vm set at task creation") - .event_loop_shared() - .enqueue_task_concurrent(task); + this.vm.event_loop_shared().enqueue_task_concurrent(task); } } } @@ -619,22 +593,24 @@ pub(crate) fn execute_simple_s3_request( } }; - let task_ptr = S3HttpSimpleTask::new(S3HttpSimpleTask { - // written below via `MaybeUninit::write` before any read. - http: core::mem::MaybeUninit::uninit(), - sign_result: result, - callback_context, - callback, - range: options.range, - headers, - vm: Some(bun_ptr::BackRef::new(VirtualMachine::get())), - response_buffer: MutableString::default(), - result: HTTPClientResult::default(), - concurrent_task: ConcurrentTask::default(), - proxy_url: Box::default(), - body: Box::<[u8]>::from(options.body), - poll_ref: KeepAlive::init(), - }); + let task_ptr = S3HttpSimpleTask::new( + S3HttpSimpleTask::builder() + // written below via `MaybeUninit::write` before any read. + .http(core::mem::MaybeUninit::uninit()) + .sign_result(result) + .callback_context(callback_context) + .callback(callback) + .maybe_range(options.range) + .headers(headers) + .vm(bun_ptr::BackRef::new(VirtualMachine::get())) + .response_buffer(MutableString::default()) + .result(HTTPClientResult::default()) + .concurrent_task(ConcurrentTask::default()) + .proxy_url(Box::default()) + .body(Box::<[u8]>::from(options.body)) + .poll_ref(KeepAlive::init()) + .build(), + ); // SAFETY: `task_ptr` is a freshly heap-allocated pointer; exclusive access here. let task = unsafe { &mut *task_ptr }; task.poll_ref.ref_(bun_io::posix_event_loop::get_vm_ctx( @@ -672,27 +648,29 @@ pub(crate) fn execute_simple_s3_request( let vm = VirtualMachine::get(); let verbose = vm.as_mut().get_verbose_fetch(); let reject_unauthorized = vm.get_tls_reject_unauthorized(); - task.http.write(AsyncHTTP::init( - options.method, - url, - task.headers.entries.clone().expect("OOM"), - headers_buf, - &raw mut task.response_buffer, - body, - HTTPClientResultCallback::new::( - task_ptr, - // SAFETY: `task_ptr` was just heap-allocated above and `async_http` is supplied by - // the HTTP thread as a live pointer for the duration of the callback. - S3HttpSimpleTask::http_callback, - ), - FetchRedirect::Follow, - HttpOptions { - http_proxy, - verbose: Some(verbose), - reject_unauthorized: Some(reject_unauthorized), - ..Default::default() - }, - )); + task.http.write( + AsyncHTTP::init() + .method(options.method) + .url(url) + .headers(task.headers.entries.clone().expect("OOM")) + .headers_buf(headers_buf) + .response_buffer(&raw mut task.response_buffer) + .request_body(body) + .callback(HTTPClientResultCallback::new::( + task_ptr, + // SAFETY: `task_ptr` was just heap-allocated above and `async_http` is supplied by + // the HTTP thread as a live pointer for the duration of the callback. + S3HttpSimpleTask::http_callback, + )) + .redirect_type(FetchRedirect::Follow) + .options(HttpOptions { + http_proxy, + verbose: Some(verbose), + reject_unauthorized: Some(reject_unauthorized), + ..Default::default() + }) + .call(), + ); // queue http request bun_http::http_thread::init(&Default::default()); let mut batch = thread_pool::Batch::default(); diff --git a/src/s3_signing/Cargo.toml b/src/s3_signing/Cargo.toml index 17618528c418..f2f1f4d41115 100644 --- a/src/s3_signing/Cargo.toml +++ b/src/s3_signing/Cargo.toml @@ -10,6 +10,7 @@ path = "lib.rs" workspace = true [dependencies] +bon.workspace = true strum.workspace = true bstr.workspace = true scopeguard.workspace = true diff --git a/src/s3_signing/credentials.rs b/src/s3_signing/credentials.rs index ec6840fe8a8f..fe09c5aaca70 100644 --- a/src/s3_signing/credentials.rs +++ b/src/s3_signing/credentials.rs @@ -221,12 +221,18 @@ impl Default for S3Credentials { } } +// Separate impl block so `#[bon::bon]` only re-emits `new_value`, not the +// rest of the (large) `S3Credentials` impl below. +#[bon::bon] impl S3Credentials { /// Construct a value (refcount = 1) from owned field data. Exists so /// higher-tier callers (e.g. `bun_runtime`) can build the refcounted /// signing credentials from the lower-tier `bun_dotenv::S3Credentials` /// POD mirror without naming the private `ref_count` field. - #[allow(clippy::too_many_arguments)] + /// + /// Named setters: six of these parameters are `Box<[u8]>`, so positional + /// arguments could transpose `access_key_id` and `secret_access_key`. + #[builder] pub fn new_value( access_key_id: Box<[u8]>, secret_access_key: Box<[u8]>, @@ -249,7 +255,9 @@ impl S3Credentials { virtual_hosted_style: false, } } +} +impl S3Credentials { pub fn estimated_size(&self) -> usize { size_of::() + self.access_key_id.len() @@ -754,24 +762,23 @@ impl S3Credentials { ) .into_boxed_slice(); } else { - let canonical = CanonicalRequest::format( - &mut tmp_buffer, - header_key, - method_name.as_bytes(), - normalized_path, - search_params.map(|p| &p[1..]).unwrap_or(b""), - content_disposition, - content_encoding, - content_md5.as_deref(), - &host, - acl, - aws_content_hash, - &amz_date, - session_token, - storage_class, - signed_headers, - ) - .map_err(|_| SignError::NoSpaceLeft)?; + let canonical = CanonicalRequest::format(&mut tmp_buffer) + .key(header_key) + .method(method_name.as_bytes()) + .path(normalized_path) + .query(search_params.map(|p| &p[1..]).unwrap_or(b"")) + .maybe_content_disposition(content_disposition) + .maybe_content_encoding(content_encoding) + .maybe_content_md5(content_md5.as_deref()) + .host(&host) + .maybe_acl(acl) + .hash(aws_content_hash) + .date(&amz_date) + .maybe_session_token(session_token) + .maybe_storage_class(storage_class) + .signed_headers(signed_headers) + .call() + .map_err(|_| SignError::NoSpaceLeft)?; let mut sha_digest = [0u8; bun_sha_hmac::SHA256::DIGEST]; // was `bun_jsc::VirtualMachine::get().rare_data().boring_engine()`; // BoringSSL ignores the ENGINE arg, so pass null (see `boring_engine()` doc). @@ -1363,10 +1370,14 @@ impl SignedHeaders { struct CanonicalRequest; +#[bon::bon] impl CanonicalRequest { + /// Named setters: seven of these parameters are `&[u8]` and six are + /// `Option<&[u8]>`; transposing any pair silently corrupts the signature. // Builds the canonical request at runtime with conditional writes; profile if hot. + #[builder] pub(crate) fn format<'b>( - buf: &'b mut [u8], + #[builder(start_fn)] buf: &'b mut [u8], key: SignedHeadersKey, method: &[u8], path: &[u8], diff --git a/src/standalone_graph/StandaloneModuleGraph.rs b/src/standalone_graph/StandaloneModuleGraph.rs index 0245ccc1944c..3150d99207e2 100644 --- a/src/standalone_graph/StandaloneModuleGraph.rs +++ b/src/standalone_graph/StandaloneModuleGraph.rs @@ -1554,17 +1554,18 @@ pub(crate) fn download_to_path( let http_proxy: Option> = env.get_http_proxy_for(&url); let progress = refresher.start(b"Downloading", 0); - let mut async_http = Box::new(bun_http::AsyncHTTP::init_sync( - bun_http::Method::GET, - url, - Default::default(), - b"", - &raw mut *compressed_archive_bytes, - b"", - http_proxy, - None, - bun_http::FetchRedirect::Follow, - )); + let mut async_http = Box::new( + bun_http::AsyncHTTP::init_sync() + .method(bun_http::Method::GET) + .url(url) + .headers(Default::default()) + .headers_buf(b"") + .response_buffer(&raw mut *compressed_archive_bytes) + .request_body(b"") + .maybe_http_proxy(http_proxy) + .redirect_type(bun_http::FetchRedirect::Follow) + .call(), + ); async_http.client.progress_node = core::ptr::NonNull::new(core::ptr::from_mut(progress)); async_http.client.flags.reject_unauthorized = reject_unauthorized; diff --git a/test/js/bun/glob/scan.test.ts b/test/js/bun/glob/scan.test.ts index 01a4b6ec6fa4..be4b890f2759 100644 --- a/test/js/bun/glob/scan.test.ts +++ b/test/js/bun/glob/scan.test.ts @@ -1125,3 +1125,108 @@ describe.skipIf(!canCreateDirSymlink)("literal path segment through a symlinked expect(norm(result)).toEqual(["linkdir/file.txt"]); }); }); + +// Each boolean scan option maps to one named setter on the Rust +// `GlobWalker::init` builder. Flipping one at a time against a fixed +// result set catches any option wired to the wrong flag. +describe("glob.scan option flags", () => { + const FILES = { + "top.txt": "top", + ".hidden.txt": "hidden", + "sub/inner.txt": "inner", + }; + const DEFAULT = ["sub/inner.txt", "top.txt"]; + + test.concurrent("defaults: relative paths, no dotfiles, files only", () => { + using dir = tempDir("glob-flag-default", FILES); + expect(prepareEntries([...new Glob("**/*.txt").scanSync(String(dir))])).toEqual(DEFAULT); + }); + + test.concurrent("dot controls whether dotfiles match", () => { + using dir = tempDir("glob-flag-dot", FILES); + const cwd = String(dir); + expect({ + on: prepareEntries([...new Glob("**/*.txt").scanSync({ cwd, dot: true })]), + off: prepareEntries([...new Glob("**/*.txt").scanSync({ cwd, dot: false })]), + }).toEqual({ + on: [".hidden.txt", ...DEFAULT], + off: DEFAULT, + }); + }); + + test.concurrent("absolute controls whether returned paths are rooted at cwd", () => { + using dir = tempDir("glob-flag-absolute", FILES); + const cwd = String(dir); + expect({ + on: prepareEntries([...new Glob("**/*.txt").scanSync({ cwd, absolute: true })]), + off: prepareEntries([...new Glob("**/*.txt").scanSync({ cwd, absolute: false })]), + }).toEqual({ + on: prepareEntries(DEFAULT.map(p => path.join(cwd, p))), + off: DEFAULT, + }); + }); + + test.concurrent("onlyFiles controls whether directories are returned", () => { + using dir = tempDir("glob-flag-onlyfiles", FILES); + const cwd = String(dir); + expect({ + on: prepareEntries([...new Glob("*").scanSync({ cwd, onlyFiles: true })]), + off: prepareEntries([...new Glob("*").scanSync({ cwd, onlyFiles: false })]), + }).toEqual({ + on: ["top.txt"], + off: ["sub", "top.txt"], + }); + }); + + test.concurrent.skipIf(!canCreateDirSymlink)( + "followSymlinks controls whether a symlinked directory is traversed", + () => { + // `outside/` is a sibling of the scanned root, so the only way + // `**/*.txt` reaches `linked.txt` is through the `link` symlink. + using dir = tempDir("glob-flag-symlinks", { + "root/top.txt": "top", + "root/.hidden.txt": "hidden", + "root/sub/inner.txt": "inner", + "outside/linked.txt": "linked", + }); + const cwd = path.join(String(dir), "root"); + fs.symlinkSync(path.join("..", "outside"), path.join(cwd, "link"), "dir"); + expect({ + on: prepareEntries([...new Glob("**/*.txt").scanSync({ cwd, followSymlinks: true })]), + off: prepareEntries([...new Glob("**/*.txt").scanSync({ cwd, followSymlinks: false })]), + }).toEqual({ + on: ["link/linked.txt", ...DEFAULT], + off: DEFAULT, + }); + }, + ); + + // An omitted `cwd` must resolve to the process cwd, the same directory an + // explicit `cwd: process.cwd()` names. Run in a subprocess so the implicit + // case does not depend on this file's `process.chdir` in `beforeAll`. + test.concurrent("omitting cwd scans from the process cwd", async () => { + using dir = tempDir("glob-flag-nocwd", FILES); + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const implicit = [...new Bun.Glob("**/*.txt").scanSync()]; + const explicit = [...new Bun.Glob("**/*.txt").scanSync({ cwd: process.cwd() })]; + console.log(JSON.stringify({ implicit, explicit }));`, + ], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // Assert the exit before parsing stdout, so a subprocess crash surfaces + // stdout/stderr/exitCode instead of a bare SyntaxError from JSON.parse. + // `stderr` is not pinned: ASAN/debug builds emit benign warnings there. + expect({ stdout, stderr, exitCode }).toMatchObject({ exitCode: 0 }); + const { implicit, explicit } = JSON.parse(stdout); + expect({ implicit: prepareEntries(implicit), explicit: prepareEntries(explicit) }).toEqual({ + implicit: DEFAULT, + explicit: DEFAULT, + }); + }); +});