Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

static middleware: path ist unescaped twice for file names, leading to not downloadable content #2599

Open
3 tasks done
georgmu opened this issue Feb 29, 2024 · 0 comments
Open
3 tasks done

Comments

@georgmu
Copy link
Contributor

georgmu commented Feb 29, 2024

Issue Description

Given a file on disk with a percent sign in its name, it is not possible to download it using echo's static middleware.

Given some file names:

  • 100%.txt
  • foo%20bar.txt (this is really the name on disk)

It is not possible to download these files using the static middleware.

Checklist

  • Dependencies installed
  • No typos
  • Searched existing issues and docs

Expected behaviour

The standard way to download them would be using the folowing paths:

  • 100%.txt -> GET /100%25.txt
  • foo%20bar.txt -> GET /foo%2520bar.txt

Actual behaviour

  • 100%.txt -> GET /100%25.txt
    • echo error message: invalid URL escape "%.p (because of the double unescape and .p is no valid hex)
  • foo%20bar.txt -> GET /foo%2520bar.txt
    • echo error message: path does not exist (because file foo bar.txt does not exist, only foo%20bar.txt)

Background is that url.Path in http.Request is already escaped (as the documentation for url.Path suggests), but echo is unescaping it again.

I tried to generate a fix. For the non-embedded case this is solved by simply removing the explicit url.PathUnescape call in static.go (see georgmu@852dede which also contains some test cases). I haven't created a merge request yet, as the embedding using groups or path patterns is a bit more complicated (I haven't fully understand the case for the disablePathUnescaping toggle).

Steps to reproduce / Working code to debug

See test cases in georgmu@852dede

Version/commit

master ( commit fa70db8 )

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

No branches or pull requests

1 participant