-
Notifications
You must be signed in to change notification settings - Fork 124
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
Change default Content-Type to be application/json instead of application/octet-stream #1229
Comments
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. |
I wanted to leave this up to the WF guys... WF, as well as any other Jakarta REST implementation does have MessageBodyWriter for
It also is not a good practice not to use a well-defined Content-Type, |
@t1 What version of WildFly are you using? Do you have an example method because WildFly definitely includes a |
Jan,
regarding your last sentence I need to say that I do actively encourage people to NOT use @consumes in resource methods, but to instead make up their mind about what set of MIME types they want to support and then either stick with the default ones OR actively bundle a JAX-RS Extension that provides exactly one that is wanted. I see @consumes on resource methods as nonsense, as it restricts the method to that MIME type, while it should be up to the runtime to decide from the set of bundled providers.
…-Markus
Von: jansupol ***@***.***
Gesendet: Donnerstag, 7. März 2024 14:10
An: jakartaee/rest
Cc: Markus KARG; Comment
Betreff: Re: [jakartaee/rest] Change default Content-Type to be application/json instead of application/octet-stream (Issue #1229)
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 <https://jakarta.ee/specifications/restful-ws/3.1/jakarta-restful-ws-spec-3.1#standard_entity_providers> 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.
—
Reply to this email directly, view it on GitHub <#1229 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAM7PNZKATOX5PRGFR2AYTTYXBRLPAVCNFSM6AAAAABEIQRJDKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBTGQ3TIMJTGU> .
You are receiving this because you commented. <https://github.com/notifications/beacon/AAM7PNZ6XBLOXSKPNSYTJ4LYXBRLPA5CNFSM6AAAAABEIQRJDKWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTWHFU5O.gif> Message ID: ***@***.***>
|
I'll have to adapt and clarify my suggestion: it makes perfect sense to use |
One option might be to support |
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 So if your resource class or method is not annotated with
I agree that |
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 |
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.
If you are in WF, I assume that artifact
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 So if my analysys is right your current issue is more an implementation issue. -- Nicolas |
I think Nicolas is correct, as long as the response entity type is “handled” by the MBW.
— Santiago
On Mar 10, 2024, at 1:13 PM, NicoNes ***@***.***> wrote:
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<https://urldefense.com/v3/__https://docs.jboss.org/resteasy/docs/4.4.2.Final/userguide/html/json.html*d4e1347__;Iw!!ACWV5N9M2RV99hQ!IJfBr66PU0QOhgpuTFFTOenOMLyin9HvD31lDDvpaLazaaBUrMwL8RrFJkr5A0k2jJP0o-sHNLkJNiGV1dC5H6wTBhLY2lNXxfA$> and thus MBW ResteasyJackson2Provider <https://urldefense.com/v3/__https://github.com/resteasy/resteasy/blob/main/providers/jackson2/src/main/java/org/jboss/resteasy/plugins/providers/jackson/ResteasyJackson2Provider.java__;!!ACWV5N9M2RV99hQ!IJfBr66PU0QOhgpuTFFTOenOMLyin9HvD31lDDvpaLazaaBUrMwL8RrFJkr5A0k2jJP0o-sHNLkJNiGV1dC5H6wTBhLY68nSXtc$> exists at runtime.
So according to the specification<https://urldefense.com/v3/__https://jakarta.ee/specifications/restful-ws/3.1/jakarta-restful-ws-spec-3.1*determine_response_type__;Iw!!ACWV5N9M2RV99hQ!IJfBr66PU0QOhgpuTFFTOenOMLyin9HvD31lDDvpaLazaaBUrMwL8RrFJkr5A0k2jJP0o-sHNLkJNiGV1dC5H6wTBhLYtaK0-60$>:
* 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 <https://urldefense.com/v3/__https://github.com/resteasy/resteasy/blob/main/providers/jackson2/src/main/java/org/jboss/resteasy/plugins/providers/jackson/ResteasyJackson2Provider.java__;!!ACWV5N9M2RV99hQ!IJfBr66PU0QOhgpuTFFTOenOMLyin9HvD31lDDvpaLazaaBUrMwL8RrFJkr5A0k2jJP0o-sHNLkJNiGV1dC5H6wTBhLY68nSXtc$> : 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<https://urldefense.com/v3/__https://github.com/jamezp__;!!ACWV5N9M2RV99hQ!IJfBr66PU0QOhgpuTFFTOenOMLyin9HvD31lDDvpaLazaaBUrMwL8RrFJkr5A0k2jJP0o-sHNLkJNiGV1dC5H6wTBhLYDCoJhn0$> 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
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/jakartaee/rest/issues/1229*issuecomment-1987298416__;Iw!!ACWV5N9M2RV99hQ!IJfBr66PU0QOhgpuTFFTOenOMLyin9HvD31lDDvpaLazaaBUrMwL8RrFJkr5A0k2jJP0o-sHNLkJNiGV1dC5H6wTBhLY6JxppSs$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ABKT2N5C6TGETEFITLHJYZDYXSID5AVCNFSM6AAAAABEIQRJDKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBXGI4TQNBRGY__;!!ACWV5N9M2RV99hQ!IJfBr66PU0QOhgpuTFFTOenOMLyin9HvD31lDDvpaLazaaBUrMwL8RrFJkr5A0k2jJP0o-sHNLkJNiGV1dC5H6wTBhLYn9RFNBU$>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
Oh... okay, cool. I never studied these algorithms before, because they work exactly how I expect it. TIL. |
FWIW in WildFly, Jakarta JSON Binding is used by default. Jackson can be enabled, but it's not the default.
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. |
Please feel free to file an issue at https://issues.redhat.com/browse/RESTEASY. I'm happy to take a look at it. |
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? |
@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 :) |
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. |
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. |
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. |
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.
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.
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? |
As there seems to be not much interest in adding this to the TCK, I'm closing this. Thanks a lot! |
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 a500 Internal Server Error
if they don't request a content type by sending anAccept
header, because WF doesn't provide aMessageBodyWriter
forapplication/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 wholeMessageBodyWriter
mechanism useless; e.g. even if you, or your runtime platform, would add aYamlMessageBodyWriter
, your clients couldn't use it without you changing all of your applications. The same is true for clients sending entities without aContent-Type
header, rendering theMessageBodyReader
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 expectedContent-Type
, and every runtime will provide for aMessageBodyReader/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 usingapplication/octet-stream
at all, not to speak of relying on this to be the default. It's not even defined whatapplication/octet-stream
means exactly; is it supposed to be Java Serialization? SRLY?!?The text was updated successfully, but these errors were encountered: