Support protobuf Content-Type not limited to "application/protobuf" in UnframedGrpcService

[dinujoh]

As a user of UnframedGrpcService I want the service support Content-Type not limited to "application/protobuf" because media type for protobuf is not standardized by an RFC and so many community have different standards. For example OpenTelemetry Protocol Specification expects the client MUST set “Content-Type: application/x-protobuf” request header when sending binary-encoded Protobuf.

The UnframedGrpcService should support taking a protobuf media type override value that can be used and not throw HTTP 415 Unsupported Media Type response code.

[mscheong01]

@ikhoon May I try the fix? since there is no standard, there's no point adding headers to com.linecorp.armeria.common.MediaType. instead, we could introduce a new class that maps custom headers to GrpcSerializationFormats and receive it in GrpcServiceBuilder, then pass them to UnframedGrpcService/HttpJsonTranscodingService constructors. Also, ContentType header would have to be passed to AbstractUnframedGrpcService$deframeAndRespond to set response header. Please inform me if there is a better solution 😄

[ikhoon]

GrpcSerializationFormats is added to support framed gRPC protocols. So we should think about a solution without considering GrpcSerializationFormats.

since there is no standard, there's no point adding headers to com.linecorp.armeria.common.MediaType.

Even though application/x-protobuf is unofficial MIME Type, it is widely used in other open sources and our ProtobufRequestConverterFunction. https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/converter/protobuf/ProtobufHttpMessageConverter.html https://github.com/line/armeria/blob/5b384fbe27e7e6f9225d6db91cbb684d09dfbb5e/protobuf/src/main/java/com/linecorp/armeria/server/protobuf/ProtobufRequestConverterFunction.java#L223-L226

As we think of application/protobuf and application/x-protobuf as Google Protocol Buffers format and it is frequently used in Armeria, I prefer to add MediaType.isProtobuf() as like MediaType.isJson(). We may replace contentType.is(MediaType.PROTOBUF) with contentType.isProtobuf().

[ikhoon]

May I try the fix?

@dinujoh Are you going to fix the bug yourself? Otherwise, please let us know so @j-min5u can handle this issue. 😉

[mscheong01]

There does seem to be content types like application/x-google-protobuf or application/octet-stream, though maybe not as popular. (link)

[ikhoon]

Good point. application/x-google-protobuf is acceptable. But application/octet-stream seems too general binary format to represent protobuf.

[mscheong01]

Adding these popular protobuf content types seem enough for this issue 🙂
But I'm still wondering whether there should be a way for users to add there own content types that could be comprehended as protobuf, since usecase of new or custom content types for protobuf is possible due to the missing standard. Maybe we could create a separate discussion issue

[ikhoon]

But I'm still wondering whether there should be a way for users to add there own content types that could be comprehended as protobuf

That's a good point. We didn't get such requirements before. So it could be overkill at the moment. We may add the feature without breaking our API later if there is a desire for a custom protobuf format.

[dinujoh]

Are you going to fix the bug yourself? Otherwise, please let us know so @j-min5u can handle this issue. 😉

@j-min5u Please go ahead with the fix.

I prefer to add MediaType.isProtobuf() as like MediaType.isJson().

Are we planning to check for specific header in isProtobuf() ? or will this be a looking for regex pattern that contains protobuf or ends with protobuf ? The later one will cover most of the protobuf Content-Type use cases.

[ikhoon]

I prefer to define well-known protobuf types in MediaType and use .isProtobuf().