Clean-up data stacks in UnmarshallingContext implementations in case …

…of exception. Originally provided by Märt Bakhoff. Closes #91 and #106.
x-stream · Feb 3, 2018 · d0ad410 · d0ad410
1 parent 029c98a
commit d0ad410
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 12 deletions.
diff --git a/xstream-distribution/src/content/changes.html b/xstream-distribution/src/content/changes.html
@@ -35,6 +35,8 @@ <h1 id="upcoming-1.4.x">Upcoming 1.4.x maintenance release</h1>
 	<h2>Minor changes</h2>
 
 	<ul>
+		<li>GHPR:#91, GHPR:#106: Clean-up data stacks in UnmarshallingContext implementations in case of exception (by
+		Märt Bakhoff).</li>
 		<li>GHI:#2: Unneeded contention in DefaultConverterLookup.</li>
 		<li>GHI:#94: Fix PathConverter containing absolute Windows paths.</li>
 		<li>JIRA:XSTR-616 and GHPR:#93: Introduce StringCodec interface to support arbitrary Base64 codec

diff --git a/xstream/src/java/com/thoughtworks/xstream/core/AbstractReferenceUnmarshaller.java b/xstream/src/java/com/thoughtworks/xstream/core/AbstractReferenceUnmarshaller.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006, 2007, 2008, 2011, 2015 XStream Committers.
+ * Copyright (C) 2006, 2007, 2008, 2011, 2015, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -67,11 +67,16 @@ protected Object convert(Object parent, Class type, Converter converter) {
         } else {
             Object currentReferenceKey = getCurrentReferenceKey();
             parentStack.push(currentReferenceKey);
-            result = super.convert(parent, type, converter);
-            if (currentReferenceKey != null) {
-                values.put(currentReferenceKey, result == null ? NULL : result);
+            Object localResult = null;
+            try {
+                localResult = super.convert(parent, type, converter);
+            } finally {
+                result = localResult;
+                if (currentReferenceKey != null) {
+                    values.put(currentReferenceKey, result == null ? NULL : result);
+                }
+                parentStack.popSilently();
             }
-            parentStack.popSilently();
         }
         return result;
     }

diff --git a/xstream/src/java/com/thoughtworks/xstream/core/TreeUnmarshaller.java b/xstream/src/java/com/thoughtworks/xstream/core/TreeUnmarshaller.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2004, 2005, 2006 Joe Walnes.
- * Copyright (C) 2006, 2007, 2008, 2009, 2011 XStream Committers.
+ * Copyright (C) 2006, 2007, 2008, 2009, 2011, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -67,18 +67,18 @@ public Object convertAnother(Object parent, Class type, Converter converter) {
     }
 
     protected Object convert(Object parent, Class type, Converter converter) {
+        types.push(type);
         try {
-            types.push(type);
-            Object result = converter.unmarshal(reader, this);
-            types.popSilently();
-            return result;
-        } catch (ConversionException conversionException) {
+            return converter.unmarshal(reader, this);
+        } catch (final ConversionException conversionException) {
             addInformationTo(conversionException, type, converter, parent);
             throw conversionException;
         } catch (RuntimeException e) {
             ConversionException conversionException = new ConversionException(e);
             addInformationTo(conversionException, type, converter, parent);
             throw conversionException;
+        } finally {
+            types.popSilently();
         }
     }
 

diff --git a/xstream/src/test/com/thoughtworks/acceptance/MultipleObjectsInOneStreamTest.java b/xstream/src/test/com/thoughtworks/acceptance/MultipleObjectsInOneStreamTest.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2005, 2006 Joe Walnes.
- * Copyright (C) 2006, 2007, 2008, 2009 XStream Committers.
+ * Copyright (C) 2006, 2007, 2008, 2009, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -14,13 +14,16 @@
 import com.thoughtworks.acceptance.objects.Software;
 import com.thoughtworks.acceptance.objects.StandardObject;
 import com.thoughtworks.xstream.MarshallingStrategy;
+import com.thoughtworks.xstream.converters.ConversionException;
 import com.thoughtworks.xstream.converters.ConverterLookup;
 import com.thoughtworks.xstream.converters.DataHolder;
 import com.thoughtworks.xstream.core.ReferenceByIdMarshaller;
 import com.thoughtworks.xstream.core.ReferenceByIdUnmarshaller;
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
 import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
+import com.thoughtworks.xstream.io.ReaderWrapper;
 import com.thoughtworks.xstream.io.xml.PrettyPrintWriter;
+import com.thoughtworks.xstream.io.xml.Xpp3Driver;
 import com.thoughtworks.xstream.io.xml.XppReader;
 import com.thoughtworks.xstream.mapper.Mapper;
 import com.thoughtworks.xstream.testutil.CallLog;
@@ -176,6 +179,60 @@ public void testDrivenThroughCompressedObjectStream()
         }
     }
 
+    private static class LevelTrackingReader extends ReaderWrapper {
+        private int level;
+        protected LevelTrackingReader(HierarchicalStreamReader reader) {
+            super(reader);
+        }
+
+        public void moveDown() {
+            ++level;
+            super.moveDown();
+        }
+
+        public void moveUp() {
+            super.moveUp();
+            --level;
+        }
+
+        public int getLevel() {
+            return level;
+        }
+    }
+
+    public void testFailSafeDeserialization() throws IOException, ClassNotFoundException {
+        final String xml = ""
+            + "<object-stream>\n"
+            + "  <string>top</string>\n"
+            + "  <list>\n"
+            + "    <string>first</string>\n"
+            + "    <int-array>\n"
+            + "      <int>1</int>\n"
+            + "      <int>invalid</int>\n" // deserialization will fail here
+            + "      <int>3</int>\n"
+            + "    </int-array>\n"
+            + "    <string>last</string>\n"
+            + "  </list>\n"
+            + "  <string>bottom</string>\n"
+            + "</object-stream>";
+
+        final LevelTrackingReader reader = new LevelTrackingReader(new Xpp3Driver().createReader(new StringReader(xml)));
+        final ObjectInputStream ois = xstream.createObjectInputStream(reader);
+        final int level = reader.getLevel();
+        assertEquals("top", ois.readObject());
+        try {
+            ois.readObject();
+            fail("Thrown " + ConversionException.class.getName() + " expected");
+        } catch (final ConversionException e) {
+            assertEquals(3, reader.getLevel() - level);
+            do {
+                reader.moveUp();
+            } while (level != reader.getLevel());
+        }
+        assertEquals("bottom", ois.readObject());
+        ois.close();
+    }
+
     public void testObjectOutputStreamPropagatesCloseAndFlushEvents() throws IOException {
         // setup
         final CallLog log = new CallLog();