cxf-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bin Zhu <luckyzhu...@gmail.com>
Subject Re: SOAPFault message improvement in CXF when there is unchecked NPE
Date Fri, 07 Dec 2012 10:06:46 GMT
Hi All,
I opened JIRA 4684(https://issues.apache.org/jira/browse/CXF-4684) to track
this issue and investigated 2 possible ways to fix the problem.
1. Fix it in Fault it self.
Currently the behavior of  Fault.getMessage and it's super class's method
(Throwable.getMessage) are not consistent in this NPE Scenario.
 Fault.getMessage will return null while it's super class Throwable will
retrun "java.lang.NullPointerException"
SoapFault->Fault->UncheckedException->RuntimeException->Exception->Throwable.
// SoapFault->Fault means SoapFault class extends Fault class
When there is NPE, the message attribute in Fault is null while the
detailMessageAtrribute is "java.lang.NullPointerException".
The following getMessage logic will explain why their behavior is not
consistent:
UncheckedException.getMessage:
    public String getMessage() {
        if (null != message) {
            return message.toString();
        }
        return null;
    }
Throwable.getMessage:
public String getMessage() {
return detailMessage;
}

Solution:
Enhance the  UncheckedException.getMessage like below:
    public String getMessage() {
        if (null != message) {
            return message.toString();
        }
        else if(null != super.getMessage())
        {
        return super.getMessage();
        }
        return null;
    }
 Test result:
  It works using this fix.(return java.lang.NullPointerException)

2. Fix it in AbstractInvoker.createFault:
   Analysis:
    In AbstractInvoker.createFault, it will return " return new Fault(ex);"
which will set the Fault.message attribute to be null. Then Fault.getMessage
will return null.
   Solutions:
    Use Fault(Message message, Throwable throwable) instead of  Fault(ex).
    It will set Fault.message attribute as the value of message parameter.
    It prereqs a Message Object, but we can create the Message object
here:  Message
msg = new Message(ex.getMessage(),LOG,params);
   Result:
    On server side, it will get the expected message"
java.lang.NullPointerException", but on client side, it still got the "Fault
occurred while processing" message.
   Analysis:
    Debuged and find in AbstractFaultChainInitiatorObserver.onMessage
method it will use new Fault(ex) to create the Fault Object which will be
set into fault SOAP message.
    We should at least modify  this method.
    But we can't use  Fault(Message message, Throwable throwable) to
replace the  Fault(ex) here since there is no the message object.

After the comparison, the first correction method is preferred. Any
thoughts?


2012/12/4 Bin Zhu <luckyzhubin@gmail.com>

> Thanks for the discussion and suggestions, I will open a jira to track it
> soon.
>
>
> 2012/12/3 Ivan <xhhsld@gmail.com>
>
>> Yeah, there may other places need to considered except for the
>> AbstractInvoker.
>>
>> I think that the fix should go into the places where Fault instance is
>> created, not the places for serialization.
>>
>> Thanks.
>>
>> 2012/12/3 Aki Yoshida <elakito@gmail.com>
>>
>> > okay.
>> > In that case, it looks like the faultstring needs to be obtained from
>> > toString().
>> >
>> > What is not 100% clear to me is whether this rule only applies to the
>> > service exception thrown from AbstractInvoker or any runtime exception
>> > thrown during the processing.
>> >
>> > Depending on this, the fix can go into AbstractInvoker or in Fault
>> > itself or at the faultout interceptors 's fault serialization code as
>> > shown in Bin's mail.
>> >
>> >
>> > 2012/12/2 Ivan <xhhsld@gmail.com>:
>> > > Per the Java doc for Throwable.getMessage() method, it writes that
>> this
>> > > method may return null.
>> > > Also, as stated in the JaxWs spec 10.2.2.3, while mapping the
>> exception
>> > to
>> > > the soap fault, the order is :
>> > >
>> > > faultstring(Reason/Text
>> > > 1. SOAPFaultException.getFault().getFaultString()
>> > > 2. Exception.getMessage()
>> > > 3. Exception.toString()
>> > >
>> > > I think that this is a bug in AbstractInvoker.createFault method, a
>> null
>> > > checking for getMessage() should be required.
>> > >
>> > > Thoughts ?
>> > >
>> > > 2012/11/29 Aki Yoshida <elakito@gmail.com>
>> > >
>> > >> Hi,
>> > >> have you tried setting the exceptionMessageCauseEnabled property? By
>> > >> default, the exception cause is not written out in the fault for
>> > >> security concerns. But you can set this property to true to change
>> > >> this behavior. I think this is what you are looking for, no?
>> > >>
>> > >>
>> > >>
>> >
>> http://cxf.apache.org/docs/debugging-and-logging.html#DebuggingandLogging-Stacktraceinfaultdetails
>> > >>
>> > >> regards, aki
>> > >>
>> > >> 2012/11/29 Bin Zhu <luckyzhubin@gmail.com>:
>> > >> > Hi,
>> > >> >
>> > >> > When there is unchecked NPE thrown, the SOAPFault in CXF will
only
>> > throw
>> > >> > the "Fault occurred while processing." message rather than the
>> > original
>> > >> NPE
>> > >> > message.
>> > >> > This message("Fault occurred while processing.") hides the details
>> of
>> > the
>> > >> > exception and will make user hard to find the root cause of the
>> > >> exception.
>> > >> >
>> > >> > I did some investigation and found that
>> > >> > in
>> org.apache.cxf.binding.soap.interceptor.Soap11FaultOutInterceptor
>> > and
>> > >> > org.apache.cxf.binding.soap.interceptor.Soap12FaultOutInterceptor,
>> > >> > It will check fault.getMessage() :
>> > >> >                 if (fault.getMessage() != null) {
>> > >> >                     if (message.get("forced.faultstring") != null)
>> {
>> > >> >                         writer.writeCharacters((String)
>> > >> > message.get("forced.faultstring"));
>> > >> >                     } else {
>> > >> >                         writer.writeCharacters(fault.getMessage());
>> > >> >                     }
>> > >> >
>> > >> >                 } else {
>> > >> >                     writer.writeCharacters("Fault occurred while
>> > >> > processing.");
>> > >> >                 }
>> > >> > But for NPE, the fault.getMessage() will return null instead of
the
>> > >> > "java.lang.NullPointerException" in the getMessage() in NPE.
>> > >> >
>> > >> > It is suggested to improve the getMessage in the SOAPFault to
let
>> it
>> > >> > include the"java.lang.NullPointerException" so that the user can
>> get
>> > the
>> > >> > details of the SOAPFault. Any suggestions? Thanks.
>> > >>
>> > >
>> > >
>> > >
>> > > --
>> > > Ivan
>> >
>>
>>
>>
>> --
>> Ivan
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message