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

Is there any way to support a multipart form request? #410

Closed
mchusovlianov opened this issue Jun 6, 2017 · 22 comments
Closed

Is there any way to support a multipart form request? #410

mchusovlianov opened this issue Jun 6, 2017 · 22 comments

Comments

@mchusovlianov
Copy link

mchusovlianov commented Jun 6, 2017

I have to support multipart form request for file uploading. I'm looking for a way how to support this kind of request:
image
Is there any way to support this request?

@mchusovlianov mchusovlianov changed the title Any way to support a file upload request Is there any way to support a file upload request? Jun 6, 2017
@mchusovlianov mchusovlianov changed the title Is there any way to support a file upload request? Is there any way to support a multipart form request? Jun 6, 2017
@tamalsaha
Copy link
Collaborator

tamalsaha commented Jun 6, 2017

Are you trying to upload a file to some URL? You can do this by just implementing a handler for your pattern and add that pattern to the http.ServerMux.

You can see how to use http.ServerMux in the CoreOs code example. https://github.com/philips/grpc-gateway-example/blob/master/cmd/serve.go#L91 . You don't need to involve grpc & gateway for this.

@hangll
Copy link

hangll commented Jun 14, 2017

Is there any way I can do this via grpc gateway? Should I use streaming?

@tamalsaha
Copy link
Collaborator

@hangll , I would recommend not to involve grpc-gateway for uploading/downloading files. Please see my previous comment on just adding a handler via http mux.

@hangll
Copy link

hangll commented Jun 14, 2017

Thanks @tamalsaha. Can you explain more about why http mux would be a better option? Also, when should I use streaming?

@tamalsaha
Copy link
Collaborator

The separate mux option works with AJAX etc, if that is your use-case. Also, if you are really trying to upload / download files, using direct handler seems less confusing.

I have tried using the streaming option with grpc & gateway few months ago. It did not work for me from AJAX calls.

@rfielding
Copy link

The main issue is that when dealing with arbitrary REST requests, you generally are not constrained by existing standards. You bind together a POST with an expected json input body, and an expected output json body; all strongly typed.

But uploading and downloading files is already highly standardized, and we have no say in how that looks. For example, almost all toolkits and browsers when asked to upload a file, will use multipart/form-data, and we are not allowed to change it.

When it comes to file downloads, it is far more complex than a simple GET versus a URL and the bytes come back. If there is an Etag in the header, it is actually a query to see if the file has changed and to only send back bytes if it has (and 304 otherwise). If there is a range request in the GET, we absolutely must get back a properly formed range response. This is only the simple cases, not even getting into the other caching mechanisms.

There is a state-machine associated with up/down, and to transcode to grpc requires encoding these into a protobuf spec (which would be the same across applications). But note that at the OS level, draining an incoming 4GB upload would generally yield a 4k buffer from os.Read, and there would be 1 million of them. So when reading files coming up, or writing files going back out, the whole copy is generally one line like os.Copy(w,r); which is so efficient that it's actually faster to write it straight to disk (because it's a sequential write!) than it is to bother to try to hold it in memory.

We are currently dealing with encrypted high-definition video streams, the sequential write to disk is fast enough to not bottleneck the encryption. We might make a protobuf definiton to allow pure grpc clients to work, but I expect that performance might dictate that the mux just jump around grpc for draining files out to where they need to go (ie: the encode REST to grpc, then decode grpc to write to disk isn't technically necessary; and likely to be disastrous for performance.)

@bjolletz
Copy link

I'm having a similar issue where my grpc request contains both a file and some other properties. In my REST API, I would like to support this as a multipart/mixed request where the file will be one part and the other part will be a JSON object containing the rest of the properties.

According to the above posts, the suggested way to deal with this is to implement a separate handler in the mux. So far so good. I can extract the file and my JSON object from the multipart request. But what is then the best way to actually create the grpc request and get it sent to the server? I've dug into the code but could not find an obvious place to jack into to achieve this. What I'd basically want to do is to let the grpc gateway handle the unmarshaling of my JSON object into my grpc request and then just add the file to the request.

@achew22
Copy link
Collaborator

achew22 commented Oct 12, 2017

In the context where you're registering your handlers you can keep a hold of the grpc client and pass it into the http route handler for your multipart form. Does that work for you?

@bjolletz
Copy link

bjolletz commented Oct 13, 2017

I've figured out how to get hold of a grpc client and manually make a grpc call. But as far as I understand, this also then means that I must manually take care of everything that the auto-generated handler code usually does. Things like marshaling / unmarshaling JSON objects and grpc messages, connection management, error handling etc. This would force me to duplicate a lot of the auto-generated code which is not ideal.

What I would really want is a way to be able to intercept the HTTP (multipart) request and transform it into something that can be handled by the regular auto-generated handler. Perhaps something like a HandlerWrapper as described here. The problem with this is that the auto-generated gateway code does not provide access to its own handlers since they are created as anonymous functions inside the generated "RegisterXXXHandler" function. If these would instead have been named functions, I could have made a wrapper handler which would manipulate the request and then call the regular auto-generated handler. As it is now, I don't really see a way of invoking the auto-generated handlers or parts of them if I choose to handle a route manually.

If this is not the way to go, it would at least be nice to be able to invoke the JSON / grpc marshalers to be able to transform my JSON part of the request into grpc and vice versa for the response.

If someone could provide an example of how to best handle a multipart POST request, this would be very much appreciated.

@achew22
Copy link
Collaborator

achew22 commented Oct 25, 2017

Unfortunately, I think the reason people aren't jumping on this because there isn't a clear path forward. I just looked through the API spec defined by the Google http annotations and I don't see anything that maps into the concept of a multipart form upload. If there are parts of the currently internal API that would be useful to have exposed externally then that's a conversation I would love to have but I don't think this is the repo to do it in. I think the place to have this conversation would be in the repo that contains the spec we are implementing.

I'm going to close this but please feel free to mention me on any issue that's opened in that repo. If you're interested in changing the grpc-gateway API to allow for something else, I think that should be done in another issue.

Closing since this is essentially infeasible using the proto options.

@achew22 achew22 closed this as completed Oct 25, 2017
@bjolletz
Copy link

I'm aware that there is no support for handling multipart requests in the Google http annotations. That's why the suggestion earlier in this thread was to handle requests like this "manually" through the http.ServerMux. What I'm looking for is some guidance on how one would best implement such a manual handler.

Typically, when handling a multipart request, what I would like to do is just tell grpc-gateway how the request should be transformed into a grpc message and then let the grpc-gateway handle the actual grpc communication as usual. So what I'm looking for is some way to "hook back into" the auto-generated request handlers once I have transformed the incoming multipart request.

As far as I can see, if you choose to use the http.ServerMux to handle certain requests manually, you need to handle everything from JSON/grpc conversion to grpc communication yourself, meaning that you need to duplicate a lot of code that is already present in the auto-generated handlers, but that you can not access. I guess what I'm looking for is that the auto-generated handlers would provide some kind of API so that it's possible to use parts of them even if you handle the request manually.

@mayank-dixit
Copy link

@bjolletz did you finally figure out a fairly acceptable way to upload files using grpc-gateway as your app's main entry point?

@bjolletz
Copy link

bjolletz commented Feb 7, 2019

No I didn't find a way to manage multipart requests with grpc-gateway in a simple way. I haven't looked at grpc-gateway since then though.

@mayank-dixit
Copy link

@bjolletz What did you end up doing for file uploads for your project, then?

@bjolletz
Copy link

I was just evaluating the gRPC gateway to see if it would fit our needs. But because of this and some other issues, we decided to not go ahead with the gRPC gateway.

@rfielding
Copy link

@bjolletz if you are really manipulating actual http files, that's kind of the recommendation anyway. you want fine-grained control over http. you need multipart/form-data, range requesting, etag, detailed cookies, custom headers.

it's a completely different use case from just trying to strongly-type a new service that isn't directly hit by the browser.

@majelbstoat
Copy link

Is the goal of this project to implement the http annotations exactly, or are you open to implementing it fully, and offering some additional functionality over the top?

I hit exactly the same situation as @bjolletz this week. I have a working proxy (after help from Johan in the Slack channel), which reads multipart data, chunks it and passes it on to the GRPC service directly. However, in doing so, I ended up having a lot of parallel architecture. I had gateway metadata annotations decoding authTokens for me, custom error handlers to return specific JSON, and a couple of other things, not to mention the gateway’s standard conversion of grpc responses to JSON. I had to reimplement all of that in my proxy 😕

I don't need control over actual http files. go's standard *http.Request.MultipartReader does the job great.

What I'm thinking is something like:

service Media {
  rpc UploadImage(stream UploadImageRequest) returns (ImageResponse) {
    option (google.api.http) = {
      post: "/v1/images"
      body: "*"
    };
    option (grpcgateway.api.http) = {
      multipart: {
        chunk: "data"
      }
  }
}

message UploadImageRequest {
  bytes data = 1;
}

message ImageResponse {
  Image image = 1;
}

When receiving a request annotated with grpcgateway.api.http it would use *http.Request.MultipartReader, get the Part identified by the name specified in "chunk", do chunked streaming writes to the grpc service, send and close, and return the result.

More complex architectures are possible, where the chunk option is an array, or where the UploadImageRequest is something like a oneof of "header" and "bytes", but this basic approach would seem to cover 80% of uploading needs.

I'm happy to work on this if the project is open to the idea, but would probably need some help around swagger definitions etc.

@majelbstoat
Copy link

(If a plugin/extension interface existed, I would be happy to implement it using that too, instead of it going into core. I know other folks have been trying to do stuff with websockets around streaming, so there does seem to be an appetite for extending the gateway's functionality in a standard way.)

@johanbrandhorst
Copy link
Collaborator

As discussed on the slack channel, I think we want to limit the scope of the gateway to the annotations supported by http.proto.

Having said that, this is a common problem, and having some sort of generic extension framework for the gateway would be really interesting. It could solve a lot of problems that we may not want to solve in this repo if it was designed right. I think the new v2 branch coupled with the powerful new protobufv2 reflection capabilities we might be able to put something together that could even support third party annotations in the protofile.

@majelbstoat It'd be interesting to see if we could generalise your request in a PoC. I understand it would be a lot of ask that you implement something that we may not end up merging, but the nature of OSS is that it takes experimentation to ship big changes. Would you be interested in implementing a PoC of this? We could use that as a base for a new generic extension interface.

@majelbstoat
Copy link

Yeah, happy to do it, if y'all give me some guidance on expectations. Thinking out loud, I think there would need to be:

  • Extension to protoc-gen-grpc-gateway to support custom annotations, which probably mean pluggability. Similar to go_out=plugins=grpc:lib/golang/src maybe (although the grpc plugin is apparently going away)?
  • A way of adding a plugin to the gateway
  • Some kind of hook/handler sub-system that gives plugins the opportunity to pre-empt standard behaviour. I don't want to go too nuts too early on, but actually wrapping up some things like annotateMetadata, withOutgoingHeaderMatcher, withIncomingHeader matcher into plugins would make a lot of sense too. Meaning, having just one way of customising the gateway – plugins – rather than having to add one-off functions that perhaps have to do multiple jobs. That would be nice for composability.
  • A set of interfaces for capturing which hooks the plugin intends to implement.
  • Maybe support for plugins messing with swagger data?

Given that I understand the 2.0.0 plan is to move to a new pb implementation, anything that touches the proto gen stuff for now seems apt to become outdated. Perhaps the best place to start actually is a PoC for converting some existing points of customisation here to a plugin architecture? And then we can deal with the complexities of annotation stuff etc later on?

Also, this might want to live on its own issue :)

@johanbrandhorst
Copy link
Collaborator

I've created #1243

@rosspatil
Copy link

I actually get it managed by adding custom routes in runtime Mux.

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

10 participants