Skip to content

serve dlna: log unescaped paths - fixes #7370 - #9950

Merged
ncw merged 1 commit into
rclone:masterfrom
hsdfat:dlna-log-unescaped-path-7370
Sep 21, 2026
Merged

ncw merged 1 commit into
rclone:masterfrom
hsdfat:dlna-log-unescaped-path-7370

Conversation

@hsdfat

@hsdfat hsdfat commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

What does this change do?

serve dlna logged each request against request.URL, which prints escaped. Non-ASCII file names showed up as runs of %XX:

INFO  : /r/%D0%91%D0%B0%D1%80%D0%B4%D1%8B/%D0%9F%D0%B0%D0%B2%D0%B5%D0%BB%D0%B5%D1%86%D0%BA%D0%B8%D0%B9%20%D0%B2%D0%BE%D0%BA%D0%B7%D0%B0%D0%BB.mp3: 127.0.0.1:54940 GET 200

It now logs request.URL.Path, the unescaped path:

INFO  : /r/Барды/Павелецкий вокзал.mp3: 127.0.0.1:36062 GET 200

This is what the other serve loggers already use: webdav, s3, the lib/http auth logging, and dlna's own panic log.

There is no flag, as agreed in the issue. As a result:

  • the query string is no longer shown;
  • an escaped / (%2F) shows as /.

Both are the same as in the other serve commands.

Testing

  • TestLoggingUnescapedPath sends a request with an escaped Cyrillic path containing a space through logging(), and checks that the log line has the unescaped path. It fails on master.
  • The test captures the log with bilib.CaptureOutput, as the fs/sync and fs/operations tests do. It uses a 404 so the line is logged at ERROR, which means it doesn't have to change ci.LogLevel. Changing that would race under -race with the SSDP goroutines of the dlna server that TestInit leaves running.
  • Manual check: I ran rclone serve dlna on a directory holding Барды/Павелецкий вокзал.mp3 and fetched the file with curl. The two log lines above are from master and from this branch.
  • These pass: go build ./..., make quicktest, go test -race ./cmd/serve/dlna/, go vet and golangci-lint run ./cmd/serve/dlna/....

Linked issue

Fixes #7370

Checklist

  • This change is trivial OR it has been discussed and agreed in the linked issue.
  • I have read the contribution guidelines.
  • (If I used AI tools to help write this code) I have read and understood the AI-assisted contributions guidance, and I have tested and take ownership of this change myself.
  • I have added tests for all changes in this PR if appropriate.
  • I have added documentation for the changes if appropriate. (No doc change needed: log output only.)
  • All commit messages are in house style.
  • (Backend changes only) test_all passes for this backend. n/a, not a backend change.
  • This Pull Request is ready for review.

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.

@ncw ncw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix - thank you.

@ncw
ncw merged commit 556940e into rclone:master Sep 21, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

serve logging should use unscaped URIs

2 participants