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

1.32: hledger-web --base-url has no effect #2127

Closed
rajeevn1 opened this issue Dec 7, 2023 · 24 comments
Closed

1.32: hledger-web --base-url has no effect #2127

rajeevn1 opened this issue Dec 7, 2023 · 24 comments
Labels
A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. bounty Thar's some kind o' loot on offer.. impact3 Affects just a few users. regression A backwards step, indicating a weakness in our QA. We don't like these. severity4 Major usability/doc bug, crash, or any regression. web The hledger-web tool.

Comments

@rajeevn1
Copy link

rajeevn1 commented Dec 7, 2023

the option works in 1.31

@rajeevn1 rajeevn1 added the A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. label Dec 7, 2023
@simonmichael simonmichael added the web The hledger-web tool. label Dec 7, 2023
@simonmichael
Copy link
Owner

Thanks for the report. What's a good way to demonstrate the bug ?

@klockeph if you have some time maybe you could take a look and see if this is fallout from #2100 ?

@rajeevn1
Copy link
Author

rajeevn1 commented Dec 7, 2023

When I run hledger-web with the --socket option it still listens on port 5000 instead of the socket.

@simonmichael simonmichael added regression A backwards step, indicating a weakness in our QA. We don't like these. needs:debugging To unblock: needs debugging/investigation severity4 Major usability/doc bug, crash, or any regression. impact3 Affects just a few users. labels Dec 13, 2023
@simonmichael
Copy link
Owner

simonmichael commented Dec 15, 2023

@rajeevn1 I can't reproduce here on a mac; are you on GNU/linux ?

$ hledger-web-1.32.1 -f examples/sample.journal --socket a.sock
...
$ curl --unix-socket a.sock http://localhost/version
"1.32.1"
$ curl -i http://localhost:5000/version
HTTP/1.1 403 Forbidden
Content-Length: 0
Server: AirTunes/745.13.4

@simonmichael simonmichael added needs:repro To unblock: needs a small reproducible example and removed needs:debugging To unblock: needs debugging/investigation labels Dec 15, 2023
@rajeevn1
Copy link
Author

Yes I am on debian stable

@simonmichael
Copy link
Owner

Hmm, ubuntu should be similar but I still can't reproduce there:

# bin/hledger-web-1.32.1 --socket sock -f examples/sample.journal
14/Dec/2023:17:00:26 -1000 [Info#yesod-core] Application launched @(yesod-core-1.6.25.1-2476c48f8d67ca7e46d176d339df66dc378a0de5bfbaae5a5cb02098f201a4f1:Yesod.Core.Dispatch src/\
Yesod/Core/Dispatch.hs:188:10)
Serving web UI and API on 127.0.0.1:5000 with base url http://127.0.0.1:5000
Press ctrl-c to quit
# curl -i http://127.0.0.1:5000
curl: (7) Failed to connect to 127.0.0.1 port 5000 after 0 ms: Connection refused

@simonmichael
Copy link
Owner

# lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 22.10
Release:        22.10
Codename:       kinetic

@simonmichael simonmichael removed the needs:repro To unblock: needs a small reproducible example label Dec 15, 2023
@rajeevn1
Copy link
Author

I am trying to run it behind apache reverse proxy, which works for 1.31 but not for 1.32.

ProxyPass /hledger/ unix://hledger.sock|http://127.0.0.1:5000/
ProxyPassReverse /hledger/ unix://hledger.sock|http://127.0.0.1:5000/

I will check if I can find more info.

@simonmichael
Copy link
Owner

Could you also test with curl, just to simplify ? See if you can produce a transcript like mine above, with 1.31 and 1.32.1.

@rajeevn1
Copy link
Author

I tried that already, curl works.

@simonmichael
Copy link
Owner

simonmichael commented Dec 15, 2023

For clarity, could you show transcripts ? When you have time. I don't know what works means.

@rajeevn1
Copy link
Author

curl -q -v --unix-socket sock http://127.0.0.1:5000/version
*   Trying sock:0...
* Connected to 127.0.0.1 (sock) port 5000 (#0)
> GET /version HTTP/1.1
> Host: 127.0.0.1:5000
> User-Agent: curl/7.88.1
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Fri, 15 Dec 2023 14:19:22 GMT
< Server: Warp/3.3.31
< Content-Length: 8
< Content-Type: application/json; charset=utf-8
< Vary: Accept-Encoding, Accept, Accept-Language
< 
* Connection #0 to host 127.0.0.1 left intact
"1.32.1"

@simonmichael
Copy link
Owner

I was hoping for full transcripts like mine, but never mind. Make sure you don't have an old instance still running on port 5000.

@simonmichael
Copy link
Owner

Ah.. @rajeevn1 your command above is using the socket file and working as expected. Please show curl without --unix-socket also.

@simonmichael
Copy link
Owner

As in #2127 (comment)

@rajeevn1
Copy link
Author

I think the problem with 1.32 is that it is not handling --base-url=BASEURL correctly, and still redirecting to http://127.0.0.1:5000. I will check more when I have some time.

@simonmichael simonmichael changed the title hledger-web --socket option does not work in 1.32 hledger-web --socket/base url problem in 1.32 Dec 15, 2023
@simonmichael
Copy link
Owner

While testing I did notice that hyperlinks didn't seem to be respecting --base-url. And you're right, that seems to be what has broken in 1.32. Thanks !

@simonmichael simonmichael changed the title hledger-web --socket/base url problem in 1.32 hledger-web --base-url has no effect in 1.32 Dec 15, 2023
@simonmichael
Copy link
Owner

@klockeph Hi, this is indeed fallout from your #2100. Adding guessApprootOr breaks --base-url when people are using that (I guess you are not ?). How could we satisfy both cases ?

@simonmichael
Copy link
Owner

I guess use guessApprootOr only when --base-url option is not used ?

@klockeph
Copy link
Contributor

Oh nooo.... Sorry for that! I'm indeed not using this option.

I think what I described in #2100 (comment) could be a viable alternative.

Edit: you were faster than me. yes using guessAppRoot only when --base-url option is not used is probably the way to go :)

@simonmichael
Copy link
Owner

Unfortunately it seems to be a compile-time configuration: https://hackage.haskell.org/package/yesod-core-1.6.25.1/docs/Yesod-Core.html#v:approot

@klockeph
Copy link
Contributor

ooof. In that case, should we rather find out why guessApproot guesses a wrong approot for the specific request?

@simonmichael
Copy link
Owner

My test was like:

$ hledger-web -f/dev/null --serve --base-url http://BASE

Good:

$ curl -s http://localhost:5000/journal | rg '(href)="\w.*?"' -o
href="http://BASE/static/css/bootstrap.min.css"
href="http://BASE/static/css/bootstrap-datepicker.standalone.min.css"
href="http://BASE/static/hledger.css"
href="http://BASE/journal"

Bad (with guessAppRootOr):

$ curl -s http://localhost:5000/journal | rg '(href)="\w.*?"' -o
href="http://localhost:5000/static/css/bootstrap.min.css"
href="http://localhost:5000/static/css/bootstrap-datepicker.standalone.min.css"
href="http://localhost:5000/static/hledger.css"
href="http://localhost:5000/journal"

I didn't think that guessApproot could be expected to guess right in all cases, maybe it could but doesn't here.

I tried hard but haven't been able to implement both cases today, which means I must revert #2100 unfortunately.

@simonmichael
Copy link
Owner

This is fixed in master, and a bugfix release will follow at some point. @rajeevn1 thanks for the report, claim your https://hledger.org/REGRESSIONS.html#regression-bounty when convenient.

@simonmichael simonmichael changed the title hledger-web --base-url has no effect in 1.32 1.32: hledger-web --base-url has no effect Dec 16, 2023
@rajeevn1
Copy link
Author

submitted invoice for regression bounty, thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. bounty Thar's some kind o' loot on offer.. impact3 Affects just a few users. regression A backwards step, indicating a weakness in our QA. We don't like these. severity4 Major usability/doc bug, crash, or any regression. web The hledger-web tool.
Projects
None yet
Development

No branches or pull requests

3 participants