Skip to content

panic when use golang filter decode http2 connect tunnel data #32033

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

Closed
userlxd opened this issue Jan 25, 2024 · 9 comments · Fixed by #33377
Closed

panic when use golang filter decode http2 connect tunnel data #32033

userlxd opened this issue Jan 25, 2024 · 9 comments · Fixed by #33377
Assignees
Labels
area/golang bug investigate Potential bug that needs verification stale stalebot believes this issue/PR has not been touched recently

Comments

@userlxd
Copy link

userlxd commented Jan 25, 2024

  1. envoy config use h2 connect tunnel config from example

  2. envoy use golang filter envoy.filters.http.golang,code is

func (f *filter) DecodeData(buffer api.BufferInstance, endStream bool) api.StatusType {
	api.LogInfof("body len: %d", buffer.Len())
	api.LogInfof("body: %s", string(buffer.Bytes()))

	return api.Continue
}
  1. when request arrived, then buffer.Len() run success, but buffer.Bytes() will got panic, error is
2024-01-25 14:17:29.816][608832][error][golang] [contrib/golang/common/log/cgo.cc:24] http: panic serving: not proccessing Go
goroutine 17 [running, locked to thread]:
github.com/envoyproxy/envoy/contrib/golang/filters/http/source/go/pkg/http.(*httpRequest).RecoverPanic(0xc00007e690)
	/usr/local/go/path/pkg/mod/github.com/envoyproxy/[email protected]/contrib/golang/filters/http/source/go/pkg/http/filter.go:83 +0x6b
panic({0x7fa388a77840, 0xc000073820})
	/usr/local/go/src/runtime/panic.go:884 +0x213
github.com/envoyproxy/envoy/contrib/golang/filters/http/source/go/pkg/http.handleCApiStatus(...)
	/usr/local/go/path/pkg/mod/github.com/envoyproxy/[email protected]/contrib/golang/filters/http/source/go/pkg/http/capi_impl.go:82
github.com/envoyproxy/envoy/contrib/golang/filters/http/source/go/pkg/http.(*httpCApiImpl).HttpGetBuffer(0x7fa388ab3bc0?, 0xa388864d96?, 0x7fa387f27388?, 0x4f)
	/usr/local/go/path/pkg/mod/github.com/envoyproxy/[email protected]/contrib/golang/filters/http/source/go/pkg/http/capi_impl.go:208 +0xd5
github.com/envoyproxy/envoy/contrib/golang/filters/http/source/go/pkg/http.(*httpBuffer).Bytes(0xc000025fb0)
	/usr/local/go/path/pkg/mod/github.com/envoyproxy/[email protected]/contrib/golang/filters/http/source/go/pkg/http/type.go:368 +0x43
main.(*filter).DecodeData(0xc0000125d0?, {0x7fa388ae1478?, 0xc000025fb0?}, 0x8?)
	/code/sdp/internal/gateway/envoy/go-hook/main.go:62 +0x27
github.com/envoyproxy/envoy/contrib/golang/filters/http/source/go/pkg/http.envoyGoFilterOnHttpData(0x277cbf2469b0, 0x0, 0x277cbfd50300, 0x4f)
	/usr/local/go/path/pkg/mod/github.com/envoyproxy/[email protected]/contrib/golang/filters/http/source/go/pkg/http/shim.go:197 +0x122
@userlxd userlxd added bug triage Issue requires triage labels Jan 25, 2024
@zuercher zuercher added investigate Potential bug that needs verification area/golang and removed triage Issue requires triage labels Jan 25, 2024
@zuercher
Copy link
Member

cc @doujiang24 @wangfakang

@doujiang24
Copy link
Member

@userlxd Thanks for your reporting, it's reproducable on my side, I'll take a deeper look soon~

@doujiang24
Copy link
Member

Oh, sorry, the current implementation do not support handling request data of full-duplex http requests, neither CONNECT nor WebSocket.
I'll figure a way to support it, may need some time. Thanks again~

@userlxd
Copy link
Author

userlxd commented Jan 28, 2024

In my case, I need inject the tls.conn over tunnel to control https token header.
Wasm go plugin could be work when read tunnel data, but go native tls.conn.handshake use goroutine.
So the lowest cost way is to wait for go plugin to support it, Thank you for your contribution @doujiang24

Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Feb 27, 2024
@doujiang24
Copy link
Member

not stale, I'll fix it in the near future.

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Feb 28, 2024
@doujiang24
Copy link
Member

/assign @doujiang24

Copy link

github-actions bot commented Apr 2, 2024

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Apr 2, 2024
Copy link

github-actions bot commented Apr 9, 2024

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/golang bug investigate Potential bug that needs verification stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants