JuliaDocs / LiveServer.jl

Simple development server with live-reload capability for Julia.
MIT License
143 stars 26 forks source link

Use of WEB_DIR [#135] #148

Closed tlienart closed 2 years ago

tlienart commented 2 years ago

closes #135 (after the initial work in #144)

@mortenpi, could I get you to try this branch to see if it does indeed address the problem you mentioned in https://github.com/tlienart/LiveServer.jl/issues/135#issuecomment-1200321552 ? It does seem to on my side but it's probably best to have another pair of eyes on this.

After that and once I've had a chance to use this a bit more with Franklin, I'll do a patch release with it.

Note as there's a bunch of irrelevant cleanups in this PR, if someone reading this wants to know specifically what changed, mainly this:

https://github.com/tlienart/LiveServer.jl/pull/148/commits/9344eb93338684d02f1ea47ab6c3e8b7a6d3ee5d#diff-155622b290a60064e636101f12642b916c4120041dfe9e7f24c323bcf9a71152L98-L120

i.e. there's now a CONTENT_DIR and a WEB_DIR. The former keeps track of the path to the directory when using serve(dir=XXX) (then CONTENT_DIR[] = XXX), the latter is assigned once a user moves to a "web directory" i.e. a folder that has a index.html in it.

So if the file foo/bar/index.html exists and a user navigates to

the web directory will be WEB_DIR[] = "foo/bar".

This is used when figuring out the path of assets that may be mentioned on index.html or elsewhere with things like href="sample.jpeg" (to figure out with respect to what that path is given).

tlienart commented 2 years ago

Added set/reset functions for *_DIR thanks for the suggestion @rikhuijzer

mortenpi commented 2 years ago

@tlienart Unfortunately it seems it doesn't. It still serves a 200 OK, rather than a 301 Redirect on the URLs that don't have a trailing slash:

$ curl -I http://localhost:8000/dev/Documenter/docs/build
HTTP/1.1 200 OK
Content-Type: text/html; charset=utf-8
tlienart commented 2 years ago

Yes the change is basically just that when you navigate to a directory that has an index.html in it, it keeps track of it as the "root" of the website and computes all subsequent paths relative to that.

Did you still have an issue where an image didn't show or the CSS or whatever that I didn't see?

mortenpi commented 2 years ago

Did you still have an issue where an image didn't show or the CSS or whatever that I didn't see?

Yep: Screenshot from 2022-09-07 18-46-56

Yes the change is basically just that when you navigate to a directory that has an index.html in it, it keeps track of it as the "root" of the website and computes all subsequent paths relative to that.

I don't quite follow. The problem is that if, say, /foo/bar/index.html references assets/style.css (a relative URL), then if the URL is /foo/bar the browser interprets and requests the relative link as /foo/assets/style.css. If the URL is /foo/bar/, then it requests /foo/bar/assets/style.css. I assume you're not trying to rewrite the file foo/bar/assets/style.css as /foo/assets/style.css, depending on whether there is an index.html or not?

In my opinion, the only correct way to fix this is to return a 301 Redirect when you navigate to a directory without a trailing slash. I.e. something like the following (in pseudocode)

if isdir(url) && !endswith(url, "/")
    return response(301, url * "/")
else
    # current behavior
end
mortenpi commented 2 years ago

Actually, I think I just encountered a bug here where the correct file doesn't get served anymore after you access an index.html.. which sounds like maybe something going wrong with the way you keep track of the files?

$ curl -I http://localhost:8000/markdownast/reftest/build-html/assets/themes/documenter-dark.css
HTTP/1.1 200 OK
Content-Type: text/css; charset=utf-8

$ curl -I http://localhost:8000/markdownast/reftest/build-html/eval/index.html
HTTP/1.1 200 OK
Content-Type: text/html; charset=utf-8

$ curl -I http://localhost:8000/markdownast/reftest/build-html/assets/themes/documenter-dark.css
HTTP/1.1 404 Not Found
tlienart commented 2 years ago

we'll get there, thanks a lot @mortenpi for your feedback.

I've added the redirect scenario that you mentioned and tried to also add logic to fix a bunch of sneaky corner cases based on what's your root directory, what's the first page that one navigates to etc.

The current situation is that the example you mentioned in #135 and that you point to above (with the build without trailing slash) work; there's also a couple of other corner cases that should have been addressed (one thing that I try to do is to figure out what is the "parent web dir" correctly so that all relative paths are relative to that dir).

Given how this is going, I'll wait a bit more for feedback and testing (and thanks in advance for that!)

Edit: can you check whether the other error you mention is still present and if it is, can I kindly ask for a step-by-step so I can reproduce it locally? thanks 🙌

tlienart commented 2 years ago

Thanks @mortenpi ; I think I need to take a bit more time to think about this. One thing is that either out of my poor understanding of HTTP.jl or out of weird behaviour of HTTP.jl (or both), the req.target (input of the whole logic) is not always what I would intuitively expect. I'm now trying to go a bit more systematically over possible cases to document what should be the expected behaviour (compared to python's http.server which works pretty well).

Redirect responses are poorly documented in HTTP.jl, I had tried Response(301, redirect-body) and that didn't work, will try your suggestion with Location, the other one seems good too.

mortenpi commented 2 years ago

I had tried Response(301, redirect-body) and that didn't work, will try your suggestion with Location

Yeah, the Location headers are what give 30X redirects their meaning: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/301 and https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location