jakartaee/rest

Change default Content-Type to be application/json instead of application/octet-stream

Closed this issue · 23 comments

t1 commented

The Jakarta REST spec defines the default content type to be application/octet-stream. This leads to the ridiculous situation that, at least on WildFly, clients get a 500 Internal Server Error if they don't request a content type by sending an Accept header, because WF doesn't provide a MessageBodyWriter for application/octet-stream. The common workaround is that people annotate all of their REST methods to @Produce(APPLICATION_JSON), which was intended to differentiate methods for the same path based on the type acceptable by the client... and rendering the whole MessageBodyWriter mechanism useless; e.g. even if you, or your runtime platform, would add a YamlMessageBodyWriter, your clients couldn't use it without you changing all of your applications. The same is true for clients sending entities without a Content-Type header, rendering the MessageBodyReader mechanism useless.

Simply changing the default to be application/json would make life as a developer much easier: this is by far the most commonly requested and often even implicitly expected Content-Type, and every runtime will provide for a MessageBodyReader/Writer. It would be a breaking change, though; but this is acceptable for a major version 4.0. And while it might be possible some people rely on the current default behaviour, I have never heard of anybody actually using application/octet-stream at all, not to speak of relying on this to be the default. It's not even defined what application/octet-stream means exactly; is it supposed to be Java Serialization? SRLY?!?

mkarg commented

I am not convinced that breaking backwards compatibility here is actually the best solution, as apparently this problem only exists on WildFly. Wouldn't it be much easier to (a) convince Wildfly to contain an MBW for octet-stream or (b) simply put such on the class-path / add it to your application or (c) add a container request filter that simply sets a default content-type header? At least the last one is done in five minutes, without breaking anybody's code.

jansupol commented

WF doesn't provide a MessageBodyWriter for application/octet-stream

I wanted to leave this up to the WF guys... WF, as well as any other Jakarta REST implementation does have MessageBodyWriter for application/octet-stream, depending on the Java entity type. The Specification requires the MessageBodyWriter for application/octet-stream for:

  • byte[]
  • java.lang.String
  • java.io.InputStream
  • java.io.Reader
  • java.io.File
  • jakarta.activation.DataSource
  • StreamingOutput

It also is not a good practice not to use a well-defined Content-Type, @Consumes and @Produces in a production application.

jamezp commented

@t1 What version of WildFly are you using? Do you have an example method because WildFly definitely includes a MessageBodyWriter for application/octet-stream.

mkarg commented
  • 👍1
t1 commented

I'll have to adapt and clarify my suggestion: it makes perfect sense to use application/octet-stream for the types the spec requires them for (@jansupol listed them) and that default should stay as it is. But when I return my domain model type, e.g. Product, it's just undefined, what that octet stream should include. In this case, the default should become application/json.

jamezp commented

One option might be to support @Consumes and @Produces on a jakarta.ws.rs.core.Application which would be the default for the application and it's resources.

t1 commented

One option might be to support @consumes and @produces on a jakarta.ws.rs.core.Application which would be the default for the application and it's resources.

That still would prevent supporting other content types by simply adding a new MessageBodyReader/Writer to your application or to the runtime environment.

NicoNes commented

That still would prevent supporting other content types by simply adding a new MessageBodyReader/Writer to your application or to the runtime environment.

According to the specification, if you do not annotate your class or method with @Produces to tell the set of media type you want to support for this method then the response media type dertemination mechanism takes all MessageBodyWriter available at runtime into consideration.

So if your resource class or method is not annotated with @Produces, the client does not set no Accept header, and a MessageBodyWriter able to write your domain object e.g. Product exists at runtime, then it must be used.

Simply changing the default to be application/json would make life as a developer much easier: this is by far the most commonly requested and often even implicitly expected Content-Type, and every runtime will provide for a MessageBodyReader/Writer.

I agree that application/json would be a better choice as a media type to use when response media type dertemination mechanism does not return a concrete media-type.

t1 commented

So if your resource class or method is not annotated with @produces, the client does not set no Accept header, and a MessageBodyWriter able to write your domain object e.g. Product exists at runtime, then it must be used.

The generic MessageBodyWriters are just fine. I don't want to write specific MessageBodyWriters for all my domain objects. I want things to work OOTB! And everything works already just fine, as long as the client says what they want. Only this somewhat strange default forces me to do extra steps.

If I return a byte[], java.io.File, etc., I'd better specify what it actually contains. It could be a JSON or an image or whatever. application/octet-stream is not very helpful for a client to find out what this is supposed to be. But in this case, this is clearly my responsibility as an application developer. If I simply return or accept my domain object, the MessageBodyWriters/Readers are a super elegant mechanism... I just can't think of any way to improve on that!

NicoNes commented

That still would prevent supporting other content types by simply adding a new MessageBodyReader/Writer to your application or to the runtime environment.

I was just answering to this specific part to say that nothing in the spec prevent supporting other media-type by only adding MessageBodyReader/Writer (hand made or not) to the application or runtime environment.

And everything works already just fine, as long as the client says what they want.

If you are in WF, I assume that artifact resteasy-jackson2-provider is provided by default and thus MBW ResteasyJackson2Provider exists at runtime.
So according to the specification:

  • at the end of step 2, since neither the resource class nor method is annotatetd with Produces, then "P" would contain at least all media-types defined by ResteasyJackson2Provider : application/json, application/*+json, text/json
  • at the end of step 4, since client does not define no Accept headers, "A" would be: */*
  • at the end of step 5 "M" would be: application/json, application/*+json, text/json
  • the response media type determintation should end at step 8 with: application/json

For what I saw, RESTEasy does not stricly follow the spec at step 2 and that's why the response media type determination mechanism returns application/octet-stream instead of application/json.
That also the reason why the client is forced to specify a compatbile Accept header to make step 2 work as expected.
I let @jamezp confirm my analysys.

So if my analysys is right your current issue is more an implementation issue.
But Apart of that possible implementation issue, I agree with you that application/json would be a better default nowadays.

-- Nicolas

spericas commented
t1 commented

Oh... okay, cool. I never studied these algorithms before, because they work exactly how I expect it. TIL.
So this is a bug in RestEasy/WildFly. One could argue that there should be a TCK confirming this.

jamezp commented

And everything works already just fine, as long as the client says what they want.

If you are in WF, I assume that artifact resteasy-jackson2-provider is provided by default and thus MBW ResteasyJackson2Provider exists at runtime. So according to the specification:

FWIW in WildFly, Jakarta JSON Binding is used by default. Jackson can be enabled, but it's not the default.

  • at the end of step 2, since neither the resource class nor method is annotatetd with Produces, then "P" would contain at least all media-types defined by ResteasyJackson2Provider : application/json, application/*+json, text/json
  • at the end of step 4, since client does not define no Accept headers, "A" would be: */*
  • at the end of step 5 "M" would be: application/json, application/*+json, text/json
  • the response media type determintation should end at step 8 with: application/json

For what I saw, RESTEasy does not stricly follow the spec at step 2 and that's why the response media type determination mechanism returns application/octet-stream instead of application/json. That also the reason why the client is forced to specify a compatbile Accept header to make step 2 work as expected. I let @jamezp confirm my analysys.

So if my analysys is right your current issue is more an implementation issue. But Apart of that possible implementation issue, I agree with you that application/json would be a better default nowadays.

-- Nicolas

If anyone has an example reproducer that would be helpful :) Feel free to file an issue at https://issues.redhat.com/browse/RESTEASY and I can have a look.

jamezp commented

Oh... okay, cool. I never studied these algorithms before, because they work exactly how I expect it. TIL. So this is a bug in RestEasy/WildFly. One could argue that there should be a TCK confirming this.

Please feel free to file an issue at https://issues.redhat.com/browse/RESTEASY. I'm happy to take a look at it.

t1 commented

I've created a discussion at RestEasy.

I would close this issue, but maybe someone has an idea on how to add this to the TCK?

t1 commented

@jamezp : sorry, didn't read your comment in time. Should I move the discussion to the RESTEASY ticket queue?

  • 👍1
jamezp commented

@jamezp : sorry, didn't read your comment in time. Should I move the discussion to the RESTEASY ticket queue?

No, a discussion is totally fine. I can file an issue for it later :)

t1 commented

The only thing left open is, if we need new tests in the TCK. Should I rename this issue or close it and create a new one? Or is it too unimportant? I'm fine with all options.

jansupol commented

I agree that application/json would be a better choice as a media type to use when response media type dertemination mechanism does not return a concrete media-type.

I remember back in JAX-RS 2.1 days when JSON-B was added as an automatic serialize-all for JSON, there was a (security) argument that people do not want automatically all internal data to be serialized to a client in the case they forget to add their MBW (as JSON-B serializes all private fields). So the customers were forced to "select" JSON-B at least by media type. I am not sure how valid is that argument nowadays when Jakarta users are more accustomed to JSON-B.

mkarg commented

I would say the argument still is valid.

We also need to understand and accept that JAX-RS is just a foundational API but not a full-blown ready-to-use framework product, so it is pretty ok that the API leaves out some convenience things that app frameworks can add on their own.

Some years back XML was the lingua franca and we did the mistake to hard-code it into JAX-RS, now we had to remove that hard link, resulting in backwards incompatibilities. We should not repeat the same mistake. Who knows if in some years JSON is outdated and people all switch to protobuf etc.? So I am -1 for changing the default just "for fashion" every other season.

t1 commented

(security) argument... the customers were forced to "select" JSON-B at least by media type.

Emphasis on "customers", i.e. the consumers of the API? They should generally be considered untrustworthy, so they should not be allowed to "select" unsafe things. I don't follow that line of reasoning.

JAX-RS is just a foundational API but not a full-blown ready-to-use framework product

It serves me perfectly as a full-blown ready-to-use framework product. There is only one feature left that I'd like to be added: Problem-Details, but we discussed that otherwise.

So I am -1 for changing the default just "for fashion" every other season.

After learning a lot here, I completely agree. Only the TCK could check those specified algorithms more thoroughly 😁

  • ❤️1
mkarg commented

So I am -1 for changing the default just "for fashion" every other season.

After learning a lot here, I completely agree. Only the TCK could check those specified algorithms more thoroughly 😁

So does this imply that we can close this issue?

t1 commented

As there seems to be not much interest in adding this to the TCK, I'm closing this. Thanks a lot!