Closed endelwar closed 4 years ago
Thanks for the great report. I was able to reproduce the bug, so finding a fix will be much easier with the information you provided.
This is caused because the {http.request.uri.path}
placeholder uses the req.URL.Path
value. Using req.URL.RawPath
(or req.URL.EscapedPath()
) fixes it, but this is the un-decoded form of the URL (i.e. Path is unescaped, and RawPath is escaped).
So this is fun: it turns out len(str) != len(strings.ToLower(str))
https://play.golang.org/p/DrkIBkXcfUq
Still looking for a fix without changing the placeholder (since I think most users want the path placeholder to return the unescaped path).
Okay, so the std lib has strings.EqualFold()
which does case-insensitive string comparison, but there's no case-insensitive substring search (i.e. strings.IndexFold()
) which is really what we need here.
So I wrote my own, and now the tests pass (including a new one for this issue), so I hope that means it's correct:
func indexFold(haystack, needle string) int {
nlen := len(needle)
for i := 0; i+nlen < len(haystack); i++ {
if strings.EqualFold(haystack[i:i+nlen], needle) {
return i
}
}
return -1
}
This should fix the problem because before we were finding a position using the lower-cased version of the path, then using that same position with the original version of the path, which has a different length (😬)... this solution does not create a lowercase version of the path, but still does case-insensitive substring searching.
Should be fixed now, please give it a try with the latest build artifacts!
I've got zero new panic in the last 10 days of tests, fix is working as expected, thaks!
I'm seeing in caddy logs some panics, here is a sample:
this is an extract of the relative access.log:
Other URIs that trigger the panic are these:
I've found both GET and HEAD requests, coming from bing search and a webscraper looking for specific archive files in webroot
This happens on a debian 9 with Caddy 2.1.1 with no additional modules.
Caddyfile is the following: