Clarification: How to deal with empty entity + Content-Type header?

[mkarg]

Apparently the JAX-RS client API's SyncInvoker.post(Entity.entity(null, Non-Null-Media-Type) actually will POST an empty entity decorated with Content-Type: Non-Null-Media-Type. It's doubtful whether this is allowed by the HTTP spec, but it in fact works. So for the sake of this discussion, it is out of scope whether this is valid HTTP, but de-facto it can be done right now, so we need to handle it correctly and symmtetric on both ends. Let's call this "illegal requests" just for now.

Unfortunately the JAX-RS server API's processing has a problem with that (at least on Jersey). It sees the media-type, instantiates a message body reader for Non-Null-Media-Type, and asks it to process the entity, hence to read an empty stream. In case of Jersey, this fails for JSON-B, as Jersey asks JSON-B to parse an empty stream, which e. g. Johnzon cannot handle: JSON-B expects valid JSON, which an empty stream does not supply.

So independend of the question whether this is valid use of HTTP, we need to ensure that the JAX-RS API itself is consistent between client and server: If this is disallowed, the client must throw an exception itself, instead of sending an illegal request XOR if this is allowed, the server must be able to process it, independend of the used JSON-B parser.

So ultimately we need to discuss: Shall JAX-RS mandate that servers MUST be able to process "illegal" requests like this (so I will open a subsequent Jersey issue) XOR shall JAX-RS mandate that clients MUST prevent sending "illegal" requests like this one?

I personally have no bias. I just want that our API, TCK, and spec, is consistent. So both results are fine for me, as long as it prevents the current situation detected in Jersey+Johnzon+JSON-B.

[mkarg]

@jansupol @asoldano @andymc12 Vendor opinions?

[spericas]

My take on this is that our API should not get into the business of imposing these type of restrictions (especially since if you start, it is unclear to me where you end).

As for your use case, I believe Jersey or the JSON library in question (or even the application) should handle this case in a manner that is compatible with the application's semantics.

[andymc12]

I agree with @spericas - if we wanted to make a change, maybe some sort of wording in the spec like "implementations should log a warning when the client attempts to pass a null value while setting a content type".

[mkarg]

That's what I first thought, too. But in fact, why should Jersey or Johnzon do that? Both fulfil an API and pass the TCK. The problem is that neither JAX-RS MBW API nor JSONB API clearly demand that empty input streams MUST result in null, so it is up to each product to do that or fail -- both fulfils the current wording. That's why I think we need to enforce in the JavaDocs that MBWs MUST be able to deal with empty input streams.

[mkarg]

@chkal What do you think?

[chkal]

I don't think that we should log warnings on the client side in this situation. If the user sends an empty body with a content type, there may be a good reason to do so.

How an empty request body is handled on the server side highly depend on the media type. For XML and JSON this may be considered to be invalid (throw exception) or not (deserialize null), depending on how you look at it. But imagine an MBR which deserializes a java.util.Properties instances. Sending an empty body would absolutely be fine and result in an empty Properties instances. So I don't think that deciding whether empty bodies must be processable by an MBR is something that should be decided on the API level.

[mkarg]

@chkal I agree with this, but JSON-B is a special case: JAX-RS spec explicitly requires products to accept that media type when it supports JSON-B, so an application that relies on the JAX-RS spec must find a reliable behavior of the various JAX-RS products (otherwise it makes no sense at all to require this particular media type mapping). Hence either we drop the particular JSON-B media type mapping requirement, or we need to clarify what the outcome in case of JSON-B plus empty stream is like on the server side. Everything else leads to the fact that some products will accept it, while others do not -- which would be the same like not respecting the spec-required media type mapping. It would be enough to mention that this special case is intentionally up to the product vendor: That keeps the status quo, but the case is clear for the application vendor (he has to take care of the problem himself).

[chkal]

Yes, JSON-B is definitely a special case. It looks like we agree that the question of how empty entities should be handled cannot be defined in a generic way on the MBR API level. So if we want to clarify anything, we should do it explicitly for JSON-B. However, IMO this is out of scope for us. The spec just tells products that they must provide entity providers for JSON-B (if available). But this means "call the standard JSON-B API for parsing the request body". So whether empty streams a valid or not should be decided by the JSON-B spec.

[mkarg]

The problem is that this won't work, as the JSON-B API clearly says that the parser methods are only there for parsing JSON, but an empty stream is not JSON! So your proposal would not be backwards compatible! Hence, it is not enough that an MBR forwards the input stream to JSON-B. Again, it is our problem that we allow this combination to be received as we do not clearly define what the outcome the MBR shall deliver. So shall we drop JSON-B from our spec to be on the safe side?

[chkal]

The problem is that this won't work, as the JSON-B API clearly says that the parser methods are only there for parsing JSON, but an empty stream is not JSON!

Well, but it cannot be our responsibility to verify that the submitted data is valid JSON or not. Following this argumentation, we would also have to verify that the request contains JSON data and not some random binary data BEFORE we pass it to JAXB, which we cannot do.

[mkarg]

@chkal There is a difference: An empty stream is always a special case, it is never valid data. So as long as our spec contains explicit rules for JSON-B, we have to decide how to deal with that special case. Otherwise it makes no sense to mention JSON-B in our spec at all, as the result is completely random depending on which implementation of the MBW is in actual use. What the spec wants is to guarantee to a caller that valid JSON will be processed into a Java object, so it is pretty clear that the correct solution for invalid JSON (including empty streams) would be to also define that an empty stream must be interpreted as null. Otherwise that spec section provides more trouble than help, as it opens room instead of closing loops.

[chkal]

There is a difference: An empty stream is always a special case, it is never valid data.

As mentioned before: In the context of JSON-B: Yes, it is invalid! In the context of other entity providers: We don't know, it depends on the provider.

So as long as our spec contains explicit rules for JSON-B, we have to decide how to deal with that special case.

Not sure if I agree on this one. AFAIK there aren't explicit rules for JSON-B. We just enforce that environments that support JSON-B must provide an entity provider for it. That's all. It's actually just single sentence. Although it depends on your definition of "explicit rules".

Otherwise it makes no sense to mention JSON-B in our spec at all, as the result is completely random depending on which implementation of the MBW is in actual use.

I don't see any reason why we should define special rules for things like empty streams. IMO this should be defined by JSON-B. Our spec just states that products must provide a JSON-B entity provider. IMO this means that the provider should call JsonbBuilder.create().fromJson(stream) for whatever the client sends. Maybe we should make this more explicit in the spec. That's fine for me. But if fromJson() fails for an empty stream, the entity provider should also fail.

Just my 2 cents. I would be happy to hear other thoughts.

[spericas]

In complete agreement with @chkal

[mkarg]

IMO this means that the provider should call JsonbBuilder.create().fromJson(stream) for whatever the client sends. Maybe we should make this more explicit in the spec.

This will be incompatible with the JavaDocs of MBR: "In case the entity input stream is empty, the reader is expected to either return a Java representation of a zero-length entity or throw a jakarta.ws.rs.core.NoContentException in case no zero-length entity representation is defined for the supported Java type."

The code you suppose will neither return a zero-length entity nor will it throw NoContentException, since it will throw JsonbException according to the JavaDocs of fromJson.

So what we can do is:

• Agree that the current behavior of Jersey is a bug (as it throws the wrong exception class) and report that to the Jersey tracker. -- I assume this is ok for all of us.
• Optionally define a zero-length entity for JSON-B (here: null). -- I assume nobody wants that.
• Optionally define an annotation for entity parameters which instructs MBRs that they shall not throw an exception but return null. For example, JAX-RS 3.x could say in the spec that @Null is to be used for that. -- Which is something we could discuss.

[mkarg]

Just for the records, this is what JAX-RS 2.1 spec currently says about JAXB:

The pre-packaged JAXB and the pre-packaged primitive type
MessageBodyReader implementations MUST throw a NoContentException for zero-length message
entities.

So if we decide to not extend this section whith JSON-B, we produce a really unexpected inconsistency as people would expect we treat JSON like XML.

[mkarg]

Please see also eclipse-ee4j/jersey#4275.

[spericas]

Just for the records, this is what JAX-RS 2.1 spec currently says about JAXB:

The pre-packaged JAXB and the pre-packaged primitive type MessageBodyReader implementations MUST throw a NoContentException for zero-length message entities.

So if we decide to not extend this section whith JSON-B, we produce a really unexpected inconsistency as people would expect we treat JSON like XML.

Yes, this is a good point. I'm not sure I like the semantics defined for the JAXB case (anymore), but that's what the spec states. We could add a similar statement to the section about JSON-B.

[mkarg]

Just for the records, this is what JAX-RS 2.1 spec currently says about JAXB:
The pre-packaged JAXB and the pre-packaged primitive type MessageBodyReader implementations MUST throw a NoContentException for zero-length message entities.
So if we decide to not extend this section whith JSON-B, we produce a really unexpected inconsistency as people would expect we treat JSON like XML.
Yes, this is a good point. I'm not sure I like the semantics defined for the JAXB case (anymore), but that's what the spec states. We could add a similar statement to the section about JSON-B.

That's all I ask for. :-)

[spericas]

Just for the records, this is what JAX-RS 2.1 spec currently says about JAXB:
The pre-packaged JAXB and the pre-packaged primitive type MessageBodyReader implementations MUST throw a NoContentException for zero-length message entities.
So if we decide to not extend this section whith JSON-B, we produce a really unexpected inconsistency as people would expect we treat JSON like XML.
Yes, this is a good point. I'm not sure I like the semantics defined for the JAXB case (anymore), but that's what the spec states. We could add a similar statement to the section about JSON-B.

That's all I ask for. :-)

I'd be OK with that clarification to make it more consistent with JAXB.

[chkal]

@mkarg

This will be incompatible with the JavaDocs of MBR: "In case the entity input stream is empty, the reader is expected to either return a Java representation of a zero-length entity or throw a jakarta.ws.rs.core.NoContentException in case no zero-length entity representation is defined for the supported Java type."

Thanks a lot for pointing to this statement. To be honest, I missed checking the javadocs and just looked at the spec regarding the discussion we had.

In this case I absolutely agree. We can either enforce that the JSON-B MBR returns null or throws an NoContentException. And as the latter option is already enforced for the JAXB MBR, we should define the same behavior for JSON-B.

[mkarg]

I'm glad that finally we found an agreement.

So the next step we need is that the EF provides to us the source code of the spec.