You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@cxf.apache.org by Bin Zhu <lu...@gmail.com> on 2012/11/29 04:18:31 UTC

SOAPFault message improvement in CXF when there is unchecked NPE

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.

Re: SOAPFault message improvement in CXF when there is unchecked NPE

Posted by Bin Zhu <lu...@gmail.com>.
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 <lu...@gmail.com>

> Thanks for the discussion and suggestions, I will open a jira to track it
> soon.
>
>
> 2012/12/3 Ivan <xh...@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 <el...@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 <xh...@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 <el...@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 <lu...@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
>>
>
>

Re: SOAPFault message improvement in CXF when there is unchecked NPE

Posted by Bin Zhu <lu...@gmail.com>.
Thanks for the discussion and suggestions, I will open a jira to track it
soon.

2012/12/3 Ivan <xh...@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 <el...@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 <xh...@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 <el...@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 <lu...@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
>

Re: SOAPFault message improvement in CXF when there is unchecked NPE

Posted by Ivan <xh...@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 <el...@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 <xh...@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 <el...@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 <lu...@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

Re: SOAPFault message improvement in CXF when there is unchecked NPE

Posted by Aki Yoshida <el...@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 <xh...@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 <el...@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 <lu...@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

Re: SOAPFault message improvement in CXF when there is unchecked NPE

Posted by Ivan <xh...@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 <el...@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 <lu...@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

Re: SOAPFault message improvement in CXF when there is unchecked NPE

Posted by Aki Yoshida <el...@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 <lu...@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.