Restore ability to relay POST requests with empty bodies #1340

maggie44 · 2024-10-18T08:13:00Z

It appears that the commit in the latest release (e2064c8) was designed to check for specific request methods: "GET", "HEAD", "DELETE", "OPTIONS", "PROPFIND", "SEARCH". If the request matched one of these methods, the request body would be set to empty. If it didn’t match, the body wouldn’t be modified. However, it seems the commit didn’t account for requests like POST, which can also have an empty body. Since POST wasn’t included in the predefined list, the request body wasn’t being set to empty when necessary.

Closes: #1337

itsthejb · 2024-10-18T10:40:33Z

You'll have to excuse my lack of experience with go here. Installed build-essential and golang on Debian Bookworm, make, but then:

GOOS=linux GOARCH=amd64  go build -mod=vendor  -ldflags='-X "main.Version=2022.12.1-281-g7a74b15f" -X "main.BuildTime=2024-10-18-1039 UTC" ' github.com/cloudflare/cloudflared/cmd/cloudflared
vendor/go.opentelemetry.io/otel/attribute/set.go:7:2: cannot find package "." in:
        /root/cloudflared-docker-multiarch/vendor/cmp
vendor/github.com/quic-go/quic-go/internal/wire/transport_parameters.go:10:2: cannot find package "." in:
        /root/cloudflared-docker-multiarch/vendor/slices
make: *** [Makefile:134: cloudflared] Error 1

Looks like I need to checkout out vendor dependencies first?

maggie44 · 2024-10-18T12:05:48Z

Try running:

go mod vendor

itsthejb · 2024-10-18T12:09:14Z

Unfortunately still same prob. Here's go env:

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.19"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.19/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.8"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/root/cloudflared/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build4223151333=/tmp/go-build -gno-record-gcc-switches"

maggie44 · 2024-10-18T12:12:48Z

And that using the latest golang. If golang was installed with apt-get it’s probably out of date.

itsthejb · 2024-10-18T12:17:32Z

Ah yes - makes sense. apt version is 1.19, so somewhat old. Appears to be building no probs now. Thanks! Will report back shortly

itsthejb · 2024-10-18T14:10:30Z

Hi @maggie44,

Finally got it built and testing. However, as far as I can see this doesn't fix the issue, I'm afraid.

maggie44 · 2024-10-18T14:18:01Z

There isn't much that changed in the last release by the looks of it: 2024.9.1...2024.10.0

Are you using quick tunnels or connecting with a config file?

maggie44 · 2024-10-18T14:21:31Z

Worth double checking you are on the right branch too. If you cloned the fork of this called cloudflared-docker-multiarch, then selected the branch websocket rather than building directly off the main branch of cloudflared-docker-multiarch.

itsthejb · 2024-10-18T14:23:11Z

Yes, it's from the websocket branch:

root@cloudflared:~/cloudflared# git status
On branch websocket
Your branch is up to date with 'origin/websocket'.

nothing to commit, working tree clean

itsthejb · 2024-10-18T14:24:58Z

Yes, it is a quic tunnel. However this works fine with 2024.9.1:

Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF Starting tunnel tunnelID=d05945fa-7b50-4d35-aece-c2ddd54d4d2e
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF Version 2024.9.1
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF GOOS: linux, GOVersion: go1.22.2, GoArch: amd64
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF Settings: map[config:/etc/cloudflared/config.yml cred-file:/etc/cloudflared/d05945fa-7b5>
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF Generated Connector ID: 4f9c5ae2-1fbd-493a-8345-986fe57bdf2b
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF cloudflared will not automatically update if installed by a package manager.
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z WRN No ingress rules were defined in provided config (if any) nor from the cli, cloudflared >
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF Initial protocol quic
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF ICMP proxy will use 10.0.0.100 as source for IPv4
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF ICMP proxy will use :: as source for IPv6
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF Starting metrics server on 127.0.0.1:38677/metrics
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z INF You requested 4 HA connections but I can give you at most 2.
Oct 15 13:26:13 cloudflared cloudflared[158]: 2024-10-15T12:26:13Z WRN Your version 2024.9.1 is outdated. We recommend upgrading it to 2024.10.0

maggie44 · 2024-10-18T14:30:16Z

From your first build attempt it logged "main.Version=2022.12.1-281-g7a74b15f". Couldn't understand why the date was off, but the commit seemed right, which made me wonder whether it was the right branch.

And the output of version should show something like

./cloudflared version
cloudflared version 2024.10.0-5-g7a74b15f (built 2024-10-18-1429 UTC)`.

itsthejb · 2024-10-18T14:30:52Z

Since I'm in this context, I'm going to try bisecting to find the origin commit. This will take a little while

maggie44 · 2024-10-18T14:35:21Z

Since I'm in this context, I'm going to try bisecting to find the origin commit. This will take a little while

Sounds like a good idea. You could undo each commit one at a time and test it. There aren't many commits between the last working version.

You can skip all the tests and just do the build with make cloudflared. Will speed things up a lot.

itsthejb · 2024-10-18T14:52:14Z

Alright, completed the bisect and can confirm that the breaking commit is e2064c8

maggie44 · 2024-10-18T14:59:08Z

Probably narrows it to this then:

e2064c8#diff-6ff9dc179d3cf0539f514218792ce1d93001e2b4835a695bde442ad45edf01deR562-R579

requestMethodUsuallyLacksBody looks new.

And this part using metadata:

	switch metadata[HTTPRequestBodyHintKey] {
	case RequestBodyHintEmpty.String():
		return true
	case RequestBodyHintHasData.String():
		return false
	default:
	}

itsthejb · 2024-10-18T15:04:56Z

Just double checked that simply reverting e2064c8 fixes the issue, but your patch does not. So, yes, looks like you're very much on the right track but not quite there.

I'll be available to try other patches. Just let me know

maggie44 · 2024-10-18T15:16:00Z

The commit just added to this PR should give you your answer. Run it again and see what it prints to console.

itsthejb · 2024-10-18T15:23:02Z

Have done now. Currently have quite a lot running through the tunnel when it's up, so the output is quite busy:

Oct 18 16:21:02 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: old behavior is to reply:  false
Oct 18 16:21:02 cloudflared cloudflared[2738]: request method is  false
Oct 18 16:21:02 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:02 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: request method is  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: request method is  false

I believe old behavior is to reply: false is when testing the Proxmox UI, but it's not easy to check. Is that enough to go on? I could spin up another tunnel to test more carefully, but that would be a little annoying to do

maggie44 · 2024-10-18T15:33:45Z

Oct 18 16:21:03 cloudflared cloudflared[2738]: old behavior is to reply:  true
Oct 18 16:21:03 cloudflared cloudflared[2738]: request method is  false

Assuming it's not working still, as nothing changed other than the logging.

Looks like Proxmox is passing through a body in a type of request that doesn't usually have a body. Although web standards advise against it, it's not forbidden, so should probably be supported.

You can try refreshing this branch and trying again. It should work. And it should also print to console to confirm the method and body length.

itsthejb · 2024-10-18T15:40:21Z

Just built and checked: yes, it does indeed work 👍

Otherwise, I suppose this is really an xterm.js issue, since that's actually the implementation of the console. I suppose I could make an issue over there, although you may be better placed to explain what they're doing that might be a little non-standard?

maggie44 · 2024-10-18T15:41:31Z

Just built and checked: yes, it does indeed work 👍

Otherwise, I suppose this is really an xterm.js issue, since that's actually the implementation of the console. I suppose I could make an issue over there, although you may be better placed to explain what they're doing that might be a little non-standard?

Anything in the console? 'length of body:' or 'method'?

itsthejb · 2024-10-18T15:50:39Z

So the issue is a GET with a body? Yes that is weird

Oct 18 16:50:25 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:25 cloudflared cloudflared[2836]: method:  HEAD
Oct 18 16:50:25 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:25 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:25 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:25 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:25 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:25 cloudflared cloudflared[2836]: method:  HEAD
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  450
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  POST
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  GET
Oct 18 16:50:27 cloudflared cloudflared[2836]: length of body:  0
Oct 18 16:50:27 cloudflared cloudflared[2836]: method:  POST

itsthejb · 2024-10-18T16:07:16Z

Still available to test anything for the time being. Or otherwise if you don't need any more, I'll restore from my backup and await the fixed version

maggie44 · 2024-10-18T16:24:37Z

It's not actually the outcome I expected. It seems the requests are being sent as normal, but empty POST requests are what changed.

You could try again with the new commit here.

itsthejb · 2024-10-18T16:31:13Z

Just tested 49bb66b and it also works

maggie44 · 2024-10-18T16:37:46Z

Since we were using Xterm, we ended up exploring WebSocket issues, which turned out to be unrelated.

It appears that the commit in the latest release was designed to check for specific request methods: "GET", "HEAD", "DELETE", "OPTIONS", "PROPFIND", "SEARCH". If the request matched one of these methods, the request body would be set to empty. If it didn’t match, the body wouldn’t be modified. However, it seems the commit didn’t account for requests like POST, which can also have an empty body. Since POST wasn’t included in the predefined list, the request body wasn’t being set to empty when necessary.

Great job with the debugging!

itsthejb · 2024-10-18T16:48:35Z

Great stuff! And thanks for taking this @maggie44

maggie44 · 2024-12-03T17:43:38Z

Fixed for now by reverting the original commit: f407dbb

maggie44 marked this pull request as draft October 18, 2024 08:13

maggie44 mentioned this pull request Oct 18, 2024

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

Closed

maggie44 force-pushed the websocket branch 2 times, most recently from 08484d5 to 5dd5283 Compare October 18, 2024 15:17

maggie44 force-pushed the websocket branch from 5dd5283 to 5d54612 Compare October 18, 2024 15:32

maggie44 force-pushed the websocket branch 2 times, most recently from 4ca5ad9 to 7a74b15 Compare October 18, 2024 16:06

Restore ability to relay POST requests with empty bodies

49bb66b

maggie44 force-pushed the websocket branch from 7a74b15 to 49bb66b Compare October 18, 2024 16:22

maggie44 changed the title ~~Fix websocket handling when using metadata from edge~~ Restore ability to relay POST requests with empty bodies Oct 18, 2024

maggie44 marked this pull request as ready for review October 18, 2024 16:39

Remove test for requestMethodUsuallyLacksBody

fbab858

maggie44 closed this Dec 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restore ability to relay POST requests with empty bodies #1340

Restore ability to relay POST requests with empty bodies #1340

maggie44 commented Oct 18, 2024 •

edited

Loading

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024 •

edited

Loading

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

itsthejb commented Oct 18, 2024 •

edited

Loading

maggie44 commented Oct 18, 2024 •

edited

Loading

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024 •

edited

Loading

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024 •

edited

Loading

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Dec 3, 2024

Restore ability to relay POST requests with empty bodies #1340

Restore ability to relay POST requests with empty bodies #1340

Conversation

maggie44 commented Oct 18, 2024 • edited Loading

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024 • edited Loading

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

itsthejb commented Oct 18, 2024 • edited Loading

maggie44 commented Oct 18, 2024 • edited Loading

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024 • edited Loading

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024 • edited Loading

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Oct 18, 2024

itsthejb commented Oct 18, 2024

maggie44 commented Dec 3, 2024

maggie44 commented Oct 18, 2024 •

edited

Loading

maggie44 commented Oct 18, 2024 •

edited

Loading

itsthejb commented Oct 18, 2024 •

edited

Loading

maggie44 commented Oct 18, 2024 •

edited

Loading

maggie44 commented Oct 18, 2024 •

edited

Loading

itsthejb commented Oct 18, 2024 •

edited

Loading