JBoss Remoting Gateway using bogus HttpMarshaller?

[tfennelly]

This will seem like an unusual post :)

A long time ago I added a HttpMarshaller class with the JBoss Remoting gateway code. For some reason (that now appears to be bizarre) I added code to unwrap JBossRemoting InvocationResponse types. I think I must have done this in error, but I'm not totally sure :)

JBossRemoting (at least the CoyoteInvoker) wraps all response objects in a InvocationResponse instance where the client was a JBoss Remoting client. The code I added in the HttpMarshaller totally screws up JBoss Remoting clients as they are expecting an InvocationResponse in the response stream (has no effect on non JBoss Remoting clients).

I think the handling of the InvocationResponse in the HttpMarshaller class is invalid and should be removed. Anyone know of a reason why it should not be removed?

T.

[kconner]

According to your comment it was to add support for byte[], so the question really becomes 'why was it necessary to add this?'. I find it hard to believe you would do this for no good reason.

Kev

[tfennelly]

Yeah... that comment would explain that class's handling of the byte[], which was what the class was originally created for.

I found out why I added the "special" handling of the InvocationResponse. There's a comment on one of the linked JIRAs on the second commit re the webserive-producer quickstart not working. I tested it out after removing the "special" handling of the InvocationResponse and sure enough, it breaks. The reason is as follows....

The JBoss Remoting HttpClientInvoker uses the HTTPUnMarshaller class to read and decode the response stream (which will be an InvocationResponse when a Remoting Client calls a Remoting Server). Problem is that the HTTPUnMarshaller only uses the Content-Type header to decide whether or not to delegate to the SerializableUnMarshaller (it handles InvocationResponses) to decode the stream. When something on the server side sets the content type to a non-Binary type (e.g. text/xml as with the SOAPProcessor), the HTTPUnMarshaller decides to just read the stream as a raw stream of bytes.

This explains why I added the Special handling of the InvocationResponse in the HttpUnmarshaler. It was an invalid fix however IMO. The real issue is with JBR and it's handling of http and InvocationResponse. If JBR Server is going to wrap responses for JBR Clients, then it is effectively changing the content type, in which case it should reset the content type to octetstream (or whatever) in the response and store the original content type somewhere on the InvocationResponse and use it after unwrapping on the client side.

[kconner]

Okay, so it sounds as if we should stick with what we currently have until the remoting guys fix it.

Why is this an issue now? Why has it come up?

Kev

[tfennelly]

This has always been an issue. I guess we just didn't come across the right combination to produce it.

Anyway. I think we can work around it for now and provide backward compatibility for anyone using JBR as a Http client in production (nuts, but I suppose it is possible :) ).

The HttpMarshaller executes on the same thread as the InvocationHandler (in our case the JBossRemotingGateway). If we attach the InvocationRequest object to the thread from inside the JBossRemotingGateway, we can then access it from inside the HttpMarshaller. In the HttpMarshaller, we can check:

1. Are we talking to a Remoting Client (set in the request metadata).
2. Is the response payload a Binary Payload type (set in the response metadata).
3. Is the response object an InvocationResponse (passed to the HttpMarshaller).

If all of the above are "true", we unwrap the InvocationResponse and write the wrapped payload. We do this because under these conditions the JBR client will assume (incorrectly) that the payload is not an InvocationResponse and will attempt to read it as a raw stream of bytes.

This is not fool proof because we have no guaranteed way of serializing the underlying payload to bytes, but we didn't before this either, so we're no worse off :)

[kconner]

"tfennelly" wrote:
Anyway. I think we can work around it for now and provide backward compatibility for anyone using JBR as a Http client in production (nuts, but I suppose it is possible :) ).

Nuts perhaps, but safer than breaking code which is already working :)

At least it will give us some breathing space until the underlying issue is fixed.

"tfennelly" wrote:
The HttpMarshaller executes on the same thread as the InvocationHandler (in our case the JBossRemotingGateway). If we attach the InvocationRequest object to the thread from inside the JBossRemotingGateway, we can then access it from inside the HttpMarshaller. In the HttpMarshaller, we can check:

1. Are we talking to a Remoting Client (set in the request metadata).
2. Is the response payload a Binary Payload type (set in the response metadata).
3. Is the response object an InvocationResponse (passed to the HttpMarshaller).

If all of the above are "true", we unwrap the InvocationResponse and write the wrapped payload. We do this because under these conditions the JBR client will assume (incorrectly) that the payload is not an InvocationResponse and will attempt to read it as a raw stream of bytes.

This is not fool proof because we have no guaranteed way of serializing the underlying payload to bytes, but we didn't before this either, so we're no worse off :)

It does sound scary but, if it leaves the clients unchanged we should at least try it. As you say, we are no worse off than previously and, hopefully, it will be stable enough until the underlying cause is addressed.

Thanks Tom, this looks like a good compromise.

Kev

[tfennelly]

okidoki... just noticed I had a typo in the last post.... #2 in that list should have read "Is the response payload a *non* Binary Payload type (set in the response metadata)."