trimTrailingSlash doesn't work on paths with *

[victormihalache]

What version of Hono are you using?

4.6.1

What runtime/platform is your app running on?

Cloudflare Workers

What steps can reproduce the bug?

Just make a Hono project and put this in src/index.ts

import { Hono } from "hono";
import { trimTrailingSlash } from "hono/trailing-slash";

const app = new Hono();

app.use(trimTrailingSlash());

app.get("/my-path/*", (c) => {
  return c.text("hi");
});

export default app;

What is the expected behavior?

Navigating to http://localhost:8787/my-path/something/else/ should redirect to http://localhost:8787/my-path/arst/arst

What do you see instead?

It stays the exact same :D

Additional information

Manually putting the trimTrailingSlash code inside the function will work:

app.get("/my-path/*", (c) => {
  if (c.req.path !== "/" && c.req.path[c.req.path.length - 1] === "/") {
    const url = new URL(c.req.url);
    url.pathname = url.pathname.substring(0, url.pathname.length - 1);
    return c.redirect(url.toString(), 301);
  }

  return c.text("hi");
});

[victormihalache]

Whilst waiting for a fix (if this is considered a bug) this version is cleaner and does the same thing:

if (ctx.req.path.endsWith("/")) {
  return ctx.redirect(ctx.req.url.slice(0, -1), 301);
}

[victormihalache]

To fix the issue all that needs to be done is remove the check for a 404 page here and here (for appending).

When I use this middleware I actually expect any page, regardless of anything else, to get the trailing slash trimmed; therefore I don't understand why if the page is found one should not obey that rule.

An option like "strict" or "always" could be added such that that check is removed (or, the other way around, add one called something like "onlyUnhandled" to get the current behaviour and make the new one the default).

[MathurAditya724]

Interesting, and I think we should do the redirecting before await next(), because we will also be running the handler/s even tho we end upon redirecting the request. Based upon this code -

import { Hono } from "hono";
import { trimTrailingSlash } from "hono/trailing-slash";

const app = new Hono().use(trimTrailingSlash());

app.get("/sample/*", (c) => {
  console.log("This endpoint was called!");
  return c.text("Hello Hono!");
});

export default app;

I can work on a PR to resolve this

[victormihalache]

The only issue I can see arising from this is when you have a specific path for which you want to keep the trailing slash regardless of the overall rule (and the same goes for the other function, where you might want to remove a trailing slash for a particular set of routes).

The issue I'm facing is that when going any route like /my-app, /my-app/, or /my-app/something/else(/) I want to "proxy"/"forward" the request to somewhere else, like a Cloudflare Pages app. To achieve this I do the following:

const application = new Hono();

application.get("/my-app", (context) => {
  return context.redirect("/my-app/");
});

application.on(
  "GET",
  ["/my-app/", "/my-app/:path{.+$}"],
  async (context) => {
    if (context.req.path.endsWith("/") && context.req.path !== "/my-app/") {
      return context.redirect(context.req.url.slice(0, -1), 301);
    }

    return await fetch(`https://my-other-app.pages.dev/${context.req.param("path") || ""}`);
  }
);

Setting aside the fact that I currently have my own copy of the trimpTrailingSlash for /my-app/*, you can see how I had to set up a redirect from /my-app to /my-app/ and how I've excluded /my-app/ from getting the trailing slash trimmed, to kind-of act like you'd expect the root / to act (can never trim that you). This is because SvelteKit (the app I'm "proxying") fetches resources from paths like: ./_app/immutable/assets/0.DrT8Lpa1.css and ./_app/immutable/entry/start.CCT2RJoh.js when on the root /, but ../_app/immutable/assets/0.DrT8Lpa1.css when in a subroute like /something/nested (notice the ./ and ../ at the beginning). For reference look at the note under the trailingSlash section on SvelteKit's documentation; it was the issue I was facing:

[...] the semantics of relative paths differ between the two cases (./y from /x is /y, but from /x/ is /x/y), and /x and /x/ are treated as separate URLs [...]

When I wasn't forcing a trailing slash only for /my-app/ the SvelteKit app would try to fetch from http://localhost/_app/... instead of http:/localhost/my-app/_app/..., resulting in a 404.

Having the adapter enforce the trailing slash for truly any path would help already, and having an exclude option I can pass that just adds those strings in a check before trimming would be a perfect solution to this use-case.

[MathurAditya724]

I think the best way for you to configure this would be to have 2 different route handlers, One with the trimTrailingSlash middleware and the other with appendTrailingSlash to enforce it on them.

Because I believe if you have applied this middleware, irrespective of the handler if there is a / at the end of the route we should remove it (other than the base home route) and vice versa for the other one.

Something like this -

import { Hono } from "hono";
import { trimTrailingSlash, appendTrailingSlash } from "hono/trailing-slash";

const routerWithTrim = new Hono().use(trimTrailingSlash());

routerWithTrim.get("/some", (c) => {
  return c.text("Hello Hono!");
});

const routerWithoutTrim = new Hono().use(appendTrailingSlash());

routerWithoutTrim.get("/another/", (c) => {
  return c.text("Hello Hono!");
});

const app = new Hono().route("/", routerWithTrim).route("/", routerWithoutTrim);

export default app;

[victormihalache]

I go agree that that should be default behaviour. What I don't agree with, however, is not allowing for any exceptions given how easy it is to implement, and given that it's the only configuration one might ever need to set (like in my case).

Maybe 99% of people won't use it, but those that do will have to resort to copy-pasting the implementation in the code, and tweak it to handle their exception, since it's trivial to do.

In any case, enforcing the default at all times, unlike the current implementation, is already a good step forward!