[CXF-7653]:Fix NPE in ClientProxy

[jimma]

… from provider endpoint

[rmannibucau]

it goes through a Properties access so it is synchronized, maybe read it once per client instance to avoid an useless locking?

[jimma]

@rmannibucau Thanks for review. You are right. I improved the code a bit, please check if it's good now.

[rmannibucau]

Was more thinking about doing it in the constructor but works for me

[coheigea]

The problem with this patch is that the default behaviour is still to throw a NPE in CXF - i.e. remove the change to the test in systests/uncategorized and you get a NPE. Perhaps instead we need to revisit the original fix.

[jimma]

@coheigea I wrongly thought this was an specific issue and it isn't. Looked at again and found this is actually caused by the null value return for primitive type in InvocationHandler(ClientProxy) :

If the value returned by this method is {@code null} and the interface method's return type is
primitive, then a {@code NullPointerException} will be thrown by the method invocation on the proxy instance.

Please review again. Thanks.

[andrei-ivanov]

nitpick: you can use == to check class equality

[jimma]

Thanks for review. @andrei-ivanov . I agree with you, == is more direct for class equality.

[andrei-ivanov]

nitpick: you can use == to check class equality

[jimma]

+1 .

[dkulp]

Why is all this code removed? If the WSDL states that there should be a response and there isn't, an exception should be thrown. The above is correct. By pushing some of this down into the ClientProxy's, anything that doesn't use the client proxies (like Camel endpoints) wouldn't have this check in place.

[jimma]

Why is all this code removed? If the WSDL states that there should be a response and there isn't, an exception should be thrown. The above is correct.

When I ran reproducer EmptySoapBodyTest and found the NPE is thrown from Proxy class and root cause for this NPE is a null return value for a primitive type in InvocationHandler(ClientProxy), that's why I remove this and add this check in ClientProxy. I only looked at EmptySoapBodayTest , any other reproducer I need to have a look for CXF-7653 ?

Another reason why I remove these lines is this check doesn't work for this case: for example we have a simple wsdl which generates the following operation :

public HelloResponse doSomething(HelloRequest request);

To work with Dispatch/Provider , a provider class created to simply return an empty StreamSource like:

@WebServiceProvider(
          serviceName="HelloService",
          portName="HelloPort",
          targetNamespace="http://cxf.apache.org/provider",
          wsdlLocation="WEB-INF/hello.wsdl"
  )
@BindingType(value=javax.xml.ws.soap.SOAPBinding.SOAP11HTTP_BINDING)
@ServiceMode (value=javax.xml.ws.Service.Mode.PAYLOAD)
public class HelloProvider implements Provider<Source> {
  public Source invoke(Source req)  {
      return new StreamSource();
  }
}

The client dispatch expects receiving a null value. Add this check in ClientImpl throws IllegalStateException instead of return a null result.

By pushing some of this down into the ClientProxy's, anything that doesn't use the client proxies (like Camel endpoints) wouldn't have this check in place.

For the caml endpoints without using the client proxy, it gets a null value result. Is the null value a problem for ClientCallback and something else ?

[jimma]

After talked with @dkulp and @coheigea , looked at more details . This PR possibly doesn't fix CXF-7653 completely and probably has other side effect. I'll close this PR and fix the issue after CXF-7653 with another PR #417.