APItools / router.lua

A barebones router for Lua. It matches urls and executes lua functions.
MIT License
195 stars 47 forks source link

adding support for reading args without preceding / e.g /hello?a=b #14

Closed rohitjoshi closed 8 years ago

rohitjoshi commented 8 years ago

This PR fixes issue when query parameters are passed without preceding / e.g below request uri doesn't match to ["/hello"]

GET  /hello?name=rjoshi  

will not match to

 r:match({
   GET = {
      ["/hello"]       = function(params) ngx.print("someone said hello " .. params.name) end
    }

With this PR, it will work for below both

GET /hello?name=rjoshi 
GET /hello/?name=rjoshi
lloydzhou commented 8 years ago

@rohitjoshi just rtrim the URI using "/", before call execute method can doing this feature.

rohitjoshi commented 8 years ago

@lloydzhou issue is not / at the end. If request URI has query parameters without /, router doesn't match the request. e.g GET /hello?name=value doesn't match to ["/hello"]. See same issue is reported by https://github.com/APItools/router.lua/issues/10

lloydzhou commented 8 years ago
ngx.var.request_uri    include query string
ngx.var.uri            do not include query string
rohitjoshi commented 8 years ago

Ok, got it. you might want to update the example at https://github.com/APItools/router.lua/blob/master/README.md#usage-with-openresty

Few weeks ago, I had tried router.lua and gave up. I see other person have reported same issue. https://github.com/APItools/router.lua/issues/10

lloydzhou commented 8 years ago

@rohitjoshi i'm not the owner of this project. you can create new pull request or ask @kikito to update the example.

kikito commented 8 years ago

Hi there, I am no longer part of 3Scale or APItools so I can't directly make these changes neither. Pinging @mikz in case he wants to update the example in the blog and close this.

mikz commented 8 years ago

Sorry. Totally missed this. Thanks @kikito for bringing it up.

mikz commented 8 years ago

I'm all in for merging this. But to do so I'd need:

Now it looks like it is a README issue, rather than issue in the code.