Mounting at `/` exhibits odd behavior

[blairj09]

Reprex:

  pr() %>% 
    pr_mount("/", 
             pr() %>% 
               pr_get("/hello", function() "hi")
    ) %>% 
    pr_run()

When this is executed, docs doesn't render and returns a 404.

[meztez]

I have a clumsy fix

in plumber.R

...
# We aren't going to serve this endpoint; see if any mounted routers will
      mountSteps <- lapply(sort(names(private$mnts), decreasing = TRUE), function(mountPath) {

We use this condition to see if a mounted router can be used to serve request which is why both "/" and "/docs" validate. Since "/" subrouter is executed first, you get a 404 error.

if (nchar(path) >= nchar(mountPath) && substr(path, 0, nchar(mountPath)) == mountPath) {

I will try to think of a better solution before submitting a patch. Decrease sorting works because it will evaluates the longest path first.

[meztez]

This could also happens if we had two routers, one at "/aaa/" and one at "/aaa/bbb/".

pr <- pr() %>% 
  pr_mount("/aaa", pr()) %>%
  pr_mount("/aaa/bbb",
           pr() %>% 
             pr_get("/hello", function() "hi")
  ) %>% pr_run()

I'm thinking either we sort mounts on mounting with something like

mount = function(path, router) {
      # Ensure that the path has both a leading and trailing slash.
      if (!grepl("^/", path)) {
        path <- paste0("/", path)
      }
      if (!grepl("/$", path)) {
        path <- paste0(path, "/")
      }

      private$mnts[[path]] <- router
      private$mnts <- private$mnts[order(names(private$mnts), decreasing = TRUE)]
    }

or what I suggested above.

It still is not clear to reviewer reading the code why we need to sort so I think a better alternative should be found.

Or document a priority behavior on which router is going to be used to route a request when multiple subrouter match the PATH_INFO.

I believe documentation was not a subrouter before so this behavior did not happen.

[schloerke]

👀

[schloerke]

@meztez Great analysis and fix!

In the case of /aaa/bbb/foo and mounts ordered c(/aaa/bbb, /aaa), I think it is fair to say that a request of /aaa/bbb/foo should attempt through /aaa/bbb (vs /aaa). ... the mount with the largest match.

I still don't fully like the end solution as maybe /aaa can handle /aaa/bbb/foo. However, this is no worse than the current implementation.

PR incoming

[schloerke]

We have decided to punt on this issue for the v1.1.0 release. The changes were possibly too jarring and could be resolved via pr_merge().