Repository navigation
serve dlna: log unescaped paths - fixes #7370 - #9950
Merged
Merged
Conversation
The request log printed the escaped URL, so non-ASCII file names showed up as long runs of %XX escapes. Log the unescaped URL path instead, as the other serve commands do.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this change do?
serve dlnalogged each request againstrequest.URL, which prints escaped. Non-ASCII file names showed up as runs of%XX:It now logs
request.URL.Path, the unescaped path:This is what the other serve loggers already use: webdav, s3, the
lib/httpauth logging, and dlna's own panic log.There is no flag, as agreed in the issue. As a result:
/(%2F) shows as/.Both are the same as in the other serve commands.
Testing
TestLoggingUnescapedPathsends a request with an escaped Cyrillic path containing a space throughlogging(), and checks that the log line has the unescaped path. It fails on master.bilib.CaptureOutput, as thefs/syncandfs/operationstests do. It uses a 404 so the line is logged at ERROR, which means it doesn't have to changeci.LogLevel. Changing that would race under-racewith the SSDP goroutines of the dlna server thatTestInitleaves running.rclone serve dlnaon a directory holdingБарды/Павелецкий вокзал.mp3and fetched the file withcurl. The two log lines above are from master and from this branch.go build ./...,make quicktest,go test -race ./cmd/serve/dlna/,go vetandgolangci-lint run ./cmd/serve/dlna/....Linked issue
Fixes #7370
Checklist
test_allpasses for this backend. n/a, not a backend change.