search

cloudflare / cloudflared

Cloudflare Tunnel client (formerly Argo Tunnel)
https://developers.cloudflare.com/cloudflare-one/connections/connect-apps/install-and-setup/tunnel-guide
Apache License 2.0
9.14k stars 804 forks source link

🐛 2024.10.0 appears to have broken xterm.js (e.g. Proxmox WebUI) when served over CF tunnel #1337

Open itsthejb opened 1 week ago

itsthejb commented 1 week ago

Describe the bug

Prior to 2024.10.0 it was possible to serve Proxmox web UI over a CF tunnel. However since 2024.10.0 the default xterm.js CT/VM web console is failing with Connection failed (Error 501: ). The noVNC implementation continues to work without issues.

To Reproduce Steps to reproduce the behavior:

Expected behavior

Environment and versions

Logs and errors

Additional context

OdoMarTus commented 1 week ago

I confirm. I have the same problem. However, it appeared for me about two or three weeks ago. There are also reports that the problem also appeared at the turn of May and June: https://forum.proxmox.com/threads/error-connection-error-501-not-implemented.11767/post-95568

On my OpenWRT router I have Cloudflare version: 2024.4.1-2 and it causes the above problem The agent on Linux version 2024.9.1 is correct, but in version: 2024.10.0 is not correct.

itsthejb commented 1 week ago

Confirmed that, at least in my case, the issue only began after 2024.9.1. I reverted to a Proxmox CT backup that had cloudflared@2024.9.1 and I'm able to work around the issue introduced in 2024.10.0. This forces me to hold the package, however...

q0Jop commented 1 week ago

I can also confirm. I did not run into this issue until upgrading to 2024.10.0 recently.

kmendell commented 1 week ago

I confirm this issue as well.

maggie44 commented 1 week ago

I think it is probably this commit: https://github.com/cloudflare/cloudflared/commit/e2064c820f32802f58baa027903043fe0ed052e0#diff-6ff9dc179d3cf0539f514218792ce1d93001e2b4835a695bde442ad45edf01deR516 @chungthuang

It looks like it used to be that if it is not a websocket (false), then req.Body is left as it is (not set to http.NoBody).

Now it is checking if RequestBodyHintEmpty.String() exists and if it does returns true, which sets req.Body to http.NoBody, which could be Websocket incompatible. Websockets would have empty bodies, so if the metadata is being applied, then this would now be configuring a NoBody, when before it didn't (I assume websockets were excluded from the logic for a reason).

I suspect this has wider implications on websockets more broadly beyond Proxmox.

I think it would be worth adding a websocket to the tests too. At the moment the tests in that commit only covers HTTP so things like this could get missed in the current test coverage.

maggie44 commented 1 week ago

Could someone try this PR out to confirm if it fixes the issue?

https://github.com/cloudflare/cloudflared/pull/1340

Clone it, run make (assuming you have Golang and make installed) and it will build a cloudflared binary. Run that built binary instead of the original and see if it works.

itsthejb commented 1 week ago

@maggie44 I'll try this out later

itsthejb commented 1 week ago

I bisected the issue (https://github.com/cloudflare/cloudflared/pull/1340#issuecomment-2422660954) and found the breaking commit to be e2064c820f32802f58baa027903043fe0ed052e0

ignSKRRRTT commented 6 days ago

ISSUE #1337 🔥 🤑

itsthejb commented 6 days ago

Haha yes I also noticed this 🤣

Note there is now a fix for this ready to land hopefully soon

GoncaloGarcia commented 3 days ago

Hey everyone! Thank you for letting us know of this problem and for the effort in identifying the cause of the issue. We decided to fully revert the change and will take some time to evaluate a better approach. Please let us know if you still see this issue after upgrading to 2024.10.1.

itsthejb commented 3 days ago

Can confirm things are working as expected in 2024.10.1

itsthejb commented 1 day ago

@GoncaloGarcia In spite of reverting the change, installing the new version it really feels like the interface is less reliable than previous. While the console loads, it frequently disconnects and I have to refresh the page. Do you think this could still be related to other changes in 2024.10.0?

Reopening the issue for the time being for visibility