search

xitrum-framework / xitrum

Async and clustered Scala web framework and HTTP(S) server
http://xitrum-framework.github.io/
MIT License
446 stars 52 forks source link

%2F in path params #647

Closed georgeOsdDev closed 7 years ago

georgeOsdDev commented 7 years ago

Hi! 3.28 has breaking change from 3.27. pathParam which contain %2F now not match named pathParam, but only :* capture this. Is this expected behavior?

Routes

@GET("test1/:p1")
class SiteIndex1 extends DefaultLayout {
  def execute() {
    respondText(s"""p1 -> ${param[String]("p1")}""")
  }
}

@GET("test2/:*")
class SiteIndex2 extends DefaultLayout {
  def execute() {
    respondText(s"""* -> ${param[String]("*")}""")
  }
}

Xitrum 3.28.3

[INFO] Normal routes:
GET  /           quickstart.action.SiteIndex
GET  /test1/:p1  quickstart.action.SiteIndex1
GET  /test2/:*   quickstart.action.SiteIndex2
[INFO] Error routes:
404  quickstart.action.NotFoundError
500  quickstart.action.ServerError
[INFO] Xitrum routes:
GET  /xitrum/xitrum-3.28.3.js  xitrum.js
GET  /xitrum/metrics/viewer    xitrum.metrics.XitrumMetricsViewer
[INFO] Xitrum SockJS routes:
/xitrum/metrics/channel  xitrum.metrics.XitrumMetricsChannel  websocket: true, cookie_needed: false
[INFO] HTTP server started on port 8000
[INFO] HTTPS server started on port 4430
[INFO] Xitrum 3.28.3 started in development mode
[WARN] *** For security, change secureKey in config/xitrum.conf to your own! ***
[INFO] 0:0:0:0:0:0:0:1 GET /test1/123 -> quickstart.action.SiteIndex1, pathParams: {p1: 123} -> 200, 146 [ms]
[INFO] 0:0:0:0:0:0:0:1 GET /test1/123/456 -> quickstart.action.NotFoundError -> 404, 862 [ms]
[INFO] 0:0:0:0:0:0:0:1 GET /test1/123%2F456 -> quickstart.action.NotFoundError -> 404, 8 [ms]
[INFO] 0:0:0:0:0:0:0:1 GET /test2/123 -> quickstart.action.SiteIndex2, pathParams: {*: 123} -> 200, 4 [ms]
[INFO] 0:0:0:0:0:0:0:1 GET /test2/123/456 -> quickstart.action.SiteIndex2, pathParams: {*: 123/456} -> 200, 2 [ms]
[INFO] 0:0:0:0:0:0:0:1 GET /test2/123%2F456 -> quickstart.action.SiteIndex2, pathParams: {*: 123/456} -> 200, 2 [ms]

Xitrum 3.27.0

[INFO] Normal routes:
GET  /           quickstart.action.SiteIndex
GET  /test1/:p1  quickstart.action.SiteIndex1
GET  /test2/:*   quickstart.action.SiteIndex2
[INFO] Error routes:
404  quickstart.action.NotFoundError
500  quickstart.action.ServerError
[INFO] Xitrum routes:
GET  /xitrum/xitrum.js       xitrum.js
GET  /xitrum/metrics/viewer  xitrum.metrics.XitrumMetricsViewer
[INFO] Xitrum SockJS routes:
/xitrum/metrics/channel  xitrum.metrics.XitrumMetricsChannel  websocket: true, cookie_needed: false
[INFO] HTTP server started on port 8000
[INFO] HTTPS server started on port 4430
[INFO] Xitrum 3.27.0 started in development mode
[WARN] *** For security, change secureKey in config/xitrum.conf to your own! ***
[INFO] 0:0:0:0:0:0:0:1 GET /test1/123 -> quickstart.action.SiteIndex1, pathParams: {p1: 123} -> 200, 125 [ms]
[INFO] 0:0:0:0:0:0:0:1 GET /test1/123/456 -> quickstart.action.NotFoundError -> 404, 4199 [ms]
[INFO] 0:0:0:0:0:0:0:1 GET /test1/123%2F456 -> quickstart.action.SiteIndex1, pathParams: {p1: 123/456} -> 200, 3 [ms]
[INFO] 0:0:0:0:0:0:0:1 GET /test2/123 -> quickstart.action.SiteIndex2, pathParams: {*: 123} -> 200, 8 [ms]
[INFO] 0:0:0:0:0:0:0:1 GET /test2/123/456 -> quickstart.action.SiteIndex2, pathParams: {*: 123/456} -> 200, 8 [ms]
[INFO] 0:0:0:0:0:0:0:1 GET /test2/123%2F456 -> quickstart.action.SiteIndex2, pathParams: {*: 123/456} -> 200, 1 [ms]

Related? https://github.com/xitrum-framework/xitrum/issues/627

ngocdaothanh commented 7 years ago

Yes I think #627 may cause this problem. I'll investigate more.

georgeOsdDev commented 7 years ago

🎉 🎉 🍻