trailing slash middleware drops root

[mabiede]

@jerben I had issues with the trailing slash middleware.

1. A get route to the index page on "" (or "/")
2. Get Request on the root of the page (in dev e.g. localhost:3000)
3. 404 is returned (index function never gets called)

Behaviour description: The request to "/" is dropped by the trailing slash middleware and "" (empty string) cannot be matched in routes and a 404 is returned.

Adding an additional statement to check if uri is equal to root solves the issue:

let trailing_slash () =
  let filter handler req =
    let target = req.Opium.Request.target in
    let uri = target |> Uri.of_string in
    let uri =
      uri
      |> Uri.path
      |> (fun path ->
           if Uri.equal ("/" |> Uri.of_string) uri
           then path (* don't drop root *)
           else path |> CCString.rdrop_while (Char.equal '/'))
      |> Uri.with_path uri
    in
    let req = Opium.Request.{ req with target = Uri.to_string uri } in
    handler req
  in
  Rock.Middleware.create ~name:"trailing_slash" ~filter
;;

Related file: https://github.com/oxidizing/sihl/blob/master/sihl/src/web_trailing_slash.ml

[mabiede]

FYI: After several tests, I think the middleware is fine, but I'm still experiencing this issue.

[mabiede]

let router = choose
  [ get "/" Handler.Public.index
  ; get "" Handler.Public.index ]

Results in routes_of_router:

GET /
GET /

And with the trailing_slash middleware, it matches both to "" --> Results in 404 not found.

[mabiede]

Occurs with Sihl.1.0.0~rc2.

(Sihl.1.0.0~rc1 can have a routes_of_router GET (without slash))

[joseferben]

@mabiede Thanks for the detailed bug report! Fixed and merged: https://github.com/oxidizing/sihl/pull/507 I briefly considered turning a request to //// into / as well, but I am not sure yet whether this is the responsibility of the trailing slash middleware.