refactor(github): carry the bearer shape's case-insensitivity as a compile flag - #754
Conversation
…mpile flag Splitting the value shapes into one pattern each left `(?i:...)` wrapping the whole of the bearer pattern, which is a group that does nothing: with no second alternative to scope it away from, the flag applies to the pattern either way. Moving it to `Pattern.CASE_INSENSITIVE` keeps the behaviour exactly — without `UNICODE_CASE` the flag is ASCII-only, which is what the inline group was — and the javadoc now says so, so the ASCII scope is not read as an oversight and widened later. Also adjusts the wording for readability: the javadoc still opened by naming itself the bearer and JWT shapes, which stopped being true when the two became separate fields.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
🤖 ThrillhouseBot PR SummaryWhat this PR doesRefactors BEARER_SHAPED_VALUE in GitHubApiError so case-insensitive matching is applied via Pattern.compile(..., Pattern.CASE_INSENSITIVE) instead of an inline (?i:...) group, and rewrites the surrounding javadoc to document the bearer/JWT shape split, the ASCII-only case-folding scope, and why the JWT shape stays case-sensitive. No behavioral change is intended or produced. Description vs. ImplementationNo mismatch found between the PR description and the change. Changes Overview
Changed Files
Risk Assessment
No new issues found in this PR, but the review cannot be approved until required CI is confirmed green.
|
| Check | Type | Status | Detail |
|---|---|---|---|
| test | check-run | ⏳ Pending | - |
| trivy | check-run | ⏳ Pending | - |
| frontend | check-run | ⏳ Pending | - |
| format | check-run | ⏳ Pending | - |
| dependency-review | check-run | ⏳ Pending | - |
Automated review by ThrillhouseBot. Reply with /review to re-run.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
1 participant
What type of PR is this?
Description
#750 split
CREDENTIAL_SHAPED_VALUEintoBEARER_SHAPED_VALUEandJWT_SHAPED_VALUEto get the combined pattern's complexity back under budget. That split left one loose end, which the analysis on that PR flagged and which merged with it:The rule is right.
(?i:bearer\s+[\w.~+/=-]{4,})existed to scope case-insensitivity to the bearer alternative of a two-alternative pattern — that scoping was #746's fix, and it is whyeyjafjallajokull.internal.example.comstopped coming out as***.com. Once the alternation was gone there was nothing left to scope the flag away from, so the group wraps the entire pattern and does nothing.The flag moves to the compile call:
Behaviour is identical, and the identity matters.
Pattern.CASE_INSENSITIVEwithoutUNICODE_CASEmatches ASCII case only — exactly what(?i:...)did. That is the correct scope here: the shape is matching the literal HTTPBearerauth-scheme token, not prose, so Unicode case folding would only widen what a log line masks. The javadoc now records it, so the ASCII scope reads as the decision it is rather than as something to "fix" by addingUNICODE_CASE.JWT_SHAPED_VALUEstays case-sensitive, unchanged.Related Issues
Follow-up to #750; no separate issue filed, since the finding arrived through the analysis on that PR and is a two-line correction to it.
How Has This Been Tested?
No new test, and deliberately so: this is a refactor with no behavioural delta, so there is no red state to demonstrate. A test written for it would pass before the change as well, which proves nothing.
The existing coverage is what pins the equivalence, and two tests in
GitHubApiErrorTestbear directly on it:masksABearerHeaderWhateverCaseItArrivedIn— the case-insensitivity survives the move off the inline groupdoesNotMaskOrdinaryTextThatMerelyBeginsLikeAJwtHeaderInSomeOtherCase— the JWT shape stays case-sensitive, i.e. the flag did not leak across the splitBoth are controls: green before and after.
Gates
./mvnw -B spotless:apply→ clean./mvnw -B clean compile spotbugs:check spotless:check→ BugInstance size is 0, BUILD SUCCESS./mvnw -B clean test→ Tests run: 3324, Failures: 0, Errors: 0, Skipped: 0git diff -U0 HEADon changed main code → one executable line changed (thePattern.compilecall), zero uncovered lines, zero uncovered branchesChecklist
Additional Notes
The rest of the diff is javadoc. The block above
BEARER_SHAPED_VALUEstill opened by calling itself "the bearer and JWT shapes", which was accurate while it documented one field and stopped being so when the split made two. It now says which shape each paragraph is about and why the two are documented together.