frp icon indicating copy to clipboard operation
frp copied to clipboard

[Feature Request] Support h2c in vhost server

Open sword-jin opened this issue 1 year ago • 8 comments

Describe the feature request

Like the title, we can enable h2c support in vhost server, there is a PR already

https://github.com/fatedier/frp/pull/4561

Describe alternatives you've considered

No response

Affected area

  • [ ] Docs
  • [ ] Installation
  • [ ] Performance and Scalability
  • [ ] Security
  • [ ] User Experience
  • [ ] Test and Release
  • [ ] Developer Infrastructure
  • [ ] Client Plugin
  • [ ] Server Plugin
  • [ ] Extensions
  • [ ] Others

sword-jin avatar Nov 28 '24 04:11 sword-jin

@fatedier I think it's safe to wrapper the handler with h2c if we don't want to introduce new optional parameter

sword-jin avatar Nov 28 '24 04:11 sword-jin

Could you provide relevant documentation or examples from other projects? Also, please add an end-to-end (e2e) test case to ensure existing functionality is not affected.

fatedier avatar Nov 28 '24 04:11 fatedier

I see, I will write a e2e test case, I think we can see the real user case in the e2e case

sword-jin avatar Nov 28 '24 04:11 sword-jin

I can't reprocude the bug when running all the components locally, this is my playground https://github.com/sword-jin/caddy-frp-grpc-streaming/blob/be0c28f7a1a0972a6103b8434d43f73ea51f9939/cmd/client/main.go#L46

sequenceDiagram
    autonumber
    actor B as Browser
    box Purple On Cloud
    participant C as Caddy
    participant FS as Frps
    end
    box Grey On Local
    participant FC as Frpc
    participant LS as LocalServer
    end
    
    B ->> C: request 443
    C ->> C: tls termination
    C ->> FS: reverse proxy to 8888
    FS ->>+ FC: notify the client new connection
    FC ->>- FS: new tcp connection
    FS ->> FC: forward request
    FC ->> LocalServer: forward request

This diagram Illuminates how a request reach out the local server. This repo trys to build the same worfklow, but fortunately it doesn't work.

In the playground, we use a mock certificate, and the client skip the validation

When the frps and caddy on real cloud with a real tls certificate, the client always gets stream error: internal: protocol error: no Grpc-Status trailer: unexpected EOF at https://github.com/sword-jin/caddy-frp-grpc-streaming/blob/be0c28f7a1a0972a6103b8434d43f73ea51f9939/cmd/client/main.go#L46-L48

Form the logs of caddy and frps and local server, the request has been handled successfully, the problem is the trailer header is missing.

What I did to fix is add a h2c layer in frp vhost server and use http2 transport

I check the frps's response, left is without h2c, right is with h2c, the difference is left doesn't have Trailer:Grpc-Status which cause the error

image

sword-jin avatar Nov 29 '24 02:11 sword-jin

@fatedier I think it's safe to wrapper the handler with h2c if we don't want to introduce new optional parameter

Can the code be modified in this way? And verify that it does not affect other requests?

fatedier avatar Dec 05 '24 03:12 fatedier

"golang.org/x/net/http2/h2c" is completely compatible to the existing http1.1, we can verify it doesn't affect all the current behavior(by current e2e test

but for http2.transparent, we need another option flag

sword-jin avatar Dec 13 '24 01:12 sword-jin

A quick look shows that server-side support for h2c seems compatible and can be directly added. However, proxying to the backend using http2.Transport is incompatible. Perhaps we could split the work and submit the former first.

For proxying to the backend, there are two main approaches to consider:

Achieving compatibility with both, possibly through an upgrade mechanism or a fallback mechanism. Allowing the backend service to configure its own interaction protocol. I prefer the second approach, but the current code has limited support in this regard. There has always been a plan for future refactoring to provide more fine-grained configuration support.

fatedier avatar Dec 13 '24 04:12 fatedier

I agree, https://github.com/fatedier/frp/pull/4582 adding h2c wapper is compatible, for http2.Transport, I'd like to take a deep look or you can support the second approach if you are free.

sword-jin avatar Dec 13 '24 04:12 sword-jin

Issues go stale after 14d of inactivity. Stale issues rot after an additional 3d of inactivity and eventually close.

github-actions[bot] avatar Dec 28 '24 00:12 github-actions[bot]