fix: prevent partial response-header match TypeError (#527)#3772
Open
manwithacat wants to merge 1 commit intobigskysoftware:devfrom
Open
fix: prevent partial response-header match TypeError (#527)#3772manwithacat wants to merge 1 commit intobigskysoftware:devfrom
manwithacat wants to merge 1 commit intobigskysoftware:devfrom
Conversation
) hasHeader() tested a regex against getAllResponseHeaders() output, which false-positively matched any header containing an htmx header name as a substring — e.g. X-HX-Trigger or HX-Trigger-User. After the false-positive, xhr.getResponseHeader('HX-Trigger') returned null, and handleTriggerHeader crashed on triggerBody.indexOf('{'). Refactors hasHeader(xhr, regex) → hasHeader(xhr, name), using getResponseHeader(name) !== null as the check. This is the exact check the 12 call sites need, eliminates the substring-match class of bug, and removes a layer of indirection (no more regex construction or getAllResponseHeaders() scan per check). Adds a regression test with X-HX-Trigger: foo that previously crashed inside handleTriggerHeader. Fixes bigskysoftware#527. Addresses @1cg's suggestion in that thread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #527 — a four-year-old bug that @1cg (thread comment) explicitly invited a fix for.
The bug
hasHeader(xhr, regex)ranregex.test(xhr.getAllResponseHeaders())— testing against the concatenated header string. When a response carried a header whose name contained an htmx header name as a substring — the issue reporter's example wasX-HX-Trigger: foo— the regex/HX-Trigger:/imatched the substring and returnedtrue, butxhr.getResponseHeader('HX-Trigger')returnednull(because the actual header name isX-HX-Trigger).handleTriggerHeaderthen crashed atsrc/htmx.js:2068ontriggerBody.indexOf('{').The fix
Refactor
hasHeaderto usegetResponseHeader(name) !== nulldirectly:Every one of the 12 call sites already knew the exact header name (they used it in both the regex and the paired
getResponseHeadercall), so the update is mechanical:The substring-match class of bug is eliminated —
hasHeaderandgetResponseHeadernow use identical header lookup semantics.Tests
New regression test in
test/core/headers.js:does not crash or fire trigger when X-HX-Trigger header is present without HX-Trigger— sendsX-HX-Trigger: foo, asserts thefoolistener is not fired. Before the fix this crashed withTypeError: Cannot read properties of null (reading 'indexOf').Full suite: 847 passed, 0 failed, 3 skipped, 99.9% coverage. ESLint clean.
A note on @1cg's original concern
The thread mentions Chrome "issuing pretty gnarly looking error messages when you ask for a header that doesn't exist." That was the original reason for the regex approach. Modern Chrome/Firefox/Safari all silently return
nullfromgetResponseHeader()for missing headers with no console noise, so that justification no longer applies.