leafo / lapis

A web framework for Lua and OpenResty written in MoonScript
http://leafo.net/lapis/
MIT License
3.08k stars 249 forks source link

route name cannot be one of the paths #767

Open bungle opened 1 year ago

bungle commented 1 year ago

This used to work before, but now there is this code: https://github.com/leafo/lapis/blob/v1.14.0/lapis/application.moon#L179-L194

    filled_routes = {}

    each_route @, true, (path, handler) ->
      route_name, path_string = if type(path) == "table"
        next(path), path[next path]
      else
        nil, path

      if route_name
        return if filled_routes[route_name]
        filled_routes[route_name] = true

      return if filled_routes[path_string]
      filled_routes[path_string] = true

      @router\add_route path, @wrap_handler handler

This means that if route_name == path_string OR that route_name has collision with other route's path_string the add_route may be skipped.

I have been trying to update this library from 1.8.3 to 1.14.0 in Kong, and found this to be problematic for us as we do: https://github.com/Kong/kong/blob/master/kong/api/api_helpers.lua#L429

app:match(route_path, route_path, app_helpers.respond_to(methods))

Not sure if this was intent, but just letting you know.

bungle commented 1 year ago

This would probably fix it:

    -- this will hold both paths and route names to prevent them from being
    -- redeclared by paths lower in precedence
    filled_route_names = {}
    filled_route_paths = {}

    each_route @, true, (path, handler) ->
      route_name, path_string = if type(path) == "table"
        next(path), path[next path]
      else
        nil, path

      if route_name
        return if filled_route_names[route_name]
        filled_route_names[route_name] = true

      return if filled_route_paths[path_string]
      filled_route_paths[path_string] = true

      @router\add_route path, @wrap_handler handler
leafo commented 4 months ago

This is related to https://github.com/leafo/lapis/issues/760

The route name was intended to be in the form of a lua module name, not a route path.