You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by Daniel Kulp <dk...@apache.org> on 2011/04/06 12:35:03 UTC
Re: svn commit: r1089385 - /cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
I think I'm -1 to this change. To me, this looks like it may leak security
information and such to the client.
The only message sent back to the client should be the top level message.
The "causes" should be logged server side and not reflected back. If there
are certain places where we CAN send back a specific cause, we should just do
that. We specifically don't send the stacks and such back to the client (by
default) exactly for that reason.
In the case of the SAAJIn, if it's an XMLStreamException, just do something
like:
throw new SoapFault(new org.apache.cxf.common.i18n.Message(
e.getMessage(), BUNDLE), e, message
.getVersion().getSender());
Dan
On Wednesday 06 April 2011 6:21:42 AM ningjiang@apache.org wrote:
> Author: ningjiang
> Date: Wed Apr 6 10:21:42 2011
> New Revision: 1089385
>
> URL: http://svn.apache.org/viewvc?rev=1089385&view=rev
> Log:
> CXF-3442 Fault should not swallow the cause exception message
>
> Modified:
> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
>
> Modified: cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
> URL:
> http://svn.apache.org/viewvc/cxf/trunk/api/src/main/java/org/apache/cxf/in
> terceptor/Fault.java?rev=1089385&r1=1089384&r2=1089385&view=diff
> ==========================================================================
> ==== --- cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
> (original) +++
> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java Wed Apr
> 6 10:21:42 2011 @@ -44,7 +44,13 @@ public class Fault extends
> UncheckedExce
>
> public Fault(Message message, Throwable throwable) {
> super(message, throwable);
> - this.message = message.toString();
> + StringBuffer buffer = new StringBuffer();
> + buffer.append(message.toString());
> + if (throwable != null) {
> + buffer.append(" Caused by :");
> + buffer.append(throwable.getMessage());
> + }
> + this.message = buffer.toString();
> code = FAULT_CODE_SERVER;
> }
--
Daniel Kulp
dkulp@apache.org
http://dankulp.com/blog
Talend - http://www.talend.com
Re: svn commit: r1089385 - /cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
Posted by Alessio Soldano <as...@redhat.com>.
On 04/06/2011 12:35 PM, Daniel Kulp wrote:
> I think I'm -1 to this change. To me, this looks like it may leak security
> information and such to the client.
>
Just my 2 cents, I agree with Dan, the default behaviour should not be
to provide the "cause" of the exception there.
Alessio
Re: svn commit: r1089385 - /cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
Posted by Willem Jiang <wi...@gmail.com>.
Hi Dan,
There are some other place which need to know about the cause of soap
fault such as In the
org.apache.cxf.databinding.source.XMLStreamDataReader, if there are some
saop fault is throw from it, we can't know what exactly error from the
client side.
If the CXF server is put into production, and client is developed by the
other people outside the company, it will be very difficult for the
client to trace the real error if the CXF doesn't send the cause of
exception back to the client.
Because there are lots of place that CXF throws the Fault/SOAPFault
message, I changed the Fault class for it.
Willem
On 4/6/11 6:35 PM, Daniel Kulp wrote:
>
> I think I'm -1 to this change. To me, this looks like it may leak security
> information and such to the client.
>
> The only message sent back to the client should be the top level message.
> The "causes" should be logged server side and not reflected back. If there
> are certain places where we CAN send back a specific cause, we should just do
> that. We specifically don't send the stacks and such back to the client (by
> default) exactly for that reason.
>
>
> In the case of the SAAJIn, if it's an XMLStreamException, just do something
> like:
>
> throw new SoapFault(new org.apache.cxf.common.i18n.Message(
> e.getMessage(), BUNDLE), e, message
> .getVersion().getSender());
>
> Dan
>
>
>
> On Wednesday 06 April 2011 6:21:42 AM ningjiang@apache.org wrote:
>> Author: ningjiang
>> Date: Wed Apr 6 10:21:42 2011
>> New Revision: 1089385
>>
>> URL: http://svn.apache.org/viewvc?rev=1089385&view=rev
>> Log:
>> CXF-3442 Fault should not swallow the cause exception message
>>
>> Modified:
>> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
>>
>> Modified: cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
>> URL:
>> http://svn.apache.org/viewvc/cxf/trunk/api/src/main/java/org/apache/cxf/in
>> terceptor/Fault.java?rev=1089385&r1=1089384&r2=1089385&view=diff
>> ==========================================================================
>> ==== --- cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
>> (original) +++
>> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java Wed Apr
>> 6 10:21:42 2011 @@ -44,7 +44,13 @@ public class Fault extends
>> UncheckedExce
>>
>> public Fault(Message message, Throwable throwable) {
>> super(message, throwable);
>> - this.message = message.toString();
>> + StringBuffer buffer = new StringBuffer();
>> + buffer.append(message.toString());
>> + if (throwable != null) {
>> + buffer.append(" Caused by :");
>> + buffer.append(throwable.getMessage());
>> + }
>> + this.message = buffer.toString();
>> code = FAULT_CODE_SERVER;
>> }
>
--
Willem
----------------------------------
FuseSource
Web: http://www.fusesource.com
Blog: http://willemjiang.blogspot.com (English)
http://jnn.javaeye.com (Chinese)
Twitter: willemjiang
Connect at CamelOne May 24-26
The Open Source Integration Conference
http://camelone.com
Re: svn commit: r1089385 - /cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
Posted by Willem Jiang <wi...@gmail.com>.
I agree we should not leek the security information by default.
After digging the code of FaultOutInterceptor, I think we could add a
flag check in FaultOutInterceptor to let CXF check the cause of the
Fault and change the exception message of Fault.
To support the faultStackTraceEnabled flag across the Soap11FaultOut and
Soap12FaultOut interceptor I suggest we use
"http://cxf.apache.org/fault" as the stackTrace target namespace.
Any thought?
BTW, I will revert my change on the Fault and add flag check in
FaultOutInterceptor shortly.
Willem
On 4/6/11 10:00 PM, Daniel Kulp wrote:
> On Wednesday 06 April 2011 9:48:34 AM Christian Schneider wrote:
>> Hi Dan,
>>
>> I think the cause of the exception is very import to diagnose problems
>> but I also understand that the leakage of security information is a big
>> problem.
>>
>> So how about introducing a config setting that controls if the cause and
>> stacktrace is transmitted.
>> So for development systems the users may set the flag to true and for
>> production systems they set it to false.
>
> There is already some level of support for this. If you add
> "faultStackTraceEnabled" "true" as a property (endpoint property, bus
> property, etc...), then the stack trace is added to the details for the fault.
>
> That said, it's likely not tested very well (and likely not documented) and
> I'm not sure if it digs through all the causes or not or if it's just the
> stack trace for the root cause. I think it's ALSO only for Soap 1.1.
>
> However, I'd definitely support updates to that code to push it into a common
> superclass of the two SoapFaultOut interceptors or into the
> FaultOutInterceptor in rt/core and update it to be more flexible about sending
> the internal causes and such.
>
>
> Dan
>
>
>>
>> Christian
>>
>> Am 06.04.2011 12:35, schrieb Daniel Kulp:
>>> I think I'm -1 to this change. To me, this looks like it may leak
>>> security information and such to the client.
>>>
>>> The only message sent back to the client should be the top level message.
>>> The "causes" should be logged server side and not reflected back. If
>>> there are certain places where we CAN send back a specific cause, we
>>> should just do that. We specifically don't send the stacks and such
>>> back to the client (by default) exactly for that reason.
>>>
>>>
>>> In the case of the SAAJIn, if it's an XMLStreamException, just do
>>> something like:
>>>
>>> throw new SoapFault(new org.apache.cxf.common.i18n.Message(
>>>
>>> e.getMessage(), BUNDLE), e, message
>>> .getVersion().getSender());
>>>
>>> Dan
>>>
>>> On Wednesday 06 April 2011 6:21:42 AM ningjiang@apache.org wrote:
>>>> Author: ningjiang
>>>> Date: Wed Apr 6 10:21:42 2011
>>>> New Revision: 1089385
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1089385&view=rev
>>>> Log:
>>>> CXF-3442 Fault should not swallow the cause exception message
>>>>
>>>> Modified:
>>>> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
>>>>
>>>> Modified:
>>>> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java URL:
>>>> http://svn.apache.org/viewvc/cxf/trunk/api/src/main/java/org/apache/cxf/
>>>> in terceptor/Fault.java?rev=1089385&r1=1089384&r2=1089385&view=diff
>>>> =======================================================================
>>>> === ==== ---
>>>> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
>>>> (original) +++
>>>> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java Wed
>>>> Apr 6 10:21:42 2011 @@ -44,7 +44,13 @@ public class Fault extends
>>>> UncheckedExce
>>>>
>>>> public Fault(Message message, Throwable throwable) {
>>>>
>>>> super(message, throwable);
>>>>
>>>> - this.message = message.toString();
>>>> + StringBuffer buffer = new StringBuffer();
>>>> + buffer.append(message.toString());
>>>> + if (throwable != null) {
>>>> + buffer.append(" Caused by :");
>>>> + buffer.append(throwable.getMessage());
>>>> + }
>>>> + this.message = buffer.toString();
>>>>
>>>> code = FAULT_CODE_SERVER;
>>>>
>>>> }
>
--
Willem
----------------------------------
FuseSource
Web: http://www.fusesource.com
Blog: http://willemjiang.blogspot.com (English)
http://jnn.javaeye.com (Chinese)
Twitter: willemjiang
Connect at CamelOne May 24-26
The Open Source Integration Conference
http://camelone.com
Re: svn commit: r1089385 - /cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
Posted by Daniel Kulp <dk...@apache.org>.
On Wednesday 06 April 2011 9:48:34 AM Christian Schneider wrote:
> Hi Dan,
>
> I think the cause of the exception is very import to diagnose problems
> but I also understand that the leakage of security information is a big
> problem.
>
> So how about introducing a config setting that controls if the cause and
> stacktrace is transmitted.
> So for development systems the users may set the flag to true and for
> production systems they set it to false.
There is already some level of support for this. If you add
"faultStackTraceEnabled" "true" as a property (endpoint property, bus
property, etc...), then the stack trace is added to the details for the fault.
That said, it's likely not tested very well (and likely not documented) and
I'm not sure if it digs through all the causes or not or if it's just the
stack trace for the root cause. I think it's ALSO only for Soap 1.1.
However, I'd definitely support updates to that code to push it into a common
superclass of the two SoapFaultOut interceptors or into the
FaultOutInterceptor in rt/core and update it to be more flexible about sending
the internal causes and such.
Dan
>
> Christian
>
> Am 06.04.2011 12:35, schrieb Daniel Kulp:
> > I think I'm -1 to this change. To me, this looks like it may leak
> > security information and such to the client.
> >
> > The only message sent back to the client should be the top level message.
> > The "causes" should be logged server side and not reflected back. If
> > there are certain places where we CAN send back a specific cause, we
> > should just do that. We specifically don't send the stacks and such
> > back to the client (by default) exactly for that reason.
> >
> >
> > In the case of the SAAJIn, if it's an XMLStreamException, just do
> > something like:
> >
> > throw new SoapFault(new org.apache.cxf.common.i18n.Message(
> >
> > e.getMessage(), BUNDLE), e, message
> > .getVersion().getSender());
> >
> > Dan
> >
> > On Wednesday 06 April 2011 6:21:42 AM ningjiang@apache.org wrote:
> >> Author: ningjiang
> >> Date: Wed Apr 6 10:21:42 2011
> >> New Revision: 1089385
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1089385&view=rev
> >> Log:
> >> CXF-3442 Fault should not swallow the cause exception message
> >>
> >> Modified:
> >> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
> >>
> >> Modified:
> >> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java URL:
> >> http://svn.apache.org/viewvc/cxf/trunk/api/src/main/java/org/apache/cxf/
> >> in terceptor/Fault.java?rev=1089385&r1=1089384&r2=1089385&view=diff
> >> =======================================================================
> >> === ==== ---
> >> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
> >> (original) +++
> >> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java Wed
> >> Apr 6 10:21:42 2011 @@ -44,7 +44,13 @@ public class Fault extends
> >> UncheckedExce
> >>
> >> public Fault(Message message, Throwable throwable) {
> >>
> >> super(message, throwable);
> >>
> >> - this.message = message.toString();
> >> + StringBuffer buffer = new StringBuffer();
> >> + buffer.append(message.toString());
> >> + if (throwable != null) {
> >> + buffer.append(" Caused by :");
> >> + buffer.append(throwable.getMessage());
> >> + }
> >> + this.message = buffer.toString();
> >>
> >> code = FAULT_CODE_SERVER;
> >>
> >> }
--
Daniel Kulp
dkulp@apache.org
http://dankulp.com/blog
Talend - http://www.talend.com
Re: svn commit: r1089385 - /cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
Posted by Christian Schneider <ch...@die-schneider.net>.
Hi Dan,
I think the cause of the exception is very import to diagnose problems
but I also understand that the leakage of security information is a big
problem.
So how about introducing a config setting that controls if the cause and
stacktrace is transmitted.
So for development systems the users may set the flag to true and for
production systems they set it to false.
Christian
Am 06.04.2011 12:35, schrieb Daniel Kulp:
> I think I'm -1 to this change. To me, this looks like it may leak security
> information and such to the client.
>
> The only message sent back to the client should be the top level message.
> The "causes" should be logged server side and not reflected back. If there
> are certain places where we CAN send back a specific cause, we should just do
> that. We specifically don't send the stacks and such back to the client (by
> default) exactly for that reason.
>
>
> In the case of the SAAJIn, if it's an XMLStreamException, just do something
> like:
>
> throw new SoapFault(new org.apache.cxf.common.i18n.Message(
> e.getMessage(), BUNDLE), e, message
> .getVersion().getSender());
>
> Dan
>
>
>
> On Wednesday 06 April 2011 6:21:42 AM ningjiang@apache.org wrote:
>> Author: ningjiang
>> Date: Wed Apr 6 10:21:42 2011
>> New Revision: 1089385
>>
>> URL: http://svn.apache.org/viewvc?rev=1089385&view=rev
>> Log:
>> CXF-3442 Fault should not swallow the cause exception message
>>
>> Modified:
>> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
>>
>> Modified: cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
>> URL:
>> http://svn.apache.org/viewvc/cxf/trunk/api/src/main/java/org/apache/cxf/in
>> terceptor/Fault.java?rev=1089385&r1=1089384&r2=1089385&view=diff
>> ==========================================================================
>> ==== --- cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java
>> (original) +++
>> cxf/trunk/api/src/main/java/org/apache/cxf/interceptor/Fault.java Wed Apr
>> 6 10:21:42 2011 @@ -44,7 +44,13 @@ public class Fault extends
>> UncheckedExce
>>
>> public Fault(Message message, Throwable throwable) {
>> super(message, throwable);
>> - this.message = message.toString();
>> + StringBuffer buffer = new StringBuffer();
>> + buffer.append(message.toString());
>> + if (throwable != null) {
>> + buffer.append(" Caused by :");
>> + buffer.append(throwable.getMessage());
>> + }
>> + this.message = buffer.toString();
>> code = FAULT_CODE_SERVER;
>> }
--
----
http://www.liquid-reality.de