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

fix: Use guessAppRootOr to enable relative root if desired. #2100

Closed
wants to merge 2 commits into from
Closed

fix: Use guessAppRootOr to enable relative root if desired. #2100

wants to merge 2 commits into from

Conversation

klockeph
Copy link
Contributor

This is useful when serving on 0.0.0.0, such that querying from any other device with : does not fall back to 0.0.0.0:PORT, which fails. (#2099)

Tested: Manually

This is useful when serving on 0.0.0.0, such that querying from any
other device with <IP>:<PORT> does not fallback to 0.0.0.0:PORT,
which would fail.

Tested: Manually
@simonmichael
Copy link
Owner

Cool. What's a description of the user-visible difference this makes, suitable for the manual ?

On mac, with or without your fix, running hledger-web with --host 0.0.0.0 or --host localhost seems to break it for web browsers - Safari shows an empty page and Firefox refuses to browse it. (Also one has to use --port to avoid AirTunes which is using port 5000.)

@klockeph
Copy link
Contributor Author

You mean serving from Mac?
Unfortunately, I cannot test that sorry.

The user-visible change would be that if the web interface is reachable from multiple domains or IPs, they will not necessarily be redirected to the base_url; instead hledger-web will try to keep them at the root/host from the URL of the request.
Apart from this it fixes the bug where 0.0.0.0 would only work for access from localhost.

If this is not desired behavior we could maybe check if base_url is set and if it is, keep ApprootMaster?
I just wasn't sure how the arguments are passed and if base_url will always contain "IP:PORT" if unset.

@simonmichael
Copy link
Owner

simonmichael commented Oct 16, 2023 via email

@klockeph
Copy link
Contributor Author

Ah I see - when hledger-web is accessible at multiple IPs, it will keep responding from the same IP address you visited ?

Yes exactly that. Plus it would also keep the same hostname, imagine you have the host as "foo" and "192.168.1.123", then visiting http://192.168.1.123:5000 would redirect to http://192.168.1.123:5000/journal and visiting http://foo:5000 would redirect to http://foo:5000/journal.

I guess you have observed it fixing your issue on GNU/linux.

Actually, I tested the change on Windows so far. My Linux Haskell setup is super broken at the moment... :/

I don't know if this interacts with the other options

The only option it could interact with is base_url if I understand correctly.

@klockeph
Copy link
Contributor Author

Tested on Linux as well now; also works.

@simonmichael
Copy link
Owner

I think we need to clarify how this interacts with --base-url. The doc for that option says

--base-url=URL : set the base url (default: http://IPADDR:PORT). Note: affects url generation but not route parsing. Can be useful if running behind a reverse web proxy that does path rewriting.

Note to self: translate this to userese.

@klockeph
Copy link
Contributor Author

We could just let base-url overwrite the guessAppRootOr. So if base-url is given, don't use the guess. Otherwise use guess and fall back to IP:PORT.
However I'm not proficient in Haskell, so if you want to go for this I'd be happy if someone else picks up the patch.

@simonmichael simonmichael added web The hledger-web tool. needs:testing To unblock: needs more developer testing or general usage needs:docs To unblock: needs corresponding documentation or doc updates and removed needs:testing To unblock: needs more developer testing or general usage needs:docs To unblock: needs corresponding documentation or doc updates labels Oct 18, 2023
@simonmichael
Copy link
Owner

I can't fully test this on mac, but I see that we print both the listening address and port and the base url and port, so it should be easy enough to troubleshoot any issues. I will shortly merge this as 6312446, thank you !

simonmichael added a commit that referenced this pull request Jul 18, 2024
A followup to #2099, #2100 and #2127. Now relative links to js/css
resources will use the same hostname etc. the main page was requested
from, making them work better when accessed via multiple IP
addresses/hostnames without an explicit --base-url setting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
web The hledger-web tool.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants