Thanks. I'll look into it. -Ron
I've created JBREM-1046 "HTTPClientInvoker throws NullPointerException when HttpURLConnection.getInputStream() returns null" and committed a fix.
Observing no HTTP related failures in hudson (or anywhere else), I've closed the case.
I reran the app using your patched remoting build and it fixes the problem as I diagnosed it but this has raised some related issues with JBossWS which I need to check with Alessio Soldano.
The problem occurs when I use WS-Addressing to redirect a fault. This is appropriate with either a one way message or a request reply exchange. In either of these cases when a fault occurs the invoker ends up with a null input stream.
For one way messages WS employs (defaults to) an HTTPUnmarshaller which barfs on a null input stream. Your fix resolves this by not calling the unmarshaller.
For request-reply messages WS sets a SOAP HTTP Unmarshaller. This detects a null input stream and constructs an empty SOAP message. So, in this case your fix causes a problem for WS. SInce the unmarshaller no longer gets called the invoker returns null instead of an empty message. The current WS code does nto expect a null pointer here and falls over when it tries to run SOAP handlers.
Clearly there is a problem with WS here whichever way you look at it. Having looked at how the client ends up calling the HTTP invoker I suspect WS needs the unmarshaller to be called in both cases even when the stream is null i.e. we may need to back out this fix. Ho wsensible this is as regards any other remoting clients I am not in a position to say. Perhaps you could comment?
I'll point Alessio at this discussion to see what he says. Sorry, for the previous misdiagnosis -- it was hard to see the wood for the trees without the patch.
I suspect WS needs the unmarshaller to be called in both cases even when the stream is null i.e. we may need to back out this fix. Ho wsensible this is as regards any other remoting clients I am not in a position to say. Perhaps you could comment?
Well, logically, I think it doesn't make sense to call an unmarshaller with a null InputStream. Having fixed what I think should be considered a bug, I think rolling back the fix would amount to building in special treatment for a particular case (SOAP unmarshaller). On the other hand, by eliminating a call to the unmarshaller, I have changed the behavior of Remoting in an SP release, something I try to avoid. So I think I'll reopen the case and add a parameter, say "unmarshalnullstream", such that HttpClientInvoker defaults to the original behavior which can be changed to the new behavior by setting "unmarshalnullstream=true".
If you use the SOAP unmarshaller in both (one way / two way) cases, then I assume this change would fix your problem.
I was going to release a version 2.5.0.SP1 today, but I'll hold off briefly while we resolve this problem. I'll attach another jboss-remoting.jar to JBREM-1046.
Sorry, for the previous misdiagnosis
I'd say it was a partial diagnosis, treated with a partial cure, rather than a misdiagnosis. Anyway, np, it's all good.
Ok, I've added to HTTPClientInvoker a variable, "unmarshalNullStream", which controls the behavior when there is a null InputStream (or ErrorStream). If "unmarshalNullStream" is set to "true" (the default value), HTTPClientInvoker will call UnMarshaller.read() in the case of a null InputStream. If it is set to false, then HTTPClientInvoker will skip the call to UnMarshaller.read().
Also, I've attached an updated jboss-remoting.jar to JBREM-1046.
Please let me know if the latest changes solve your problem.
By the way, I've made these changes in Remoting version 2.5 only. Is there a chance that JBossWebServices would encounter the same problem in AS 4.2 (which uses Remoting version 2.2.x)?
Thanks Ron. Your fix looks fine. I'm not certaiin whether this problem affects AS 4.2 but I strongly suspect it does. Alessio will know for sure?
OK, I probably need to re-run/debug Andrew's test to better understand what happens here, anyway:
- yes, I would expect the same behavior with AS 4.2.x
- so, considering the final changes for this issue, I understand in case of oneway invocations we would need to set the new property in the provided Client before calling the remoting code or set the unmarshaller (perhaps another one doing nothing) even if we call invokeOneway, right?
yes, I would expect the same behavior with AS 4.2.x
Ok, I'll close the issue for Remoting 2.5 and clone it for Remoting 2.4. Thanks.
so, considering the final changes for this issue, I understand in case of oneway invocations we would need to set the new property in the provided Client before calling the remoting code or set the unmarshaller (perhaps another one doing nothing) even if we call invokeOneway, right?
What you say sounds correct, but I don't know how and when WS creates the Remoting Client, so I'll list some options. In the following I'm assuming that you use the SOAP unmarshaller, which checks for a null InputStream, for two way invocations.
1. If, for example, you create a new Client for each invocation, then you could set "unmarshalNullStream" to false for a one way invocation and use the default behavior for a two way invocation.
2. Similarly, if a given Client is always used for one way invocations, you could set "unmarshalNullStream" to false for that client; and, on the other hand, if a given Client is always used for two way invocations, you could use the default behavior for that Client.
3. If a Client sometimes makes one way invocations and sometimes makes two way invocations, then probably you should use the default behavior and use an unmarshaller for one way invocations that checks for a null InputStream, as the SOAP unmarshaller does for two way invocations.
Does that make sense?
I think Ron's new API with the flag is the best way to resolve this.
You need to supply flag unmarshallNullStream as true in all cases, This will mean you won't get bitten if Ron changes the default in a later release. And you should also always supply a SOAP handler for both request-reply and one way messages. In the one way case it does not seem to matter whether the return value is null or an empty SOAP message (i.e. one with a null Envelope). If you use the same unmarshaller as for request-reply you will get an empty message so the current handler should be ok.
Ok, I've closed JBREM-1046 "HTTPClientInvoker throws NullPointerException when HttpURLConnection.getInputStream() returns null" for Remoting version 2.5.0.SP1 and cloned it (JBREM-1052) for version 2.2.2.SP11, which will ship with the next EAP 4.2/4.3 CP releases.
Let me know if any problems arise.
OK, thanks Ron and Andrew. I'll create a jira issue today for updating the jbossws code accordingly.