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

Built-in support of grpc-web clients #4823

Open
lbensaad opened this issue Sep 5, 2018 · 11 comments
Open

Built-in support of grpc-web clients #4823

lbensaad opened this issue Sep 5, 2018 · 11 comments
Milestone

Comments

@lbensaad
Copy link

lbensaad commented Sep 5, 2018

Supporting grpc-web clients is possible only through a reverse proxy like Envoy which might reduce the performance. Is it possible to add direct support as this wrapper for grpc-go?

@hsaliak
Copy link
Contributor

hsaliak commented Sep 6, 2018

@lbensaad is performance your only concern for this request? Have you run into any such issues?
Do you have other reasons why you would like this natively?

@lbensaad
Copy link
Author

lbensaad commented Sep 6, 2018

@hsaliak, performance is the main reason, another reason is management, a standalone gateway or proxy is an extra headache to me because i need to install it, manage it and maintain it.

@dapengzhang0
Copy link
Member

A side note, the in progress project running gprc server as a servlet may support grpc-web client.

@ejona86 ejona86 added this to the Unscheduled milestone Sep 11, 2018
@simplesteph
Copy link
Contributor

Performance shouldn't be of concern, but definitely ease of deployment and testing. I'd love not to have to deploy a proxy just to test a web app. Embedded in the app over the same port or another port would absolutely simplify deployment, and would probably be a one-line code as from a user perspective. Again, simplicity is the goal here

@georgwastaken
Copy link

We only have a small web-frontend for which we want to avoid hosting a standalone proxy. Would a small wrapper like the one linked by @lbensaad be also possible/sufficient for the java server? I'd love to take a stab at implementing the grpc-web support but am not sure if its too much for a first contribution.

@carl-mastrangelo
Copy link
Contributor

@bnczk It almost certainly is too bug (but I won't stop you!) I was considering doing this as an experiment too, but wanted to go through the designs first. The big thing I see as a problem is making this secure by default. This means handling CORS and XSRF tokens, which I see as API complexity. It is not safe to not tackle those two issues, so they are part of the gRPC-Web design constraints.

@frankbenoit
Copy link

In my company, we have a Java Eclipse desktop application (Eclipse RCP) which can be remote controlled by a GRPC API.
Now we want to add a Browser-UI,
Having a proxy would increase the complexity a lot, as this is only an optional feature (but requested strongly by some users).
Please make this possible.

ulfjack added a commit to EngFlow/grpc-java that referenced this issue Mar 14, 2021
This is a basic implementation of the grpc-web protocol over HTTP/2.

Do NOT use this as-is in production; it does not properly configure
HTTP access controls through CORS.

Unfortunately, some of the changes here are a bit at odds with the
current class structure:
- The Deframer is wrapped, but the code expects it *not* to be wrapped
  in some places
- The Framer uses Sink which we overwrite with custom implementations
  of framing (to implement Base64 encoding) and writeTrailers

It also also unclear to me under what circumstances headers may not be
written yet when trailers are sent.

Also note that this *DOES NOT* add support for grpc-web over HTTP/1.1.

Related to grpc#4823.
ulfjack added a commit to EngFlow/grpc-java that referenced this issue Mar 14, 2021
This is a basic implementation of the grpc-web protocol over HTTP/2.

Do NOT use this as-is in production; it does not properly configure
HTTP access controls through CORS.

Unfortunately, some of the changes here are a bit at odds with the
current class structure:
- The Deframer is wrapped, but the code expects it *not* to be wrapped
  in some places
- The Framer uses Sink which we overwrite with custom implementations
  of framing (to implement Base64 encoding) and writeTrailers

It also also unclear to me under what circumstances headers may not be
written yet when trailers are sent.

Also note that this *DOES NOT* add support for grpc-web over HTTP/1.1.

Related to grpc#4823.
@ulfjack
Copy link
Contributor

ulfjack commented Mar 14, 2021

I posted a first draft for grpc-web support at https://github.com/EngFlow/grpc-java/tree/grpc-web. I'd welcome any feedback you have, as well as answers to my open questions (in the commit description).

@ulfjack
Copy link
Contributor

ulfjack commented Mar 18, 2021

@ejona86, is this something you could take a look at? It would be great if I could start submitting pieces - our internal patch set keeps growing, which makes it harder for us to stay on the latest gRPC releases. Currently looking into XSRF protection.

@visortelle
Copy link

We want to build a desktop version of our app (using Electron) that uses grpc-web

Of course, we want to get rid of the Envoy first.

The same thing for Rust works just beatifully:

https://github.com/hyperium/tonic/blob/master/tonic-web/README.md

As I can see, it's not so much code in Rust-tonic implementation.

This PR is looking in the same direction, but it's been in review for almost a year: #8596

@ulfjack how do you think, maybe your implementation may be done as a separate package? Not to wait another year.

@niloc132
Copy link
Contributor

@visortelle for https://github.com/deephaven/deephaven-core we updated an old patch to support grpc-java in servlets (both javax.servlet and jakarta.servlet are supported), and we added a simple filter that can live on top of the GrpcServlet, and automatically translate grpc-web into grpc so that the servlet can handle it.

The GrpcWebFilter hasn't been merged yet to our main branch, but lives for now in deephaven/deephaven-core@main...niloc132:deephaven-core:grpc-web-servlet-filter until I finalize a few things. Known missing features include support for -Bin headers and grpc-web-text content-type. This new class is already licensed as apache2, and once it is merged to main you will be able to obtain it from maven central.

Once #8596 has landed as-is, I intend to propose adding GrpcWebFilter to make it easy for downstream servlet projects to support both grpc and grpc-web. I also have a working patch to add TransportAttr support for mtls (and potentially modernize grpc-netty's support for accessing a javax.security.cert.X509Certificate instance instead of the deprecated javax.security.cert.X509Certificate type, plus a few specific quality of life improvements.

Feel free to reach out to me personally for more discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests