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

Use relative paths in directory listing URLs #136

Closed
jtackaberry opened this issue Sep 9, 2022 · 12 comments
Closed

Use relative paths in directory listing URLs #136

jtackaberry opened this issue Sep 9, 2022 · 12 comments
Labels
enhancement New feature or request v2 v2 release

Comments

@jtackaberry
Copy link

I run SWS behind an ingress controller in Kubernetes and typically use subpaths on the ingress, rewriting paths relative to / when proxying to SWS. This has been working perfectly, because of course serving static files is a relatively simple task. :)

I had a need to enable directory listings and noticed the URLs become invalid with this configuration due to the use of absolute paths relative to /

The simplest fix is to use relative URLs (./filename) in the directory listing HTML. The user's browser will take care of the rest, providing valid URLs that get cleanly rewritten by the upstream proxy.

Then things should work properly regardless of whether SWS is behind a proxy subpath, and no additional user configuration is required. I don't see any downside?

@joseluisq joseluisq added enhancement New feature or request v2 v2 release labels Sep 9, 2022
@joseluisq
Copy link
Collaborator

Sounds reasonable.
However, I would like to suggest going with filename/ dir paths instead of ./filename. Similar to how Nginx addresses this, just to give an example. What do you think?
BTW I also don't see any impact in doing it so.

@jtackaberry
Copy link
Author

Yep that works. At first I was thinking about the scenario where the user was hitting a directory URL without the trailing slash, but testing the non-./-prefixed path out just now with Firefox and Chrome, they handle that just fine. And if Nginx does it that way, that's a compelling enough reason.

Thanks!

@joseluisq
Copy link
Collaborator

Alright!
BTW since SWS provides a redirect-trailing-slash flag that can potentially be false then I will fall back to absolute paths only in this case because using relative dir paths without a trailing slash will definitely be a "breaking change".

@jtackaberry
Copy link
Author

Why would it be breaking if redirect-trailing-slash is false? Clients will generate the same URL whether the current URL (the one with the directory listing) has a trailing slash or not. (That's what I verified in my previous comment.)

@joseluisq
Copy link
Collaborator

joseluisq commented Sep 9, 2022

In the context of relative paths discussed in the issue, what I meant is the following:
Suppose that you have a http://localhost with redirect-trailing-slash=false but if you enter to http://localhost/assets/ adding a trailing slash then the auto index page can be broken for other dirs in the list.
For example, if there is a assets/images/ then it will become http://localhost/assets/assets/images/ when the link is clicked on the browser.

So basically what we have to make sure of is:

  • when there is a trailing slash in the request URI
    • then don't use the current request URI path (base path) as a prefix for every dir entry in the auto index (nginx approach)
  • otherwise when there is no trailing slash in the request URI
    • then use the current request URI path (base path) as a prefix for every dir entry in the auto index

@jtackaberry
Copy link
Author

jtackaberry commented Sep 9, 2022

But if you're accessing http://localhost/assets/ wouldn't the index entry for the images directory just be images/ such that it would fully resolve to https://localhost/assets/images/? (And it would resolve this way whether the URL was http://localhost/assets or http://localhost/assets/.) I feel like I must be missing something basic. :(

Edit: that is, the directory index links are always relative to the request URI, not SERVER_ROOT.

@jtackaberry
Copy link
Author

Ahh, wait, indeed, I was missing something basic. The way I tested how browsers generate absolute URIs from relative paths with URIs ending in / vs not was invalid. I understand the challenge now.

@joseluisq
Copy link
Collaborator

But if you're accessing http://localhost/assets/ wouldn't the index entry for the images directory just be images/ such that it would fully resolve to https://localhost/assets/images/?

No, if you enter to http://localhost/assets and you have images/ as an index entry when that entry is clicked then the browser will redirect you to http://localhost/images/ (tested).
That's why the current base dir as a prefix should work for this use case.

@jtackaberry
Copy link
Author

jtackaberry commented Sep 9, 2022

No, if you enter to http://localhost/assets and you have images/ as an index entry when that entry is clicked then the browser will redirect you to http://localhost/images/ (tested).

I'm with you now. Thanks for the cluebatting. My previous tests were invalid and that lead me down the wrong path. (Sorry for the pun.)

That's why the current base dir as a prefix should work for this use case.

But only the last component of the path should be used as the base in that case. That is, if the request is for http://localhost/a/b/c and there's a directory images/ in that path, then the link would be c/images/. (Perhaps this is what you meant -- just making sure. :))

@joseluisq
Copy link
Collaborator

But only the last component of the path should be used as the base in that case.

Yes, that's correct.

So basically what we have to make sure of is:

  • when there is a trailing slash in the request URI

    • then don't use the current request URI path (base path) as a prefix for every dir entry in the auto index (nginx approach)
  • otherwise when there is no trailing slash in the request URI

    • then use the current request URI path (base path) as a prefix for every dir entry in the auto index

That's why I remarked this above to take it into account for both cases.

@jtackaberry
Copy link
Author

Yep, it all makes sense now. Thanks Jose!

@joseluisq
Copy link
Collaborator

Released on v2.12.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v2 v2 release
Projects
None yet
Development

No branches or pull requests

2 participants