From 4beb3d51cd86ff1da0e83a9cbb1b583d24960f59 Mon Sep 17 00:00:00 2001 From: amckenzie Date: Mon, 21 Oct 2019 15:19:42 +0100 Subject: [PATCH] Remove accidental logging of envelope payload if exception thrown during json to object conversion --- CHANGELOG.md | 4 ++ .../JsonObjectToObjectConverter.java | 14 +++-- .../JsonObjectToObjectConverterTest.java | 54 +++++++++---------- 3 files changed, 34 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 782528f..b1a8863 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ on [Keep a CHANGELOG](http://keepachangelog.com/). This project adheres to ## [Unreleased] +## [1.20.3] - 2019-10-21 +### Fixed +- Remove accidental logging of JsonEnvelope payload if exception thrown during json to object conversion + ## [1.20.2] - 2019-08-12 ### Changed - Improved validation of yaml files parsing with better error messages diff --git a/utilities-core/src/main/java/uk/gov/justice/services/common/converter/JsonObjectToObjectConverter.java b/utilities-core/src/main/java/uk/gov/justice/services/common/converter/JsonObjectToObjectConverter.java index ff30999..54c8dd8 100644 --- a/utilities-core/src/main/java/uk/gov/justice/services/common/converter/JsonObjectToObjectConverter.java +++ b/utilities-core/src/main/java/uk/gov/justice/services/common/converter/JsonObjectToObjectConverter.java @@ -1,5 +1,6 @@ package uk.gov.justice.services.common.converter; +import static java.lang.String.format; import static uk.gov.justice.services.common.converter.JSONObjectValueObfuscator.obfuscated; import uk.gov.justice.services.common.converter.exception.ConverterException; @@ -17,23 +18,20 @@ public class JsonObjectToObjectConverter implements TypedConverter { @Inject - ObjectMapper mapper; + private ObjectMapper objectMapper; @Override public R convert(final JsonObject source, final Class clazz) { try { - final R object = mapper.readValue(mapper.writeValueAsString(source), clazz); + final R object = objectMapper.readValue(objectMapper.writeValueAsString(source), clazz); if (object == null) { - throw new ConverterException(String.format("Failed to convert %s to Object", obfuscated( - source))); + throw new ConverterException(format("Error while converting to %s from json:[%s]", clazz.getName(), obfuscated(source))); } return object; - } catch (IOException e) { - throw new IllegalArgumentException( - String.format("Error while converting %s to JsonObject", obfuscated( - source)), e); + } catch (final IOException e) { + throw new IllegalArgumentException(format("Error while converting to %s from json (obfuscated):[%s]", clazz.getName(), obfuscated(source))); } } } diff --git a/utilities-core/src/test/java/uk/gov/justice/services/common/converter/JsonObjectToObjectConverterTest.java b/utilities-core/src/test/java/uk/gov/justice/services/common/converter/JsonObjectToObjectConverterTest.java index 466c39e..6c61569 100644 --- a/utilities-core/src/test/java/uk/gov/justice/services/common/converter/JsonObjectToObjectConverterTest.java +++ b/utilities-core/src/test/java/uk/gov/justice/services/common/converter/JsonObjectToObjectConverterTest.java @@ -1,7 +1,9 @@ package uk.gov.justice.services.common.converter; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.core.IsCollectionContaining.hasItems; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.doThrow; @@ -21,12 +23,16 @@ import javax.json.JsonObject; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; +import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) @@ -42,13 +48,14 @@ public class JsonObjectToObjectConverterTest { @Rule public ExpectedException thrown = ExpectedException.none(); - @Mock - private ObjectMapper mapper; + @Spy + private ObjectMapper objectMapper = new ObjectMapperProducer().objectMapper(); + + @InjectMocks + private JsonObjectToObjectConverter jsonObjectToObjectConverter; @Test public void shouldConvertPojoToJsonObject() throws Exception { - final JsonObjectToObjectConverter jsonObjectToObjectConverter = new JsonObjectToObjectConverter(); - jsonObjectToObjectConverter.mapper = new ObjectMapperProducer().objectMapper(); final JsonObject jsonObject = jsonObject(); final Pojo pojo = jsonObjectToObjectConverter.convert(jsonObject, Pojo.class); @@ -63,22 +70,20 @@ public void shouldConvertPojoToJsonObject() throws Exception { @Test public void shouldConvertToPojoWithUTCDateTime() throws Exception { - final JsonObjectToObjectConverter converter = new JsonObjectToObjectConverter(); - converter.mapper = new ObjectMapperProducer().objectMapper(); - assertThat(converter + assertThat(jsonObjectToObjectConverter .convert(Json.createObjectBuilder().add("dateTime", "2016-07-25T13:09:01.0+00:00").build(), PojoWithDateTime.class).getDateTime(), equalTo(ZonedDateTime.of(2016, 7, 25, 13, 9, 1, 0, ZoneId.of("UTC")))); - assertThat(converter + assertThat(jsonObjectToObjectConverter .convert(Json.createObjectBuilder().add("dateTime", "2016-07-25T13:09:01.0Z").build(), PojoWithDateTime.class).getDateTime(), equalTo(ZonedDateTime.of(2016, 7, 25, 13, 9, 1, 0, ZoneId.of("UTC")))); - assertThat(converter + assertThat(jsonObjectToObjectConverter .convert(Json.createObjectBuilder().add("dateTime", "2016-07-25T13:09:01Z").build(), PojoWithDateTime.class).getDateTime(), equalTo(ZonedDateTime.of(2016, 7, 25, 13, 9, 1, 0, ZoneId.of("UTC")))); - assertThat(converter + assertThat(jsonObjectToObjectConverter .convert(Json.createObjectBuilder().add("dateTime", "2016-07-25T16:09:01.0+03:00").build(), PojoWithDateTime.class).getDateTime(), equalTo(ZonedDateTime.of(2016, 7, 25, 13, 9, 1, 0, ZoneId.of("UTC")))); @@ -87,30 +92,19 @@ public void shouldConvertToPojoWithUTCDateTime() throws Exception { @Test public void shouldThrowExceptionOnConversionError() throws IOException { - thrown.expect(ConverterException.class); - thrown.expectMessage("Failed to convert"); - thrown.expectMessage("xxx"); - final JsonObjectToObjectConverter jsonObjectToObjectConverter = new JsonObjectToObjectConverter(); - jsonObjectToObjectConverter.mapper = mapper; - final JsonObject jsonObject = jsonObject(); - when(mapper.writeValueAsString(jsonObject)).thenReturn(null); + final UUID uuid = UUID.randomUUID(); - jsonObjectToObjectConverter.convert(jsonObject, Pojo.class); - } + final JsonObject jsonObject = Json.createObjectBuilder().add("id", uuid.toString()).build(); - @Test - public void shouldThrowExceptionOnNullResult() throws IOException { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("Error while converting"); - thrown.expectMessage("xxx"); - final JsonObjectToObjectConverter jsonObjectToObjectConverter = new JsonObjectToObjectConverter(); - jsonObjectToObjectConverter.mapper = mapper; - final JsonObject jsonObject = jsonObject(); - - doThrow(IOException.class).when(mapper).writeValueAsString(jsonObject); + doThrow(IOException.class).when(objectMapper).writeValueAsString(jsonObject); - jsonObjectToObjectConverter.convert(jsonObject, Pojo.class); + try { + jsonObjectToObjectConverter.convert(jsonObject, Pojo.class); + } catch (final IllegalArgumentException expected) { + assertThat(expected.getMessage(), is("Error while converting to uk.gov.justice.services.common.converter.JsonObjectToObjectConverterTest$Pojo from json (obfuscated):[{\"id\":\"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\"}]")); + assertThat(expected.getCause(), is(nullValue())); + } } private JsonObject jsonObject() {