From f8ee412316b1a94d3dc35498359cc2f0ca273216 Mon Sep 17 00:00:00 2001 From: Riccardo Sirchia Date: Mon, 11 Jan 2021 06:26:40 +0100 Subject: [PATCH] Fix serialization of AssumptionViolatedException (#1654) Added serializable descriptions of values and matchers and use them in writeObject() serialization of AssumptionViolatedException. Fixes #1192 --- .../internal/AssumptionViolatedException.java | 28 +++++ .../SerializableMatcherDescription.java | 47 ++++++++ .../SerializableValueDescription.java | 38 ++++++ .../AssumptionViolatedExceptionTest.java | 112 ++++++++++++++++++ ...WithValueAndMatcherCanBeReserialized_v4_13 | Bin 0 -> 3795 bytes ...houtValueAndMatcherCanBeReserialized_v4_13 | Bin 0 -> 3638 bytes 6 files changed, 225 insertions(+) create mode 100644 src/main/java/org/junit/internal/SerializableMatcherDescription.java create mode 100644 src/main/java/org/junit/internal/SerializableValueDescription.java create mode 100644 src/test/resources/org/junit/assumptionViolatedExceptionWithValueAndMatcherCanBeReserialized_v4_13 create mode 100644 src/test/resources/org/junit/assumptionViolatedExceptionWithoutValueAndMatcherCanBeReserialized_v4_13 diff --git a/src/main/java/org/junit/internal/AssumptionViolatedException.java b/src/main/java/org/junit/internal/AssumptionViolatedException.java index 15c27af1b279..0e79b562f5f2 100644 --- a/src/main/java/org/junit/internal/AssumptionViolatedException.java +++ b/src/main/java/org/junit/internal/AssumptionViolatedException.java @@ -1,5 +1,8 @@ package org.junit.internal; +import java.io.IOException; +import java.io.ObjectOutputStream; + import org.hamcrest.Description; import org.hamcrest.Matcher; import org.hamcrest.SelfDescribing; @@ -108,4 +111,29 @@ public void describeTo(Description description) { } } } + + /** + * Override default Java object serialization to correctly deal with potentially unserializable matchers or values. + * By not implementing readObject, we assure ourselves of backwards compatibility and compatibility with the + * standard way of Java serialization. + * + * @param objectOutputStream The outputStream to write our representation to + * @throws IOException When serialization fails + */ + private void writeObject(ObjectOutputStream objectOutputStream) throws IOException { + ObjectOutputStream.PutField putField = objectOutputStream.putFields(); + putField.put("fAssumption", fAssumption); + putField.put("fValueMatcher", fValueMatcher); + + // We have to wrap the matcher into a serializable form. + putField.put("fMatcher", SerializableMatcherDescription.asSerializableMatcher(fMatcher)); + + // We have to wrap the value inside a non-String class (instead of serializing the String value directly) as + // A Description will handle a String and non-String object differently (1st is surrounded by '"' while the + // latter will be surrounded by '<' '>'. Wrapping it makes sure that the description of a serialized and + // non-serialized instance produce the exact same description + putField.put("fValue", SerializableValueDescription.asSerializableValue(fValue)); + + objectOutputStream.writeFields(); + } } diff --git a/src/main/java/org/junit/internal/SerializableMatcherDescription.java b/src/main/java/org/junit/internal/SerializableMatcherDescription.java new file mode 100644 index 000000000000..e0365572001c --- /dev/null +++ b/src/main/java/org/junit/internal/SerializableMatcherDescription.java @@ -0,0 +1,47 @@ +package org.junit.internal; + +import java.io.Serializable; + +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.hamcrest.Matcher; +import org.hamcrest.StringDescription; + +/** + * This class exists solely to provide a serializable description of a matcher to be serialized as a field in + * {@link AssumptionViolatedException}. Being a {@link Throwable}, it is required to be {@link Serializable}, but most + * implementations of {@link Matcher} are not. This class works around that limitation as + * {@link AssumptionViolatedException} only every uses the description of the {@link Matcher}, while still retaining + * backwards compatibility with classes compiled against its class signature before 4.14 and/or deserialization of + * previously serialized instances. + */ +class SerializableMatcherDescription extends BaseMatcher implements Serializable { + + private final String matcherDescription; + + private SerializableMatcherDescription(Matcher matcher) { + matcherDescription = StringDescription.asString(matcher); + } + + public boolean matches(Object o) { + throw new UnsupportedOperationException("This Matcher implementation only captures the description"); + } + + public void describeTo(Description description) { + description.appendText(matcherDescription); + } + + /** + * Factory method that checks to see if the matcher is already serializable. + * @param matcher the matcher to make serializable + * @return The provided matcher if it is null or already serializable, + * the SerializableMatcherDescription representation of it if it is not. + */ + static Matcher asSerializableMatcher(Matcher matcher) { + if (matcher == null || matcher instanceof Serializable) { + return matcher; + } else { + return new SerializableMatcherDescription(matcher); + } + } +} diff --git a/src/main/java/org/junit/internal/SerializableValueDescription.java b/src/main/java/org/junit/internal/SerializableValueDescription.java new file mode 100644 index 000000000000..4d055d7a4019 --- /dev/null +++ b/src/main/java/org/junit/internal/SerializableValueDescription.java @@ -0,0 +1,38 @@ +package org.junit.internal; + +import java.io.Serializable; + +/** + * This class exists solely to provide a serializable description of a value to be serialized as a field in + * {@link AssumptionViolatedException}. Being a {@link Throwable}, it is required to be {@link Serializable}, but a + * value of type Object provides no guarantee to be serializable. This class works around that limitation as + * {@link AssumptionViolatedException} only every uses the string representation of the value, while still retaining + * backwards compatibility with classes compiled against its class signature before 4.14 and/or deserialization of + * previously serialized instances. + */ +class SerializableValueDescription implements Serializable { + private final String value; + + private SerializableValueDescription(Object value) { + this.value = String.valueOf(value); + } + + /** + * Factory method that checks to see if the value is already serializable. + * @param value the value to make serializable + * @return The provided value if it is null or already serializable, + * the SerializableValueDescription representation of it if it is not. + */ + static Object asSerializableValue(Object value) { + if (value == null || value instanceof Serializable) { + return value; + } else { + return new SerializableValueDescription(value); + } + } + + @Override + public String toString() { + return value; + } +} diff --git a/src/test/java/org/junit/AssumptionViolatedExceptionTest.java b/src/test/java/org/junit/AssumptionViolatedExceptionTest.java index 574cdb13059d..f5dbfcb9813b 100644 --- a/src/test/java/org/junit/AssumptionViolatedExceptionTest.java +++ b/src/test/java/org/junit/AssumptionViolatedExceptionTest.java @@ -4,12 +4,27 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsNot.not; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assume.assumeThat; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.io.Serializable; + +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; import org.hamcrest.Matcher; import org.hamcrest.StringDescription; import org.junit.experimental.theories.DataPoint; import org.junit.experimental.theories.Theories; import org.junit.experimental.theories.Theory; +import org.junit.rules.TestName; import org.junit.runner.RunWith; @RunWith(Theories.class) @@ -23,6 +38,14 @@ public class AssumptionViolatedExceptionTest { @DataPoint public static Matcher NULL = null; + @Rule + public TestName name = new TestName(); + + private static final String MESSAGE = "Assumption message"; + private static Matcher SERIALIZABLE_IS_THREE = new SerializableIsThreeMatcher(); + private static final UnserializableClass UNSERIALIZABLE_VALUE = new UnserializableClass(); + private static final Matcher UNSERIALIZABLE_MATCHER = not(is(UNSERIALIZABLE_VALUE)); + @Theory public void toStringReportsMatcher(Integer actual, Matcher matcher) { assumeThat(matcher, notNullValue()); @@ -92,4 +115,93 @@ public void canSetCauseWithInstanceCreatedWithExplicitThrowableConstructor() { AssumptionViolatedException e = new AssumptionViolatedException("invalid number", cause); assertThat(e.getCause(), is(cause)); } + + @Test + public void assumptionViolatedExceptionWithoutValueAndMatcherCanBeReserialized_v4_13() + throws IOException, ClassNotFoundException { + assertReserializable(new AssumptionViolatedException(MESSAGE)); + } + + @Test + public void assumptionViolatedExceptionWithValueAndMatcherCanBeReserialized_v4_13() + throws IOException, ClassNotFoundException { + assertReserializable(new AssumptionViolatedException(MESSAGE, TWO, SERIALIZABLE_IS_THREE)); + } + + @Test + public void unserializableValueAndMatcherCanBeSerialized() throws IOException, ClassNotFoundException { + AssumptionViolatedException exception = new AssumptionViolatedException(MESSAGE, + UNSERIALIZABLE_VALUE, UNSERIALIZABLE_MATCHER); + + assertCanBeSerialized(exception); + } + + @Test + public void nullValueAndMatcherCanBeSerialized() throws IOException, ClassNotFoundException { + AssumptionViolatedException exception = new AssumptionViolatedException(MESSAGE); + + assertCanBeSerialized(exception); + } + + @Test + public void serializableValueAndMatcherCanBeSerialized() throws IOException, ClassNotFoundException { + AssumptionViolatedException exception = new AssumptionViolatedException(MESSAGE, + TWO, SERIALIZABLE_IS_THREE); + + assertCanBeSerialized(exception); + } + + private void assertCanBeSerialized(AssumptionViolatedException exception) + throws IOException, ClassNotFoundException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ObjectOutputStream oos = new ObjectOutputStream(baos); + oos.writeObject(exception); + + ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); + ObjectInputStream ois = new ObjectInputStream(bais); + AssumptionViolatedException fromStream = (AssumptionViolatedException) ois.readObject(); + + assertSerializedCorrectly(exception, fromStream); + } + + private void assertReserializable(AssumptionViolatedException expected) + throws IOException, ClassNotFoundException { + String resourceName = name.getMethodName(); + InputStream resource = getClass().getResourceAsStream(resourceName); + assertNotNull("Could not read resource " + resourceName, resource); + ObjectInputStream objectInputStream = new ObjectInputStream(resource); + AssumptionViolatedException fromStream = (AssumptionViolatedException) objectInputStream.readObject(); + + assertSerializedCorrectly(expected, fromStream); + } + + private void assertSerializedCorrectly( + AssumptionViolatedException expected, AssumptionViolatedException fromStream) { + assertNotNull(fromStream); + + // Exceptions don't implement equals() so we need to compare field by field + assertEquals("message", expected.getMessage(), fromStream.getMessage()); + assertEquals("description", StringDescription.asString(expected), StringDescription.asString(fromStream)); + // We don't check the stackTrace as that will be influenced by how the test was started + // (e.g. by maven or directly from IDE) + // We also don't check the cause as that should already be serialized correctly by the superclass + } + + private static class SerializableIsThreeMatcher extends BaseMatcher implements Serializable { + + public boolean matches(Object item) { + return IS_THREE.matches(item); + } + + public void describeTo(Description description) { + IS_THREE.describeTo(description); + } + } + + private static class UnserializableClass { + @Override + public String toString() { + return "I'm not serializable"; + } + } } diff --git a/src/test/resources/org/junit/assumptionViolatedExceptionWithValueAndMatcherCanBeReserialized_v4_13 b/src/test/resources/org/junit/assumptionViolatedExceptionWithValueAndMatcherCanBeReserialized_v4_13 new file mode 100644 index 0000000000000000000000000000000000000000..1409545b5ab9fea37211df850ec4740292026c14 GIT binary patch literal 3795 zcmds4U2GIp6h5<*Qnpab|DWYATTp~4r9~vgYP%G&uoTi3NFni!*R^1=%*BA#<+=FXH|DCnE}aQDtV=X~dP z&YXMeE-dv0^m?LfUJ6`Wnj^j+RBO`q+;g_)Fv&|}4U4M-2!BQfpdr9aWNcS*;WFnb zgfkgvdjnR^oMTSFPcdm#xF|yBOrlo8>f$9lAxd2&TGHZ59FbgHOUPy^fYQmVqtK5|cvDOc2HH?6tN|7(2I}{6~?erCGA3d)T zd}l2XF=iNpX3Wg7Jx7i_*m;q+REylshAqg-P*5p;~h!^gNCf8Me%J>)tHiA3|f$hbdTVX;~+sJ)~;lOA=m4 z>pjXHUPj$+xs~o6Pa-S*q-JA`ZrqcwF-dpqTZk;7Gu$-G!O((~iNJNa@Xe}M;*NPt zV7})(F&j$f`X#J=Avk;$&WFvE0^FT?*6G?8BN(V= zdm153*BK_z$O)Ca+yHu*K4?aPyAlp3D9_?%f3BaRxR;j1s^t<6{wq9%C1De$jo#$E zhwmMi9Q%S@b}8_&+=#yg-!};#?X-}-i zT8_p9!Hx3?> zu#eKqY7DE?tgcMMmqyhCez4MyQFOM2 zpXpdp!JWiuL^bK6$Iu$>&pmQ)j+O>qAF$`BSJyANC`#j;W}HtmwpE$!;*h*cO-(d% zn|aTni4gBke(tP`Gmt%F(=0-x_+QWU_J8%~_3jK9MQC&Igut?RHQBp3KmxpJ6t3KU z^Zr%^V_0+{BJn;o@+WLK_@imzT}GKUA_SW!I~rGi?)l^gc5?~#T4?iezEzVq%(c-C zuR%s#YZw@ZWVYU34R0!lcr@vE3BBcXo|}y3X9_^U$NLYD?|gSYx;k%tc>Q;E)j-fy x$=q4M+ay-#>fb*9^8VHLUO_Jlu#~1Cj)Ce<_`h)N6n+x|gaso1zw%ofP=pB~L5d2Z$r3QI1S$zoB~?d9#VuUL*x4EZ>(c)bM>k_Ox8T(o^xpnP}TdOXVhVK2IxVAZGv>ZP@-gkD_@T=c- za7;H;FwbTr|MiEHhfmLc^I!#ZS7D{c;)rY5TJXhkqN(d^Tr(jr^C)6Xm5>sl2Pg^ zl0}j6k8&s^pJ2_z-zt=vTsMN6Juyn#8eY#!E3b==a-7EGYbYm$&)94qwXm7UBzc57MMH1v}p0m z#-kC>!@LA)yctRjd!9u~ZCfq;RxrnpC(s!8vlOo_v~1O|Dxq598ya3K>OIFLucL0i zzLf3*&m$ZCq-IBk?mW`4BhPr|TY{`%ZL;ZMV`KMiiV(;)$`WFex*`V4a4+yew?TS*&TXEL}d>l9NvZ-Nk63kZzw$5bREkO98|Nvh>)b~DpP3W zq)A?R5Irm&bfUmR4X1OImvM8TJU~%AN=t6l8Vx7^6`sM;uoKfpm$?$)z2ll=YY=sx z0-re!KNIVzl%L9t_?O`OHs_j^s9GbuuagZ|>qXe1Q&`hL^?=6R~?~QfD+B zAA5c$axn#D38rN=YOQiMYn`fNOX!qbS}|dq)zE_&m4=Wt3-UHmDuztUxs2dkTE13> zNMewdLzx&{SzgT406NqY;TTY(GA4*#0B4*w?-*3>B1~iir;i@5(qf?gllO$OgdhJK2Ty4@M(JfC!zy)~ zti^pdit*Jb6iv)@cvF2kqb1KU+Rb*M6vRJCA#{_`*?pVT?X}5mi zz+6>9OVr4dZPt_Q%mykSMGihU@H0VxA06&_94mNj)I8t|tN55kXJ7J~&J>ONQ#g&7 zCS45}T4VirMDESgf+OfA>^bJ@M&mw;(l}=s7mAGCO(uMtlMk7xiKcG15=fc|@e?X4 zU-NMW60?G45t_#TdZlmR>pwo-&;?Evx+R_vSP8#QjxG<71MeJ#yAR&~bFYDMJo+Xg z@qRP)=WICpqiM@ literal 0 HcmV?d00001