Mailing-List: contact commits-help@cxf.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cxf.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: coheigea@apache.org
To: commits@cxf.apache.org
Message-Id: <87809e87b4db437bae0cdf35b77109cd@git.apache.org>
Subject: cxf git commit: More work on unifying error messages
Date: Tue, 27 Jan 2015 12:18:01 +0000 (UTC)

Repository: cxf
Updated Branches:
  refs/heads/master 7cd4cf67b -> 27b3b6998


More work on unifying error messages


Project: http://git-wip-us.apache.org/repos/asf/cxf/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/27b3b699
Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/27b3b699
Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/27b3b699

Branch: refs/heads/master
Commit: 27b3b6998c0295172a9c613dafc24ffc4b35e726
Parents: 7cd4cf6
Author: Colm O hEigeartaigh <coheigea@apache.org>
Authored: Tue Jan 27 12:16:57 2015 +0000
Committer: Colm O hEigeartaigh <coheigea@apache.org>
Committed: Tue Jan 27 12:17:11 2015 +0000

----------------------------------------------------------------------
 .../security/wss4j/StaxActionInInterceptor.java | 38 ++-------
 .../security/wss4j/WSS4JStaxInInterceptor.java  | 28 +------
 .../security/wss4j/StaxRoundTripActionTest.java | 19 ++---
 .../systest/ws/fault/ModifiedRequestTest.java   | 81 ++++++++++++++++----
 .../ws/security/WSSecurityClientTest.java       |  2 -
 .../cxf/systest/ws/fault/modified-server.xml    | 10 +++
 6 files changed, 92 insertions(+), 86 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf/blob/27b3b699/rt/ws/security/src/main/java/org/apache/cxf/ws/security/wss4j/StaxActionInInterceptor.java
----------------------------------------------------------------------
diff --git a/rt/ws/security/src/main/java/org/apache/cxf/ws/security/wss4j/StaxActionInInterceptor.java b/rt/ws/security/src/main/java/org/apache/cxf/ws/security/wss4j/StaxActionInInterceptor.java
index 5c9473b..e64a71c 100644
--- a/rt/ws/security/src/main/java/org/apache/cxf/ws/security/wss4j/StaxActionInInterceptor.java
+++ b/rt/ws/security/src/main/java/org/apache/cxf/ws/security/wss4j/StaxActionInInterceptor.java
@@ -21,9 +21,7 @@ package org.apache.cxf.ws.security.wss4j;
 import java.util.List;
 import java.util.logging.Logger;
 
-import org.apache.cxf.binding.soap.SoapFault;
 import org.apache.cxf.binding.soap.SoapMessage;
-import org.apache.cxf.binding.soap.SoapVersion;
 import org.apache.cxf.common.logging.LogUtils;
 import org.apache.cxf.interceptor.Fault;
 import org.apache.cxf.message.MessageUtils;
@@ -69,8 +67,8 @@ public class StaxActionInInterceptor extends AbstractPhaseInterceptor<SoapMessag
         if (incomingSecurityEventList == null) {
             LOG.warning("Security processing failed (actions mismatch)");
             WSSecurityException ex = 
-                new WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
-            throw createSoapFault(soapMessage.getVersion(), ex);
+                new WSSecurityException(WSSecurityException.ErrorCode.SECURITY_ERROR);
+            throw WSS4JUtils.createSoapFault(soapMessage, soapMessage.getVersion(), ex);
         }
         
         // First check for a SOAP Fault with no security header if we are the client
@@ -104,8 +102,8 @@ public class StaxActionInInterceptor extends AbstractPhaseInterceptor<SoapMessag
                 && !isEventInResults(requiredEvent, incomingSecurityEventList)) {
                 LOG.warning("Security processing failed (actions mismatch)");
                 WSSecurityException ex = 
-                    new WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
-                throw createSoapFault(soapMessage.getVersion(), ex);
+                    new WSSecurityException(WSSecurityException.ErrorCode.SECURITY_ERROR);
+                throw WSS4JUtils.createSoapFault(soapMessage, soapMessage.getVersion(), ex);
             }
             
             if (WSSConstants.ENCRYPT.equals(action)) {
@@ -118,8 +116,8 @@ public class StaxActionInInterceptor extends AbstractPhaseInterceptor<SoapMessag
                 if (!foundEncryptionPart) {
                     LOG.warning("Security processing failed (actions mismatch)");
                     WSSecurityException ex = 
-                        new WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
-                    throw createSoapFault(soapMessage.getVersion(), ex);
+                        new WSSecurityException(WSSecurityException.ErrorCode.SECURITY_ERROR);
+                    throw WSS4JUtils.createSoapFault(soapMessage, soapMessage.getVersion(), ex);
                 }
             } 
         }
@@ -143,28 +141,4 @@ public class StaxActionInInterceptor extends AbstractPhaseInterceptor<SoapMessag
         return null;
     }
     
-    /**
-     * Create a SoapFault from a WSSecurityException, following the SOAP Message Security
-     * 1.1 specification, chapter 12 "Error Handling".
-     * 
-     * When the Soap version is 1.1 then set the Fault/Code/Value from the fault code
-     * specified in the WSSecurityException (if it exists).
-     * 
-     * Otherwise set the Fault/Code/Value to env:Sender and the Fault/Code/Subcode/Value
-     * as the fault code from the WSSecurityException.
-     */
-    private SoapFault 
-    createSoapFault(SoapVersion version, WSSecurityException e) {
-        SoapFault fault;
-        javax.xml.namespace.QName faultCode = e.getFaultCode();
-        if (version.getVersion() == 1.1 && faultCode != null) {
-            fault = new SoapFault(e.getMessage(), e, faultCode);
-        } else {
-            fault = new SoapFault(e.getMessage(), e, version.getSender());
-            if (version.getVersion() != 1.1 && faultCode != null) {
-                fault.setSubCode(faultCode);
-            }
-        }
-        return fault;
-    }
 }

http://git-wip-us.apache.org/repos/asf/cxf/blob/27b3b699/rt/ws/security/src/main/java/org/apache/cxf/ws/security/wss4j/WSS4JStaxInInterceptor.java
----------------------------------------------------------------------
diff --git a/rt/ws/security/src/main/java/org/apache/cxf/ws/security/wss4j/WSS4JStaxInInterceptor.java b/rt/ws/security/src/main/java/org/apache/cxf/ws/security/wss4j/WSS4JStaxInInterceptor.java
index 2cdeb07..0c82445 100644
--- a/rt/ws/security/src/main/java/org/apache/cxf/ws/security/wss4j/WSS4JStaxInInterceptor.java
+++ b/rt/ws/security/src/main/java/org/apache/cxf/ws/security/wss4j/WSS4JStaxInInterceptor.java
@@ -36,7 +36,6 @@ import javax.xml.stream.util.StreamReaderDelegate;
 
 import org.apache.cxf.binding.soap.SoapFault;
 import org.apache.cxf.binding.soap.SoapMessage;
-import org.apache.cxf.binding.soap.SoapVersion;
 import org.apache.cxf.common.classloader.ClassLoaderUtils;
 import org.apache.cxf.common.i18n.Message;
 import org.apache.cxf.common.logging.LogUtils;
@@ -170,7 +169,7 @@ public class WSS4JStaxInInterceptor extends AbstractWSS4JStaxInterceptor {
             // processing in the WS-Stack.
             soapMessage.put(SECURITY_PROCESSED, Boolean.TRUE);
         } catch (WSSecurityException e) {
-            throw createSoapFault(soapMessage.getVersion(), e);
+            throw WSS4JUtils.createSoapFault(soapMessage, soapMessage.getVersion(), e);
         } catch (XMLSecurityException e) {
             throw new SoapFault(new Message("STAX_EX", LOG), e, soapMessage.getVersion().getSender());
         } catch (WSSPolicyException e) {
@@ -367,31 +366,6 @@ public class WSS4JStaxInInterceptor extends AbstractWSS4JStaxInterceptor {
         return false;
     }
     
-    /**
-     * Create a SoapFault from a WSSecurityException, following the SOAP Message Security
-     * 1.1 specification, chapter 12 "Error Handling".
-     * 
-     * When the Soap version is 1.1 then set the Fault/Code/Value from the fault code
-     * specified in the WSSecurityException (if it exists).
-     * 
-     * Otherwise set the Fault/Code/Value to env:Sender and the Fault/Code/Subcode/Value
-     * as the fault code from the WSSecurityException.
-     */
-    private SoapFault 
-    createSoapFault(SoapVersion version, WSSecurityException e) {
-        SoapFault fault;
-        javax.xml.namespace.QName faultCode = e.getFaultCode();
-        if (version.getVersion() == 1.1 && faultCode != null) {
-            fault = new SoapFault(e.getMessage(), e, faultCode);
-        } else {
-            fault = new SoapFault(e.getMessage(), e, version.getSender());
-            if (version.getVersion() != 1.1 && faultCode != null) {
-                fault.setSubCode(faultCode);
-            }
-        }
-        return fault;
-    }
-    
     private void setTokenValidators(
         WSSSecurityProperties properties, SoapMessage message
     ) throws WSSecurityException {

http://git-wip-us.apache.org/repos/asf/cxf/blob/27b3b699/rt/ws/security/src/test/java/org/apache/cxf/ws/security/wss4j/StaxRoundTripActionTest.java
----------------------------------------------------------------------
diff --git a/rt/ws/security/src/test/java/org/apache/cxf/ws/security/wss4j/StaxRoundTripActionTest.java b/rt/ws/security/src/test/java/org/apache/cxf/ws/security/wss4j/StaxRoundTripActionTest.java
index bbfade1..3f63541 100644
--- a/rt/ws/security/src/test/java/org/apache/cxf/ws/security/wss4j/StaxRoundTripActionTest.java
+++ b/rt/ws/security/src/test/java/org/apache/cxf/ws/security/wss4j/StaxRoundTripActionTest.java
@@ -37,6 +37,7 @@ import org.apache.cxf.service.Service;
 import org.apache.cxf.transport.local.LocalTransportFactory;
 import org.apache.wss4j.common.ConfigurationConstants;
 import org.apache.wss4j.common.crypto.CryptoFactory;
+import org.apache.wss4j.common.ext.WSSecurityException;
 import org.apache.wss4j.stax.ext.WSSConstants;
 import org.apache.wss4j.stax.ext.WSSSecurityProperties;
 import org.apache.xml.security.stax.ext.SecurePart;
@@ -93,8 +94,7 @@ public class StaxRoundTripActionTest extends AbstractSecurityTest {
             fail("Failure expected on the wrong action");
         } catch (javax.xml.ws.soap.SOAPFaultException ex) {
             // expected
-            String error = "An error was discovered processing";
-            assertTrue(ex.getMessage().contains(error));
+            assertTrue(ex.getMessage().contains(WSSecurityException.UNIFIED_SECURITY_ERR));
         }
     }
     
@@ -146,8 +146,7 @@ public class StaxRoundTripActionTest extends AbstractSecurityTest {
             fail("Failure expected on the wrong action");
         } catch (javax.xml.ws.soap.SOAPFaultException ex) {
             // expected
-            String error = "An error was discovered processing";
-            assertTrue(ex.getMessage().contains(error));
+            assertTrue(ex.getMessage().contains(WSSecurityException.UNIFIED_SECURITY_ERR));
         }
     }
   
@@ -201,8 +200,7 @@ public class StaxRoundTripActionTest extends AbstractSecurityTest {
             fail("Failure expected on the wrong action");
         } catch (javax.xml.ws.soap.SOAPFaultException ex) {
             // expected
-            String error = "An error was discovered processing";
-            assertTrue(ex.getMessage().contains(error));
+            assertTrue(ex.getMessage().contains(WSSecurityException.UNIFIED_SECURITY_ERR));
         }
     }
     
@@ -250,8 +248,7 @@ public class StaxRoundTripActionTest extends AbstractSecurityTest {
             fail("Failure expected on the wrong action");
         } catch (javax.xml.ws.soap.SOAPFaultException ex) {
             // expected
-            String error = "An error was discovered processing";
-            assertTrue(ex.getMessage().contains(error));
+            assertTrue(ex.getMessage().contains(WSSecurityException.UNIFIED_SECURITY_ERR));
         }
     }
     
@@ -397,8 +394,7 @@ public class StaxRoundTripActionTest extends AbstractSecurityTest {
             fail("Failure expected on the wrong action");
         } catch (javax.xml.ws.soap.SOAPFaultException ex) {
             // expected
-            String error = "An error was discovered processing";
-            assertTrue(ex.getMessage().contains(error));
+            assertTrue(ex.getMessage().contains(WSSecurityException.UNIFIED_SECURITY_ERR));
         }
     }
     
@@ -448,8 +444,7 @@ public class StaxRoundTripActionTest extends AbstractSecurityTest {
             fail("Failure expected on the wrong action");
         } catch (javax.xml.ws.soap.SOAPFaultException ex) {
             // expected
-            String error = "An error was discovered processing";
-            assertTrue(ex.getMessage().contains(error));
+            assertTrue(ex.getMessage().contains(WSSecurityException.UNIFIED_SECURITY_ERR));
         }
     }
     

http://git-wip-us.apache.org/repos/asf/cxf/blob/27b3b699/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/fault/ModifiedRequestTest.java
----------------------------------------------------------------------
diff --git a/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/fault/ModifiedRequestTest.java b/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/fault/ModifiedRequestTest.java
index 8fcdaf0..16d3d08 100644
--- a/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/fault/ModifiedRequestTest.java
+++ b/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/fault/ModifiedRequestTest.java
@@ -54,6 +54,7 @@ import org.junit.BeforeClass;
  */
 public class ModifiedRequestTest extends AbstractBusClientServerTestBase {
     static final String PORT = allocatePort(ModifiedRequestServer.class);
+    static final String STAX_PORT = allocatePort(ModifiedRequestServer.class, 2);
     
     private static final String NAMESPACE = "http://www.example.org/contract/DoubleIt";
     private static final QName SERVICE_QNAME = new QName(NAMESPACE, "DoubleItService");
@@ -96,7 +97,17 @@ public class ModifiedRequestTest extends AbstractBusClientServerTestBase {
             new ModifiedTimestampInterceptor();
         cxfClient.getOutInterceptors().add(modifyInterceptor);
         
-        makeInvocation(port);
+        makeInvocation(port, false);
+        
+        // Streaming invocation
+        port = service.getPort(portQName, DoubleItPortType.class);
+        updateAddressPort(port, STAX_PORT);
+
+        cxfClient = ClientProxy.getClient(port);
+        modifyInterceptor = new ModifiedTimestampInterceptor();
+        cxfClient.getOutInterceptors().add(modifyInterceptor);
+
+        makeInvocation(port, true);
         
         ((java.io.Closeable)port).close();
         bus.shutdown(true);
@@ -124,7 +135,17 @@ public class ModifiedRequestTest extends AbstractBusClientServerTestBase {
             new ModifiedSignatureInterceptor();
         cxfClient.getOutInterceptors().add(modifyInterceptor);
         
-        makeInvocation(port);
+        makeInvocation(port, false);
+        
+        // Streaming invocation
+        port = service.getPort(portQName, DoubleItPortType.class);
+        updateAddressPort(port, STAX_PORT);
+
+        cxfClient = ClientProxy.getClient(port);
+        modifyInterceptor = new ModifiedSignatureInterceptor();
+        cxfClient.getOutInterceptors().add(modifyInterceptor);
+
+        makeInvocation(port, true);
         
         ((java.io.Closeable)port).close();
         bus.shutdown(true);
@@ -147,7 +168,13 @@ public class ModifiedRequestTest extends AbstractBusClientServerTestBase {
                 service.getPort(portQName, DoubleItPortType.class);
         updateAddressPort(port, PORT);
         
-        makeInvocation(port);
+        makeInvocation(port, false);
+        
+        // Streaming invocation
+        port = service.getPort(portQName, DoubleItPortType.class);
+        updateAddressPort(port, STAX_PORT);
+
+        makeInvocation(port, true);
         
         ((java.io.Closeable)port).close();
         bus.shutdown(true);
@@ -175,7 +202,17 @@ public class ModifiedRequestTest extends AbstractBusClientServerTestBase {
             new ModifiedEncryptedKeyInterceptor();
         cxfClient.getOutInterceptors().add(modifyInterceptor);
         
-        makeInvocation(port);
+        makeInvocation(port, false);
+        
+        // Streaming invocation
+        port = service.getPort(portQName, DoubleItPortType.class);
+        updateAddressPort(port, STAX_PORT);
+
+        cxfClient = ClientProxy.getClient(port);
+        modifyInterceptor = new ModifiedEncryptedKeyInterceptor();
+        cxfClient.getOutInterceptors().add(modifyInterceptor);
+
+        makeInvocation(port, true);
         
         ((java.io.Closeable)port).close();
         bus.shutdown(true);
@@ -203,25 +240,43 @@ public class ModifiedRequestTest extends AbstractBusClientServerTestBase {
             new ModifiedEncryptedSOAPBody();
         cxfClient.getOutInterceptors().add(modifyInterceptor);
         
-        makeInvocation(port);
+        makeInvocation(port, false);
+        
+        // Streaming invocation
+        port = service.getPort(portQName, DoubleItPortType.class);
+        updateAddressPort(port, STAX_PORT);
+
+        cxfClient = ClientProxy.getClient(port);
+        modifyInterceptor = new ModifiedEncryptedSOAPBody();
+        cxfClient.getOutInterceptors().add(modifyInterceptor);
+
+        makeInvocation(port, true);
         
         ((java.io.Closeable)port).close();
         bus.shutdown(true);
     }
     
-    private void makeInvocation(DoubleItPortType port) throws DoubleItFault {
+    private void makeInvocation(DoubleItPortType port, boolean streaming) throws DoubleItFault {
         try {
             port.doubleIt(25);
             fail("Expected failure on a modified request");
         } catch (SOAPFaultException ex) {
             SOAPFault fault = ex.getFault();
-            assertEquals("soap:Sender", fault.getFaultCode());
-            assertEquals(WSSecurityException.UNIFIED_SECURITY_ERR, fault.getFaultString());
-            Iterator<?> subcodeIterator = fault.getFaultSubcodes();
-            assertTrue(subcodeIterator.hasNext());
-            Object subcode = subcodeIterator.next();
-            assertEquals(WSSecurityException.SECURITY_ERROR, subcode);
-            assertFalse(subcodeIterator.hasNext());
+            if (streaming) {
+                assertTrue("soap:Sender".equals(fault.getFaultCode())
+                           || "soap:Receiver".equals(fault.getFaultCode()));
+                assertTrue(fault.getFaultString().contains(WSSecurityException.UNIFIED_SECURITY_ERR));
+                Iterator<?> subcodeIterator = fault.getFaultSubcodes();
+                assertFalse(subcodeIterator.hasNext());
+            } else {
+                assertEquals("soap:Sender", fault.getFaultCode());
+                assertEquals(fault.getFaultString(), WSSecurityException.UNIFIED_SECURITY_ERR);
+                Iterator<?> subcodeIterator = fault.getFaultSubcodes();
+                assertTrue(subcodeIterator.hasNext());
+                Object subcode = subcodeIterator.next();
+                assertEquals(WSSecurityException.SECURITY_ERROR, subcode);
+                assertFalse(subcodeIterator.hasNext());
+            }
         }
     }
     

http://git-wip-us.apache.org/repos/asf/cxf/blob/27b3b699/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/security/WSSecurityClientTest.java
----------------------------------------------------------------------
diff --git a/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/security/WSSecurityClientTest.java b/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/security/WSSecurityClientTest.java
index 0be1b58..b738986 100644
--- a/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/security/WSSecurityClientTest.java
+++ b/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/security/WSSecurityClientTest.java
@@ -327,9 +327,7 @@ public class WSSecurityClientTest extends AbstractBusClientServerTestBase {
         assertTrue(result.indexOf("Fault") != -1);
     }
     
-    // TODO
     @Test
-    @org.junit.Ignore
     public void testDecoupledFaultFromSecurity() throws Exception {
         Dispatch<Source> dispatcher = null;
         java.io.InputStream is = null;

http://git-wip-us.apache.org/repos/asf/cxf/blob/27b3b699/systests/ws-security/src/test/resources/org/apache/cxf/systest/ws/fault/modified-server.xml
----------------------------------------------------------------------
diff --git a/systests/ws-security/src/test/resources/org/apache/cxf/systest/ws/fault/modified-server.xml b/systests/ws-security/src/test/resources/org/apache/cxf/systest/ws/fault/modified-server.xml
index 84390e7..6db8500 100644
--- a/systests/ws-security/src/test/resources/org/apache/cxf/systest/ws/fault/modified-server.xml
+++ b/systests/ws-security/src/test/resources/org/apache/cxf/systest/ws/fault/modified-server.xml
@@ -35,4 +35,14 @@
         </jaxws:properties>
     </jaxws:endpoint>
     
+    <jaxws:endpoint xmlns:s="http://www.example.org/contract/DoubleIt" id="AsymmetricStreaming" address="http://localhost:${testutil.ports.ModifiedRequestServer.2}/DoubleItAsymmetric" serviceName="s:DoubleItService" endpointName="s:DoubleItAsymmetricPort" implementor="org.apache.cxf.systest.ws.common.DoubleItImpl" wsdlLocation="org/apache/cxf/systest/ws/fault/DoubleItFault.wsdl">
+        <jaxws:properties>
+            <entry key="ws-security.callback-handler" value="org.apache.cxf.systest.ws.common.KeystorePasswordCallback"/>
+            <entry key="ws-security.signature.properties" value="bob.properties"/>
+            <entry key="ws-security.encryption.properties" value="alice.properties"/>
+            <entry key="ws-security.encryption.username" value="alice"/>
+            <entry key="ws-security.enable.streaming" value="true"/>
+        </jaxws:properties>
+    </jaxws:endpoint>
+    
 </beans>