fix(url): ignore fragment identifiers in getPath() by sano-suguru · Pull Request #4627 · honojs/hono

sano-suguru · 2026-01-13T17:19:18Z

Summary

Modified getPath() to strip fragment identifiers (#) from URLs
This fixes incorrect route parameter parsing in Service Worker contexts

Changes

In Service Worker contexts, URLs may contain fragment identifiers (e.g., /users/1#profile-section) which were incorrectly included in route parameters. This change modifies getPath() to strip fragment identifiers, consistent with how query strings are already handled.

Before

/users/#user-list → id = "#user-list"
/users/1#profile-section → id = "1#profile-section"

After

/users/#user-list → matches /users/
/users/1#profile-section → id = "1"

Implementation

Based on the approach suggested by @usualoma in #4440:

Added # (charCode 35) to the loop termination condition
Handle fragment identifiers in the percent-encoded path branch

Test plan

Added tests for URLs with fragments
Added tests for URLs with both query strings and fragments
Added tests for percent-encoded paths with fragments
All existing tests pass

In Service Worker contexts, URLs may contain fragment identifiers (#) which should not be included in route matching. This change modifies getPath() to strip fragment identifiers from the path, consistent with how query strings are already handled. Fixes honojs#4440

When a URL contains a fragment with query-like characters (e.g., #section?foo=bar), the previous logic would incorrectly include the fragment in the path because it checked for '?' first and only looked for '#' if '?' was not found. This fix uses Math.min() to find whichever delimiter ('?' or '#') comes first, correctly handling edge cases per RFC 3986 where '#' terminates the path regardless of subsequent '?' characters in the fragment. Added test case for this edge case.

sano-suguru · 2026-01-13T17:27:08Z

Fixed: Percent-encoded path with fragment containing query-like characters

The previous implementation had a bug where it would check for ? first, and only look for # if ? was not found:

let separator = url.indexOf('?', i)
if (separator === -1) {
  separator = url.indexOf('#', i)
}

This caused incorrect behavior when a URL contains a fragment with query-like characters (e.g., https://example.com/hello%20world#section?foo=bar). In such cases, per RFC 3986, the ? after # is part of the fragment, not a query delimiter.

Fix

Changed to find whichever delimiter (? or #) comes first using Math.min():

const queryIndex = url.indexOf('?', i)
const hashIndex = url.indexOf('#', i)
const separator =
  queryIndex === -1 ? hashIndex : hashIndex === -1 ? queryIndex : Math.min(queryIndex, hashIndex)

Added Test

it('getPath - with percent encoding and fragment containing query-like chars', () => {
  const path = getPath(new Request('https://example.com/hello%20world#section?foo=bar'))
  expect(path).toBe('/hello world')
})

All tests pass.

Verify that percent-encoded hash (%23) is preserved while real fragment identifiers are correctly stripped. decodeURI preserves reserved characters per Web Standards.

usualoma

Thank you for creating the pull request! I think the direction is good.

With the current code, === -1 is evaluated three times for /path?x=1, so I think the following is better:

      const end =
        queryIndex === -1
          ? hashIndex === -1
            ? undefined
            : hashIndex
          : hashIndex === -1
          ? queryIndex
          : Math.min(queryIndex, hashIndex)
      const path = url.slice(start, end)

codecov · 2026-01-14T01:42:54Z

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.44%. Comparing base (28452f0) to head (8bdd859).
⚠️ Report is 18 commits behind head on main.

Files with missing lines	Patch %	Lines
src/utils/url.ts	90.90%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4627      +/-   ##
==========================================
- Coverage   91.44%   91.44%   -0.01%     
==========================================
  Files         173      173              
  Lines       11321    11330       +9     
  Branches     3280     3286       +6     
==========================================
+ Hits        10353    10361       +8     
- Misses        967      968       +1     
  Partials        1        1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Restructure ternary to avoid redundant separator === -1 check. For /path?x=1, reduces evaluations from 3 to 2. Co-authored-by: usualoma <usualoma@users.noreply.github.com>

Cover the undefined branch when both queryIndex and hashIndex are -1.

usualoma

Thank you!

sano-suguru · 2026-01-14T02:22:00Z

Thank you for the review and the optimization suggestion! I've updated the code as you suggested.

The restructured ternary now avoids the redundant separator === -1 check, reducing evaluations from 3 to 2 for URLs like /path?x=1.

I also added a test case to cover the undefined branch (when both queryIndex and hashIndex are -1).

(Sorry if my English is a bit rough - it's not my native language!)

yusukebe · 2026-01-20T09:08:05Z

src/utils/url.ts

      const queryIndex = url.indexOf('?', i)
-      const path = url.slice(start, queryIndex === -1 ? undefined : queryIndex)
+      const hashIndex = url.indexOf('#', i)
+      const end =


How about the following implementation? This is more short and readable.

const end = Math.min( queryIndex === -1 ? url.length : queryIndex, hashIndex === -1 ? url.length : hashIndex )

This runs Math.min every time, but in some cases that contain percent encoding, the path has a query.

@sano-suguru @usualoma What do you think of it?

@yusukebe
Sorry I missed that. Either one seems fine.
The character count after minification is probably the same, right?

Math.min(a===-1?u.length:a,b===-1?u.length:b)

a===-1?b===-1?void 0:b:b===-1?a:Math.min(a,b)

Performance-wise, there probably won't be a noticeable difference in production. I prefer the current implementation, but either one is fine.

Sorry I missed that. Either one seems fine.

No problem!

The character count after minification is probably the same, right?

Ah, yes. So it's just for readability. If you feel both are okay, I think we should keep the current implementation.

yusukebe

LGTM!

yusukebe · 2026-02-08T12:50:25Z

@sano-suguru

I've left a small comment, but let's go with this implementation. Looks good. Thanks!

sanosuguru added 2 commits January 14, 2026 02:18

test(url): add test for encoded hash (%23) in path with fragment

cddac17

Verify that percent-encoded hash (%23) is preserved while real fragment identifiers are correctly stripped. decodeURI preserves reserved characters per Web Standards.

usualoma reviewed Jan 13, 2026

View reviewed changes

sanosuguru and others added 2 commits January 14, 2026 11:13

perf(url): reduce === -1 evaluations in getPath

ff8ee74

Restructure ternary to avoid redundant separator === -1 check. For /path?x=1, reduces evaluations from 3 to 2. Co-authored-by: usualoma <usualoma@users.noreply.github.com>

test(url): add coverage for percent encoding without query or fragment

8bdd859

Cover the undefined branch when both queryIndex and hashIndex are -1.

usualoma approved these changes Jan 14, 2026

View reviewed changes

yusukebe reviewed Jan 20, 2026

View reviewed changes

yusukebe approved these changes Feb 8, 2026

View reviewed changes

yusukebe merged commit 0c1d4c7 into honojs:main Feb 8, 2026
20 checks passed

claude bot mentioned this pull request Feb 9, 2026

chore(deps-dev): bump hono from 4.11.8 to 4.11.9 in /r2-explorer Bad3r/nix-R2-CloudFlare-Flake#43

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(url): ignore fragment identifiers in getPath()#4627

fix(url): ignore fragment identifiers in getPath()#4627
yusukebe merged 5 commits intohonojs:mainfrom
sano-suguru:fix/getpath-ignore-fragment

sano-suguru commented Jan 13, 2026

Uh oh!

sano-suguru commented Jan 13, 2026

Uh oh!

usualoma left a comment

Uh oh!

codecov bot commented Jan 14, 2026 •

edited

Loading

Uh oh!

usualoma left a comment

Uh oh!

sano-suguru commented Jan 14, 2026

Uh oh!

yusukebe Jan 20, 2026

Uh oh!

usualoma Feb 8, 2026

Uh oh!

yusukebe Feb 8, 2026

Uh oh!

yusukebe left a comment

Uh oh!

yusukebe commented Feb 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

sano-suguru commented Jan 13, 2026

Summary

Changes

Before

After

Implementation

Test plan

Uh oh!

sano-suguru commented Jan 13, 2026

Fixed: Percent-encoded path with fragment containing query-like characters

Fix

Added Test

Uh oh!

usualoma left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

usualoma left a comment

Choose a reason for hiding this comment

Uh oh!

sano-suguru commented Jan 14, 2026

Uh oh!

yusukebe Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

usualoma Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

yusukebe Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

yusukebe left a comment

Choose a reason for hiding this comment

Uh oh!

yusukebe commented Feb 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jan 14, 2026 •

edited

Loading