Fix cache rendering with namespaced resources #874
Conversation
The path traversal sanitization was incorrectly filtering out paths that included the `-` character. Namespaced resources generates files like `api-v1-resource.html` which were then returning a 400.
| path << "/" << params[:version] if params[:version].present? | ||
| path << "/" << params[:resource] if params[:resource].present? | ||
| path << "/" << params[:method] if params[:method].present? | ||
| if params[:format].present? | ||
| path << ".#{params[:format]}" |
There was a problem hiding this comment.
but we are not adding this anymore?
| end | ||
| # Sanitize path against directory traversal attacks (e.g. ../../foo) | ||
| # by turning path into an absolute path before appending it to the cache dir | ||
| path = File.expand_path("#{path}.#{request.format.symbol}", '/') |
There was a problem hiding this comment.
I see you are taking the format from the request, but can it be different?
There was a problem hiding this comment.
Hi! Sorry I didn't respond earlier - yesterday was really busy.
Since Rails/Rack assemble the request format for us, I figured it was safest to use request.format rather than us needing to sanitize params[:format] ourselves. It's one small security item we don't have to do.
If we had a large variety request formats it might be possible for request.format.symbol to be different from the file extension on the file system, but we only have html and json which are pretty standard. I don't think it can be different.
I'm more than willing to change this in a new PR if you think we should. Thank you for merging it! (And all my other recent PRs - you are super responsive which is not often found 🎉 )
The path traversal sanitization was incorrectly filtering out paths that included the
-character. Namespaced resources generates files likeapi-v1-resource.htmlwhich were then returning a 400.