From 6889c6ae604b56249bb21119655ab949ccf95056 Mon Sep 17 00:00:00 2001
From: Andrey Kornev <akornev@apple.com>
Date: Fri, 17 Mar 2017 20:52:10 -0700
Subject: [PATCH] SerializedConfigValue.writeExternal and readExternal are
 inconsistent

---
 .../config/impl/SerializedConfigValue.java    | 22 +++++++++-----
 .../config/impl/ConfigValueTest.scala         | 17 +++++++++++
 .../com/typesafe/config/impl/TestUtils.scala  | 30 +++++++++++++++++++
 3 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/config/src/main/java/com/typesafe/config/impl/SerializedConfigValue.java b/config/src/main/java/com/typesafe/config/impl/SerializedConfigValue.java
index 4dc1747a..a208e8a6 100644
--- a/config/src/main/java/com/typesafe/config/impl/SerializedConfigValue.java
+++ b/config/src/main/java/com/typesafe/config/impl/SerializedConfigValue.java
@@ -3,8 +3,10 @@
  */
 package com.typesafe.config.impl;
 
+import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.DataInput;
+import java.io.DataInputStream;
 import java.io.DataOutput;
 import java.io.DataOutputStream;
 import java.io.Externalizable;
@@ -465,19 +467,23 @@ class SerializedConfigValue extends AbstractConfigValue implements Externalizabl
             SerializedField code = readCode(in);
             if (code == SerializedField.END_MARKER) {
                 return;
-            } else if (code == SerializedField.ROOT_VALUE) {
-                in.readInt(); // discard length
-                this.value = readValue(in, null /* baseOrigin */);
+            }
+            
+            DataInput input = fieldIn(in);
+            if (code == SerializedField.ROOT_VALUE) {
+                this.value = readValue(input, null /* baseOrigin */);
             } else if (code == SerializedField.ROOT_WAS_CONFIG) {
-                in.readInt(); // discard length
-                this.wasConfig = in.readBoolean();
-            } else {
-                // ignore unknown field
-                skipField(in);
+                this.wasConfig = input.readBoolean();
             }
         }
     }
 
+    private DataInput fieldIn(ObjectInput in) throws IOException {
+        byte[] bytes = new byte[in.readInt()];
+        in.readFully(bytes);
+        return new DataInputStream(new ByteArrayInputStream(bytes));
+    }
+
     private static ConfigException shouldNotBeUsed() {
         return new ConfigException.BugOrBroken(SerializedConfigValue.class.getName()
                 + " should not exist outside of serialization");
diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala
index 059c1cad..50add678 100644
--- a/config/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala
+++ b/config/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala
@@ -279,6 +279,23 @@ class ConfigValueTest extends TestUtils {
         assertTrue(b.root.toConfig eq b)
     }
 
+    /**
+      * Reproduces the issue <a href=https://github.com/typesafehub/config/issues/461>#461</a>.
+      * <p>
+      * We use a custom de-/serializer that encodes String objects in a JDK-incompatible way. Encoding used here 
+      * is rather simplistic: a long indicating the length in bytes (JDK uses a variable length integer) followed
+      * by the string's bytes. Running this test with the original SerializedConfigValue.readExternal() 
+      * implementation results in an EOFException thrown during deserialization.
+      */
+    @Test
+    def configConfigCustomSerializable() {
+        val aMap = configMap("a" -> 1, "b" -> 2, "c" -> 3)
+        val expected = new SimpleConfigObject(fakeOrigin(), aMap).toConfig
+        val actual = checkSerializableWithCustomSerializer(expected)
+
+        assertEquals(expected, actual)
+    }
+    
     @Test
     def configListEquality() {
         val aScalaSeq = Seq(1, 2, 3) map { intValue(_): AbstractConfigValue }
diff --git a/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala b/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala
index 1160de0d..13ccd160 100644
--- a/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala
+++ b/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala
@@ -16,6 +16,8 @@ import java.io.ObjectOutputStream
 import java.io.ByteArrayInputStream
 import java.io.ObjectInputStream
 import java.io.NotSerializableException
+import java.io.OutputStream
+import java.io.InputStream
 import scala.annotation.tailrec
 import java.net.URL
 import java.util.Locale
@@ -883,4 +885,32 @@ abstract trait TestUtils {
             deleteRecursive(scratch)
         }
     }
+
+    protected def checkSerializableWithCustomSerializer[T: Manifest](o: T): T = {
+        val byteStream = new ByteArrayOutputStream()
+        val objectStream = new CustomObjectOutputStream(byteStream)
+        objectStream.writeObject(o)
+        objectStream.close()
+        val inStream = new ByteArrayInputStream(byteStream.toByteArray)
+        val inObjectStream = new CustomObjectInputStream(inStream)
+        val copy = inObjectStream.readObject()
+        inObjectStream.close()
+        copy.asInstanceOf[T]
+    }
+
+    class CustomObjectOutputStream(out: OutputStream) extends ObjectOutputStream(out) {
+        override def writeUTF(str: String): Unit = {
+            val bytes = str.getBytes
+            writeLong(bytes.length)
+            write(bytes)
+        }
+    }
+
+    class CustomObjectInputStream(in: InputStream) extends ObjectInputStream(in) {
+        override def readUTF(): String = {
+            val bytes = new Array[Byte](readLong().toByte)
+            read(bytes)
+            new String(bytes)
+        }
+    }
 }