Skip to content

deps: update lshpack to v2.3.5 #31315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
github-actions wants to merge 2 commits into
base: main
Choose a base branch
from
+2 −2
Open

deps: update lshpack to v2.3.5 #31315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a39bf77
deps: update lshpack to v2.3.5 (cf0f70dd10b352194c97448eb5d00b4aa484f…
Jarred-Sumner Aug 9, 2026
d8e7f43
test: update expected lshpack commit hash for v2.3.5
robobun Aug 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/build/deps/lshpack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import type { Dependency, DirectBuild } from "../source.ts";

const LSHPACK_COMMIT = "8905c024b6d052f083a3d11d0a169b3c2735c8a1";
const LSHPACK_COMMIT = "cf0f70dd10b352194c97448eb5d00b4aa484f531";
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.

@claude claude Bot Jul 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The earlier (resolved) comment on v2.3.5's removed trailing-whitespace strip is now stale for the two paths it named — both have since gained full tchar validation — but a fourth decode loop was missed: handle_push_promise_frame (src/runtime/api/bun/h2_frame_parser.rs:3678-3712) still surfaces header.name/header.value straight to JS with no is_malformed_field_name/is_malformed_field_value check between self.decode() at :3679 and create_utf8_for_js at :3697. After this bump a PUSH_PROMISE literal name like "cookie \t\r\n" or " " (previously trimmed/rejected by lshpack itself) reaches the node:http2 client's 'push' event verbatim. Client-only and requires a non-compliant server sending PUSH_PROMISE, so not blocking — but it's the one residual gap in the class fix; mirroring decode_header_block's guard (h2_frame_parser.rs:3828-3836) after :3692 would close it.

Extended reasoning...

What changed and why the earlier comment is stale

The resolved 2026-05-24 inline comment on this PR flagged that lshpack v2.3.5 (cf0f70dd) deletes from lshpack_dec_decode() the block while(len > 0 && isspace(*(name + len - 1))) --len; if (len == 0) return LSHPACK_ERR_BAD_DATA;, so literal header names with trailing whitespace now decode verbatim instead of being trimmed/rejected. Upstream's own new test test_hdec_trailing_whitespace_name asserts a literal name of cookie \t\v\f\r\n is returned with name_len == 12 and rc == 0 — validation is now the caller's job.

That comment named two Bun call sites (dispatch.zig:546, h2_frame_parser.zig:1918-1965). Both are now covered by full RFC 9110 tchar validation — the class fix landed:

  • src/http/h2_client/dispatch.rs:621 → is_malformed_response_field (dispatch.rs:737-762) rejects any byte outside the lowercase-tchar set
  • src/runtime/api/bun/h2_frame_parser.rs:3829 — decode_header_block calls is_malformed_field_name(header.name) || is_malformed_field_value(header.value) (defined at :627-656)
  • src/runtime/api/bun/h2/connection.rs:910 calls the same helper

Grep confirms is_malformed_field_name is called at exactly those sites — and not in handle_push_promise_frame.

The residual gap

handle_push_promise_frame at h2_frame_parser.rs:3678-3712 is the fourth HPACK decode loop, and it was missed by the class fix:

while off < payload.len() {
    let header = match self.decode(&payload[off..]) {   // :3679
        Ok(h) => h,
        Err(_) => { /* GOAWAY COMPRESSION_ERROR */ return Ok(end); }
    };
    off += header.next;                                  // :3692
    let js_name = match get_http2_common_string(...) {
        Some(cached) => cached,
        None => create_utf8_for_js(&global_object, header.name)?,   // :3697
    };
    headers.push(&global_object, js_name)?;
    headers.push(&global_object, create_utf8_for_js(&global_object, header.value)?)?;  // :3703
    ...
}

Between self.decode() and create_utf8_for_js there is no call to is_malformed_field_name / is_malformed_field_value, no pseudo-header check, no connection-specific-field check, and no maxHeaderListSize/maxHeaderListPairs accounting. Everything decoded is pushed straight into the JS headers array and dispatched via onStreamPush. Contrast with the sibling loop at :3828-3836, which gates on is_malformed_field_name(header.name) || is_malformed_field_value(header.value) || (header.name.first() == Some(&b':') && !is_valid_..._pseudo_header(...)) before surfacing anything.

The only remaining upstream guard is lshpack.rs:67's name_len == 0 → EmptyHeaderName, which catches truly-empty names but not whitespace-bearing ones (name_len > 0 after v2.3.5).

Interaction with this PR (step-by-step)

  1. A server sends a PUSH_PROMISE frame whose HPACK block contains a literal-header-field with a non-Huffman name string of 63 6f 6f 6b 69 65 20 09 0d 0a ("cookie \t\r\n", 10 bytes).
  2. Before this bump (8905c024): lshpack_dec_decode decodes len = 10, the trailing-whitespace loop strips the last four bytes, len = 6, name is "cookie". get_http2_common_string matches the well-known index, and JS sees the canonical key.
  3. After this bump (cf0f70dd): the loop is gone, len stays 10, header.name = b"cookie \t\r\n". well_know is 0 (no static-table hit for a 10-byte name), so control falls through to create_utf8_for_js(&global_object, header.name) at :3697 and JS receives a header keyed "cookie \t\r\n".
  4. All-whitespace case: a literal name of 20 20 20 (" "). Before, lshpack stripped to len = 0 → return LSHPACK_ERR_BAD_DATA → Err(_) at :3681 → COMPRESSION_ERROR GOAWAY. After, name_len = 3 → passes lshpack.rs:67 → surfaces to JS as a header keyed " ".

So the LSHPACK_COMMIT bump at line 13 observably widens what a node:http2 client's 'push' event sees, on this one path — it is not purely pre-existing.

Why nothing else catches it

is_malformed_field_name (:627-656) is a lowercase-tchar allowlist and would reject space/tab/CR/LF — it just isn't wired into this loop. The path is client-only (is_server sends GOAWAY at :3648-3656), and the frame-type dispatch calls handle_push_promise_frame unconditionally for type 0x05 with no enable_push gate, so it's reachable from any server the client connects to.

Impact

Low. Requires a deliberately non-compliant server sending PUSH_PROMISE (a largely-deprecated feature) with a crafted literal header name. Worst outcome is an odd header key surfaced to JS — no crash, no memory issue, no security boundary crossed. The missing maxHeaderListSize/maxHeaderListPairs accounting on this path is fully pre-existing and independent of this bump. Not worth blocking a routine dep bump; flagged so the one-line guard can be added to complete the class fix.

Suggested fix

Mirror decode_header_block's guard — after h2_frame_parser.rs:3692:

if is_malformed_field_name(header.name) || is_malformed_field_value(header.value) {
    // set a malformed flag and continue decoding (to keep HPACK dynamic-table state in sync),
    // then RST/GOAWAY PROTOCOL_ERROR after the loop instead of dispatching onStreamPush
}

Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.

export const lshpack: Dependency = {
name: "lshpack",
Expand Down
2 changes: 1 addition & 1 deletion test/js/node/process/process.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ it("process.versions", () => {
ares: "3ac47ee46edd8ea40370222f91613fc16c434853",
libdeflate: "c8c56a20f8f621e6a966b716b31f1dedab6a41e3",
zstd: "f8745da6ff1ad1e7bab384bd1f9d742439278e99",
lshpack: "8905c024b6d052f083a3d11d0a169b3c2735c8a1",
lshpack: "cf0f70dd10b352194c97448eb5d00b4aa484f531",
};

for (const [name, expectedHash] of Object.entries(expectedVersions)) {
Expand Down