This is a follow-up to #1101. It turns out that that patch is
incomplete, as a similar check also needs to be added in the
`Router.Add()` method.
I don't understand why the test works fine, but when using it in a real
application. For example with this example:
func main() {
e := echo.New()
e.GET("/xxx/:id.json", func(c echo.Context) error {
return c.String(200, fmt.Sprintf("%#v: names: %#v; vals: %#v",
c.Path(), c.ParamNames(), c.ParamValues()))
})
log.Fatal(e.Start(":8000"))
}
Gives a 404 on `/xxx/42.json`, and for `/xxx/42` it gives the output:
/xxx/:id.json": names: []string{"id.json"}; vals: []string{"42"}
It makes sense to add the test there too; I just don't get why the test
cases that I added in ##1101 *does* produce the correct output :-/
Currently a route in the form of `/foo/:id.json` means echo will detect
the parameter name `id.json` instead of the expected `id`. I think this
is rather counter-intuitive, as adding an extension to paths is a fairly
common use case.
With this change both a `/` and a `.` will be treated as the end of a
parameter name.
Benchmark before this change:
$ go test -bench .
[..]
goos: linux
goarch: amd64
pkg: github.com/labstack/echo
BenchmarkRouterStaticRoutes-4 100000 17743 ns/op 0 B/op 0 allocs/op
BenchmarkRouterGitHubAPI-4 50000 33081 ns/op 1 B/op 0 allocs/op
BenchmarkRouterParseAPI-4 300000 5370 ns/op 0 B/op 0 allocs/op
BenchmarkRouterGooglePlusAPI-4 200000 9183 ns/op 0 B/op 0 allocs/op
PASS
ok github.com/labstack/echo 8.565s
After this change:
goos: linux
goarch: amd64
pkg: github.com/labstack/echo
BenchmarkRouterStaticRoutes-4 100000 17699 ns/op 0 B/op 0 allocs/op
BenchmarkRouterGitHubAPI-4 50000 32962 ns/op 1 B/op 0 allocs/op
BenchmarkRouterParseAPI-4 300000 5450 ns/op 0 B/op 0 allocs/op
BenchmarkRouterGooglePlusAPI-4 200000 9205 ns/op 0 B/op 0 allocs/op
PASS
ok github.com/labstack/echo 8.590s
Fixes#599
Enhancements:
Implemented Response#After()
Dynamically add/remove proxy targets
Rewrite rules for proxy middleware
Add ability to extract key from a form field
Implemented rewrite middleware
Adds a separate flag for the 'http/https server started on' message (#1043)
Applied a little DRY to the redirect middleware (#1053) and tests (#1055)
Simplify dep fetching (#1062)
Add custom time stamp format #1046 (#1066)
Update property name & default value & comment about custom logger
Add X-Requested-With header constant
Return error of context.File in c.contentDisposition
Updated deps
Updated README.md
Fixes:
Fixed Response#Before()
Fixed#990
Fix href url from armor to echo (#1051)
Fixed#1054Fixed#1052, dropped param alias feature
Avoid redirect loop in HTTPSRedirect middleware (#1058)
Fix deferring body close in middleware/compress test (#1063)
Cleanup code (#1061)
FIX - We must close gzip.Reader, only if no error (#1069)
Fix formatting (#1071)
Can be a fix for auto TLS
context.Attachment and context.Inline use context.contentDisposition under the hood.
However, context.contentDisposition does not forward the error of context.File, leading to response 200 OK even when the file does not exist.
This commit forward the return value of context.File to context.contentDisposition to prevent that.
In HTTPSRedirect and similar middlewares, determining if redirection is
needed using `c.IsTLS()` causes redirect loop when an application is running
behind a TLS termination proxy, e.g. AWS ELB.
Instead, I believe, redirection should be determined by
`c.Scheme() != "https"`. This works well even behind a TLS termination proxy.
