From f3a9dd5bb07e3e920410c838b63eb82d33bcb7df Mon Sep 17 00:00:00 2001 From: Sam Van Oort Date: Mon, 29 Feb 2016 14:36:39 -0500 Subject: [PATCH 1/3] Remove broken XStream2 test of legacy custom ConcurrentHashMapSerialiation This one requires some explanation: there is a bug As custom serialization of ConcurrentHashMaps was removed circa 2010 (see eb51d46d56369915ae4031dc167322ad126f0be6) We can assume that testing conversion is no longer a high priority. Root cause appears to be a subtle change in the XStream attribute access behavior when run under JDK8 only. This appears to be the only case that triggers it. --- .../test/java/hudson/util/XStream2Test.java | 5 - .../hudson/util/old-concurrentHashMap.xml | 225 ------------------ 2 files changed, 230 deletions(-) delete mode 100644 core/src/test/resources/hudson/util/old-concurrentHashMap.xml diff --git a/core/src/test/java/hudson/util/XStream2Test.java b/core/src/test/java/hudson/util/XStream2Test.java index ed29c8c8f21f..13169658673a 100644 --- a/core/src/test/java/hudson/util/XStream2Test.java +++ b/core/src/test/java/hudson/util/XStream2Test.java @@ -296,11 +296,6 @@ public void concurrentHashMapSerialization() throws Exception { } finally { v.delete(); } - - // should be able to read in old data just fine - Foo2 map = (Foo2) new XStream2().fromXML(getClass().getResourceAsStream("old-concurrentHashMap.xml")); - assertEquals(1,map.m.size()); - assertEquals("def",map.m.get("abc")); } @Issue("SECURITY-105") diff --git a/core/src/test/resources/hudson/util/old-concurrentHashMap.xml b/core/src/test/resources/hudson/util/old-concurrentHashMap.xml deleted file mode 100644 index 14b7ef152cb5..000000000000 --- a/core/src/test/resources/hudson/util/old-concurrentHashMap.xml +++ /dev/null @@ -1,225 +0,0 @@ - - - - - - 15 - 28 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - - - 0 - - - - - - - 0.75 - - - - abc - def - - - - - \ No newline at end of file From e136952d2ab6e5db3684bdb60af1f782d672177e Mon Sep 17 00:00:00 2001 From: Sam Van Oort Date: Wed, 2 Mar 2016 10:53:03 -0500 Subject: [PATCH 2/3] Remove remainder of ConcurrentHashMap testcase --- .../test/java/hudson/util/XStream2Test.java | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/core/src/test/java/hudson/util/XStream2Test.java b/core/src/test/java/hudson/util/XStream2Test.java index 13169658673a..aeb781f6725f 100644 --- a/core/src/test/java/hudson/util/XStream2Test.java +++ b/core/src/test/java/hudson/util/XStream2Test.java @@ -272,32 +272,6 @@ public static class Foo2 { ConcurrentHashMap m = new ConcurrentHashMap(); } - /** - * Tests that ConcurrentHashMap is serialized into a more compact format, - * but still can deserialize to older, verbose format. - */ - @Test - public void concurrentHashMapSerialization() throws Exception { - Foo2 foo = new Foo2(); - foo.m.put("abc","def"); - foo.m.put("ghi","jkl"); - File v = File.createTempFile("hashmap", "xml"); - try { - new XmlFile(v).write(foo); - - // should serialize like map - String xml = FileUtils.readFileToString(v); - assertFalse(xml.contains("java.util.concurrent")); - //System.out.println(xml); - Foo2 deserialized = (Foo2) new XStream2().fromXML(xml); - assertEquals(2,deserialized.m.size()); - assertEquals("def", deserialized.m.get("abc")); - assertEquals("jkl", deserialized.m.get("ghi")); - } finally { - v.delete(); - } - } - @Issue("SECURITY-105") @Test public void dynamicProxyBlocked() { From 7dd72969d98b63d90f71ba85a498f61e84c21bac Mon Sep 17 00:00:00 2001 From: Sam Van Oort Date: Wed, 2 Mar 2016 11:00:42 -0500 Subject: [PATCH 3/3] Remove the broken ConcurrentHashMapConverter impl, since it handles a conversion dating back to 2010 and does not work with Java 8 --- .../util/ConcurrentHashMapConverter.java | 69 ------------------- core/src/main/java/hudson/util/XStream2.java | 1 - 2 files changed, 70 deletions(-) delete mode 100644 core/src/main/java/hudson/util/ConcurrentHashMapConverter.java diff --git a/core/src/main/java/hudson/util/ConcurrentHashMapConverter.java b/core/src/main/java/hudson/util/ConcurrentHashMapConverter.java deleted file mode 100644 index ae917d499673..000000000000 --- a/core/src/main/java/hudson/util/ConcurrentHashMapConverter.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2010, InfraDNA, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ -package hudson.util; - -import com.thoughtworks.xstream.XStream; -import com.thoughtworks.xstream.converters.UnmarshallingContext; -import com.thoughtworks.xstream.converters.collections.MapConverter; -import com.thoughtworks.xstream.converters.reflection.ReflectionProvider; -import com.thoughtworks.xstream.converters.reflection.SerializableConverter; -import com.thoughtworks.xstream.io.HierarchicalStreamReader; -import com.thoughtworks.xstream.mapper.Mapper; - -import java.util.concurrent.ConcurrentHashMap; - -/** - * {@link ConcurrentHashMap} should convert like a map, instead of via serialization. - * - * @author Kohsuke Kawaguchi - */ -public class ConcurrentHashMapConverter extends MapConverter { - private final SerializableConverter sc; - - public ConcurrentHashMapConverter(XStream xs) { - this(xs.getMapper(),xs.getReflectionProvider()); - } - - public ConcurrentHashMapConverter(Mapper mapper, ReflectionProvider reflectionProvider) { - super(mapper); - sc = new SerializableConverter(mapper,reflectionProvider); - } - - @Override - public boolean canConvert(Class type) { - return type==ConcurrentHashMap.class; - } - - @Override - public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) { - // ConcurrentHashMap used to serialize as custom serialization, - // so read it in a compatible fashion. - String s = reader.getAttribute("serialization"); - if(s!=null && s.equals("custom")) { - return sc.unmarshal(reader,context); - } else { - return super.unmarshal(reader, context); - } - } -} diff --git a/core/src/main/java/hudson/util/XStream2.java b/core/src/main/java/hudson/util/XStream2.java index bfe924e70ab2..8ea539fc25f0 100644 --- a/core/src/main/java/hudson/util/XStream2.java +++ b/core/src/main/java/hudson/util/XStream2.java @@ -151,7 +151,6 @@ private void init() { registerConverter(new ImmutableSortedSetConverter(getMapper(),getReflectionProvider()),10); registerConverter(new ImmutableSetConverter(getMapper(),getReflectionProvider()),10); registerConverter(new ImmutableListConverter(getMapper(),getReflectionProvider()),10); - registerConverter(new ConcurrentHashMapConverter(getMapper(),getReflectionProvider()),10); registerConverter(new CopyOnWriteMap.Tree.ConverterImpl(getMapper()),10); // needs to override MapConverter registerConverter(new DescribableList.ConverterImpl(getMapper()),10); // explicitly added to handle subtypes registerConverter(new Label.ConverterImpl(),10);