[GERONIMO-6541] NPE during XAResource recovery with invalid Xids

git-svn-id: https://svn.apache.org/repos/asf/geronimo/components/txmanager/trunk@1673231 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java
index 7d3f349..caa691d 100644
--- a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java
+++ b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java
@@ -99,7 +99,11 @@
         Xid[] prepared = xaResource.recover(XAResource.TMSTARTRSCAN + XAResource.TMENDRSCAN);
         for (int i = 0; prepared != null && i < prepared.length; i++) {
             Xid xid = prepared[i];
-            log.trace("considering recovered xid from\n name: " + xaResource.getName() + "\n " + toString(xid));
+            if (xid.getGlobalTransactionId() == null || xid.getBranchQualifier() == null) {
+                log.warn("Ignoring bad xid from\n name: " + xaResource.getName() + "\n " + toString(xid));
+                continue;
+            }
+            log.trace("Considering recovered xid from\n name: " + xaResource.getName() + "\n " + toString(xid));
             ByteArrayWrapper globalIdWrapper = new ByteArrayWrapper(xid.getGlobalTransactionId());
             XidBranchesPair xidNamesPair = ourXids.get(globalIdWrapper);
             
@@ -226,17 +230,24 @@
         StringBuilder s = new StringBuilder();
         s.append("[Xid:class=").append(xid.getClass().getSimpleName()).append(":globalId=");
         byte[] globalId = xid.getGlobalTransactionId();
-        for (int i = 0; i < globalId.length; i++) {
-            s.append(Integer.toHexString(globalId[i]));
+        if (globalId == null) {
+            s.append("null");
+        } else {
+            for (int i = 0; i < globalId.length; i++) {
+                s.append(Integer.toHexString(globalId[i]));
+            }
+            s.append(",length=").append(globalId.length);
         }
-        s.append(",length=").append(globalId.length);
         s.append(",branchId=");
         byte[] branchId = xid.getBranchQualifier();
-        for (int i = 0; i < branchId.length; i++) {
-            s.append(Integer.toHexString(branchId[i]));
+        if (branchId == null) {
+            s.append("null");
+        } else {
+            for (int i = 0; i < branchId.length; i++) {
+                s.append(Integer.toHexString(branchId[i]));
+            }
+            s.append(",length=").append(branchId.length);
         }
-        s.append(",length=");
-        s.append(branchId.length);
         s.append("]");
         return s.toString();
     }
diff --git a/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java b/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java
index e5ff07e..bc8ec2f 100644
--- a/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java
+++ b/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java
@@ -99,6 +99,67 @@
 
     }
 
+    public void testInvalidXid() throws Exception {
+        final Xid[] xids = new Xid[] { new Xid() {
+            @Override
+            public int getFormatId() {
+                return 0;
+            }
+            @Override
+            public byte[] getGlobalTransactionId() {
+                return null;
+            }
+            @Override
+            public byte[] getBranchQualifier() {
+                return new byte[0];
+            }
+        } };
+        final NamedXAResource xares = new NamedXAResource() {
+            @Override
+            public String getName() {
+                return RM1;
+            }
+            @Override
+            public void commit(Xid xid, boolean b) throws XAException {
+            }
+            @Override
+            public void end(Xid xid, int i) throws XAException {
+            }
+            @Override
+            public void forget(Xid xid) throws XAException {
+            }
+            @Override
+            public int getTransactionTimeout() throws XAException {
+                return 0;
+            }
+            @Override
+            public boolean isSameRM(XAResource xaResource) throws XAException {
+                return false;
+            }
+            @Override
+            public int prepare(Xid xid) throws XAException {
+                return 0;
+            }
+            @Override
+            public Xid[] recover(int i) throws XAException {
+                return xids;
+            }
+            @Override
+            public void rollback(Xid xid) throws XAException {
+            }
+            @Override
+            public boolean setTransactionTimeout(int i) throws XAException {
+                return false;
+            }
+            @Override
+            public void start(Xid xid, int i) throws XAException {
+            }
+        };
+        prepareForReplay();
+        Recovery recovery = txManager.recovery;
+        recovery.recoverResourceManager(xares);
+    }
+
     private void addBranch(MockTransactionInfo[] txInfos, MockXAResource xaRes) throws XAException {
         for (int i = 0; i < txInfos.length; i++) {
             MockTransactionInfo txInfo = txInfos[i];