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

Incorrectly parsing gRPC-Web requests to native gRPC #513

Open
davihsg opened this issue Jan 16, 2025 · 1 comment
Open

Incorrectly parsing gRPC-Web requests to native gRPC #513

davihsg opened this issue Jan 16, 2025 · 1 comment
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@davihsg
Copy link

davihsg commented Jan 16, 2025

Describe the bug

The GrpcWeb module does not properly convert from application/grpc-web-text to application/grpc, which results in the error: protocol error: received message with invalid compression flag: 65 (valid flags are 0 and 1), while sending request.

After attempting to set up the GrpcWebModule following the reference example, I was unable to forward requests from gRPC-Web to native gRPC. After some investigation, I discovered that the module doesn't upgrade the connection from HTTP/1 to HTTP/2 properly. Specifically, it doesn't change the content type as expected: application/grpc-web-text was converted to application/grpc-text. The issue arises from this line.

I assume that the error with the compression flag is related to the content-type parsing, as mentioned in this comment.

Pingora info

Pingora version: 0.4.0
Rust version: cargo 1.83.0 (5ffbef321 2024-10-29)
Operating system version: macOS Sonoma 14.7.1

Steps to reproduce

I created a repository with all the artifacts necessary to reproduce the error.

First, I have a simple Hello gRPC server written in Rust with tokio 0.10.2. It runs on port 50051.

Then, I have the Pingora proxy. It also runs a simple CORS service to handle preflight requests:

impl ServeHttp for GrpcWebPreflightHttpApp {
    async fn response(&self, _http_session: &mut ServerSession) -> Response<Vec<u8>> {
        let buffer = "Grpc Web OK".as_bytes().to_vec();

        Response::builder()
            .status(200)
            .header(
                http::header::ACCESS_CONTROL_ALLOW_HEADERS,
                "content-type,x-grpc-web,x-user-agent",
            )
            .header(http::header::ACCESS_CONTROL_ALLOW_METHODS, "POST")
            .header(http::header::CONTENT_LENGTH, buffer.len())
            .header(http::header::ACCESS_CONTROL_ALLOW_ORIGIN, "null")
            .header(http::header::ACCESS_CONTROL_EXPOSE_HEADERS, "*")
            .header(http::header::ACCESS_CONTROL_MAX_AGE, 1728000)
            .body(buffer)
            .unwrap()
    }
}

The proxy configuration is as follows. The GrpcWeb module is added in init_downstream_modules and initialized for every request in early_request_filter, as shown in the reference example. For the upstream_peer, it returns the GrpcWebPreflight peer whenever the method is OPTIONS, or the Hello server peer otherwise. The ALPN version is set to H2 for the Hello server peer.

async fn upstream_peer(
        &self,
        session: &mut Session,
        _ctx: &mut Self::CTX,
    ) -> Result<Box<HttpPeer>, Box<Error>> {
        let method = &session.req_header().method;

        if method == "OPTIONS" {
            return Ok(create_grpc_web_preflight_peer());
        }

        // gRPC server
        let mut peer = Box::new(HttpPeer::new(
            String::from("0.0.0.0:50051"),
            false,
            String::from(""),
        ));

        peer.options.alpn = ALPN::H2;

        Ok(peer)
    }

The proxy runs at 0.0.0.0:6193.

gRPC requests can be sent with grpcurl:

ADDRESS="localhost:6193"

grpcurl \
  -plaintext \
  -d '{}' \
  -import-path ../../protos \
  -proto ../../protos/helloworld.proto \
  -vv \
  $ADDRESS \
  helloworld.Greeter/SayHello

For gRPC-Web, there is a simple JavaScript client that sends a Hello request. You can open index.html in the browser and provide the proxy URL.

Image

Expected results

Native gRPC requests should be ignored by the module and forwarded normally.

gRPC-Web requests with content type application/grpc-web-text should be parsed as application/grpc before being sent to the upstream peer, and the response should be parsed back to application/grpc-web+{proto | json | ... }.

Observed results

Native gRPC requests are being ignored by the module and forwarded normally, as expected.

gRPC-Web requests with content type application/grpc-web-text are being incorrectly parsed as application/grpc-text, which results in the error protocol error: received message with invalid compression flag: 65 (valid flags are 0 and 1), while sending request.

Additional context

grpc-web should be treated differently from grpc-web-text. As outlined in the gRPC documentation, grpc-web-text is text-encoded and should be parsed before being converted to native gRPC.

This is similar to how Envoy handles it here with its gRPC-Web filter.

@andrewhavck andrewhavck added the enhancement New feature or request label Jan 16, 2025
@gumpt gumpt added the bug Something isn't working label Jan 17, 2025
@andrewhavck
Copy link
Contributor

Simple solution for now to not break these payloads is to check for grpc-web-text here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants