diff options
8 files changed, 232 insertions, 61 deletions
diff --git a/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/JsonExceptionMapper.java b/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/JsonExceptionMapper.java index 9ee72f03..55b3a0d5 100644 --- a/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/JsonExceptionMapper.java +++ b/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/JsonExceptionMapper.java @@ -42,7 +42,7 @@ public class JsonExceptionMapper implements ExceptionMapper<JsonSyntaxException> @Override public Response toResponse(JsonSyntaxException exception) { logger.warn("invalid JSON request", exception); - return Response.status(Response.Status.BAD_REQUEST).entity(new SimpleResponse("Invalid JSON request")).build(); + return Response.status(Response.Status.BAD_REQUEST).entity(new SimpleResponse("Invalid request")).build(); } @Getter diff --git a/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/RestServer.java b/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/RestServer.java index decf95f9..43e39d33 100644 --- a/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/RestServer.java +++ b/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/RestServer.java @@ -100,7 +100,7 @@ public class RestServer extends ServiceManagerContainer { String.valueOf(restServerParameters.isAaf())); props.setProperty(svcpfx + PolicyEndPointProperties.PROPERTY_HTTP_SERIALIZATION_PROVIDER, String.join(",", GsonMessageBodyHandler.class.getName(), YamlMessageBodyHandler.class.getName(), - JsonExceptionMapper.class.getName())); + JsonExceptionMapper.class.getName(), YamlExceptionMapper.class.getName())); return props; } diff --git a/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/YamlExceptionMapper.java b/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/YamlExceptionMapper.java new file mode 100644 index 00000000..ac96cab0 --- /dev/null +++ b/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/YamlExceptionMapper.java @@ -0,0 +1,55 @@ +/*- + * ============LICENSE_START======================================================= + * Copyright (C) 2019 AT&T Intellectual Property. All rights reserved. + * ================================================================================ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.policy.common.endpoints.http.server; + +import javax.ws.rs.Produces; +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; +import lombok.Getter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.yaml.snakeyaml.error.YAMLException; + +/** + * Catches JSON exceptions when decoding a REST request and converts them from an HTTP 500 + * error code to an HTTP 400 error code. + */ +@Provider +@Produces("application/yaml") +public class YamlExceptionMapper implements ExceptionMapper<YAMLException> { + private static Logger logger = LoggerFactory.getLogger(YamlExceptionMapper.class); + + @Override + public Response toResponse(YAMLException exception) { + logger.warn("invalid YAML request", exception); + return Response.status(Response.Status.BAD_REQUEST).entity(new SimpleResponse("Invalid request")).build(); + } + + @Getter + private static class SimpleResponse { + private String errorDetails; + + public SimpleResponse(String errorDetails) { + this.errorDetails = errorDetails; + } + } +} diff --git a/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/YamlMessageBodyHandler.java b/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/YamlMessageBodyHandler.java index ab09c1a6..36418e4a 100644 --- a/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/YamlMessageBodyHandler.java +++ b/policy-endpoints/src/main/java/org/onap/policy/common/endpoints/http/server/YamlMessageBodyHandler.java @@ -20,11 +20,13 @@ package org.onap.policy.common.endpoints.http.server; +import com.google.gson.JsonSyntaxException; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; import java.io.OutputStreamWriter; +import java.io.Reader; import java.lang.annotation.Annotation; import java.lang.reflect.Type; import java.nio.charset.StandardCharsets; @@ -39,6 +41,7 @@ import org.onap.policy.common.utils.coder.CoderException; import org.onap.policy.common.utils.coder.StandardYamlCoder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.yaml.snakeyaml.error.YAMLException; /** * Provider that serializes and de-serializes JSON via gson. @@ -72,7 +75,7 @@ public class YamlMessageBodyHandler implements MessageBodyReader<Object>, Messag MultivaluedMap<String, Object> httpHeaders, OutputStream entityStream) throws IOException { try (OutputStreamWriter writer = new OutputStreamWriter(entityStream, StandardCharsets.UTF_8)) { - new StandardYamlCoder().encode(writer, object); + new MyYamlCoder().encode(writer, object); } catch (CoderException e) { throw new IOException(e); @@ -101,10 +104,24 @@ public class YamlMessageBodyHandler implements MessageBodyReader<Object>, Messag try (InputStreamReader streamReader = new InputStreamReader(entityStream, StandardCharsets.UTF_8)) { Class<?> clazz = (Class<?>) genericType; - return new StandardYamlCoder().decode(streamReader, clazz); + return new MyYamlCoder().decode(streamReader, clazz); + } + } - } catch (CoderException e) { - throw new IOException(e); + /** + * Yaml coder that yields YAMLException on input so that the http servlet can identify + * it and generate a bad-request status code. Only the {@link #decode(Reader, Class)} + * method must be overridden. + */ + private static class MyYamlCoder extends StandardYamlCoder { + @Override + public <T> T decode(Reader source, Class<T> clazz) { + try { + return fromJson(source, clazz); + + } catch (JsonSyntaxException e) { + throw new YAMLException(e); + } } } } diff --git a/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/JsonExceptionMapperTest.java b/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/JsonExceptionMapperTest.java index 3efbf85f..59ce0c1c 100644 --- a/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/JsonExceptionMapperTest.java +++ b/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/JsonExceptionMapperTest.java @@ -44,7 +44,7 @@ public class JsonExceptionMapperTest { Response resp = mapper.toResponse(ex); assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), resp.getStatus()); - assertEquals("{'errorDetails':'Invalid JSON request'}".replace('\'', '"'), + assertEquals("{'errorDetails':'Invalid request'}".replace('\'', '"'), new StandardCoder().encode(resp.getEntity())); } } diff --git a/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/RestServerTest.java b/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/RestServerTest.java index ee28b96d..cd40f012 100644 --- a/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/RestServerTest.java +++ b/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/RestServerTest.java @@ -20,6 +20,7 @@ package org.onap.policy.common.endpoints.http.server.test; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -34,6 +35,7 @@ import java.io.IOException; import java.io.PrintWriter; import java.net.HttpURLConnection; import java.net.URL; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Base64; import java.util.Properties; @@ -45,7 +47,7 @@ import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import lombok.Getter; -import org.junit.After; +import org.apache.commons.io.IOUtils; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -56,6 +58,7 @@ import org.onap.policy.common.endpoints.http.server.HttpServletServerFactory; import org.onap.policy.common.endpoints.http.server.JsonExceptionMapper; import org.onap.policy.common.endpoints.http.server.RestServer; import org.onap.policy.common.endpoints.http.server.RestServer.Factory; +import org.onap.policy.common.endpoints.http.server.YamlExceptionMapper; import org.onap.policy.common.endpoints.http.server.YamlMessageBodyHandler; import org.onap.policy.common.endpoints.http.server.aaf.AafAuthFilter; import org.onap.policy.common.endpoints.parameters.RestServerParameters; @@ -66,6 +69,7 @@ import org.onap.policy.common.utils.network.NetworkUtil; import org.powermock.reflect.Whitebox; public class RestServerTest { + private static final String APPLICATION_YAML = "application/yaml"; private static final String SERVER1 = "my-server-A"; private static final String SERVER2 = "my-server-B"; private static final String FACTORY_FIELD = "factory"; @@ -74,24 +78,56 @@ public class RestServerTest { private static final String PASS = "my-pass"; private static final Integer PORT = 9876; private static final String USER = "my-user"; + private static Factory saveFactory; + private static RestServer realRest; + private static int realPort; + private static RestServerParameters params; - private RestServer realRest; private RestServer rest; private HttpServletServer server1; private HttpServletServer server2; private Factory factory; private HttpServletServerFactory serverFactory; - private RestServerParameters params; + private String errorMsg; + /** + * Starts the REST server. + * @throws Exception if an error occurs + */ @BeforeClass - public static void setUpBeforeClass() { + public static void setUpBeforeClass() throws Exception { saveFactory = Whitebox.getInternalState(RestServer.class, FACTORY_FIELD); + + realPort = NetworkUtil.allocPort(); + + initRealParams(); + + realRest = new RestServer(params, null, RealProvider.class) { + @Override + protected Properties getServerProperties(RestServerParameters restServerParameters, String names) { + Properties props = super.getServerProperties(restServerParameters, names); + + String svcpfx = PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." + + restServerParameters.getName(); + props.setProperty(svcpfx + PolicyEndPointProperties.PROPERTY_HTTP_SWAGGER_SUFFIX, "false"); + + return props; + } + }; + + realRest.start(); + assertTrue(NetworkUtil.isTcpPortOpen(params.getHost(), params.getPort(), 100, 100)); } + /** + * Restores the factory and stops the REST server. + */ @AfterClass public static void tearDownAfterClass() { Whitebox.setInternalState(RestServer.class, FACTORY_FIELD, saveFactory); + + realRest.stop(); } /** @@ -103,7 +139,8 @@ public class RestServerTest { server2 = mock(HttpServletServer.class); factory = mock(Factory.class); serverFactory = mock(HttpServletServerFactory.class); - params = mock(RestServerParameters.class); + + initParams(); when(factory.getServerFactory()).thenReturn(serverFactory); when(serverFactory.build(any())).thenReturn(Arrays.asList(server1, server2)); @@ -111,27 +148,7 @@ public class RestServerTest { when(server1.getName()).thenReturn(SERVER1); when(server2.getName()).thenReturn(SERVER2); - when(params.getHost()).thenReturn(HOST); - when(params.getName()).thenReturn(PARAM_NAME); - when(params.getPassword()).thenReturn(PASS); - when(params.getPort()).thenReturn(PORT); - when(params.getUserName()).thenReturn(USER); - when(params.isAaf()).thenReturn(true); - when(params.isHttps()).thenReturn(true); - Whitebox.setInternalState(RestServer.class, FACTORY_FIELD, factory); - - realRest = null; - } - - /** - * Stops the rest server. - */ - @After - public void tearDown() { - if (realRest != null) { - realRest.stop(); - } } @Test @@ -213,41 +230,33 @@ public class RestServerTest { assertEquals("true", props.getProperty(svcpfx + PolicyEndPointProperties.PROPERTY_HTTP_HTTPS_SUFFIX)); assertEquals("true", props.getProperty(svcpfx + PolicyEndPointProperties.PROPERTY_AAF_SUFFIX)); assertEquals(String.join(",", GsonMessageBodyHandler.class.getName(), YamlMessageBodyHandler.class.getName(), - JsonExceptionMapper.class.getName()), + JsonExceptionMapper.class.getName(), YamlExceptionMapper.class.getName()), props.getProperty(svcpfx + PolicyEndPointProperties.PROPERTY_HTTP_SERIALIZATION_PROVIDER)); } @Test public void testInvalidJson() throws Exception { - when(params.getHost()).thenReturn("localhost"); - when(params.getPort()).thenReturn(NetworkUtil.allocPort()); - when(params.isHttps()).thenReturn(false); - when(params.isAaf()).thenReturn(false); - - // use real factory - Whitebox.setInternalState(RestServer.class, FACTORY_FIELD, saveFactory); - - realRest = new RestServer(params, null, RealProvider.class) { - @Override - protected Properties getServerProperties(RestServerParameters restServerParameters, String names) { - Properties props = super.getServerProperties(restServerParameters, names); - - String svcpfx = PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." - + restServerParameters.getName(); - props.setProperty(svcpfx + PolicyEndPointProperties.PROPERTY_HTTP_SWAGGER_SUFFIX, "false"); - - return props; - } - }; - - realRest.start(); - assertTrue(NetworkUtil.isTcpPortOpen(params.getHost(), params.getPort(), 100, 100)); + initRealParams(); assertEquals(200, roundTrip(new StandardCoder().encode(new MyRequest()))); assertEquals(400, roundTrip("{'bogus-json'")); + assertThat(errorMsg).contains("Invalid request"); + } + + @Test + public void testInvalidYaml() throws Exception { + initRealParams(); + + assertEquals(200, roundTrip(new StandardCoder().encode(new MyRequest()), APPLICATION_YAML)); + assertEquals(400, roundTrip("<bogus yaml", APPLICATION_YAML)); + assertThat(errorMsg).contains("Invalid request"); } private int roundTrip(String request) throws IOException { + return roundTrip(request, MediaType.APPLICATION_JSON); + } + + private int roundTrip(String request, String mediaType) throws IOException { URL url = new URL("http://" + params.getHost() + ":" + params.getPort() + "/request"); HttpURLConnection conn = (HttpURLConnection) url.openConnection(); conn.setDoInput(true); @@ -255,14 +264,22 @@ public class RestServerTest { conn.setRequestMethod("POST"); String auth = params.getUserName() + ":" + params.getPassword(); conn.setRequestProperty("Authorization", "Basic " + Base64.getEncoder().encodeToString(auth.getBytes())); - conn.setRequestProperty("Content-type", MediaType.APPLICATION_JSON); + conn.setRequestProperty("Content-type", mediaType); conn.connect(); try (PrintWriter wtr = new PrintWriter(conn.getOutputStream())) { wtr.write(request); } - return conn.getResponseCode(); + int code = conn.getResponseCode(); + + if (code == 200) { + errorMsg = ""; + } else { + errorMsg = IOUtils.toString(conn.getErrorStream(), StandardCharsets.UTF_8); + } + + return code; } @Test @@ -277,6 +294,27 @@ public class RestServerTest { assertNotNull(saveFactory.getServerFactory()); } + private static void initRealParams() { + initParams(); + + when(params.getHost()).thenReturn("localhost"); + when(params.getPort()).thenReturn(realPort); + when(params.isHttps()).thenReturn(false); + when(params.isAaf()).thenReturn(false); + } + + private static void initParams() { + params = mock(RestServerParameters.class); + + when(params.getHost()).thenReturn(HOST); + when(params.getName()).thenReturn(PARAM_NAME); + when(params.getPassword()).thenReturn(PASS); + when(params.getPort()).thenReturn(PORT); + when(params.getUserName()).thenReturn(USER); + when(params.isAaf()).thenReturn(true); + when(params.isHttps()).thenReturn(true); + } + private static class Filter extends AafAuthFilter { @Override protected String getPermissionType(HttpServletRequest request) { @@ -302,8 +340,8 @@ public class RestServerTest { } @Path("/") - @Produces(MediaType.APPLICATION_JSON) - @Consumes(MediaType.APPLICATION_JSON) + @Produces({MediaType.APPLICATION_JSON, APPLICATION_YAML}) + @Consumes({MediaType.APPLICATION_JSON, APPLICATION_YAML}) public static class RealProvider { @POST @Path("/request") diff --git a/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/YamlExceptionMapperTest.java b/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/YamlExceptionMapperTest.java new file mode 100644 index 00000000..96c23ed3 --- /dev/null +++ b/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/YamlExceptionMapperTest.java @@ -0,0 +1,50 @@ +/*- + * ============LICENSE_START======================================================= + * ONAP + * ================================================================================ + * Copyright (C) 2019 AT&T Intellectual Property. All rights reserved. + * ================================================================================ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ============LICENSE_END========================================================= + */ + +package org.onap.policy.common.endpoints.http.server.test; + +import static org.junit.Assert.assertEquals; + +import javax.ws.rs.core.Response; +import org.junit.Before; +import org.junit.Test; +import org.onap.policy.common.endpoints.http.server.YamlExceptionMapper; +import org.onap.policy.common.utils.coder.CoderException; +import org.onap.policy.common.utils.coder.StandardYamlCoder; +import org.yaml.snakeyaml.error.YAMLException; + +public class YamlExceptionMapperTest { + private YamlExceptionMapper mapper; + + @Before + public void setUp() { + mapper = new YamlExceptionMapper(); + } + + @Test + public void testToResponse() throws CoderException { + YAMLException ex = new YAMLException("expected exception"); + Response resp = mapper.toResponse(ex); + + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), resp.getStatus()); + assertEquals("errorDetails: Invalid request", + new StandardYamlCoder().encode(resp.getEntity()).trim()); + } +} diff --git a/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/YamlMessageBodyHandlerTest.java b/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/YamlMessageBodyHandlerTest.java index b8963e9b..0ddad0bf 100644 --- a/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/YamlMessageBodyHandlerTest.java +++ b/policy-endpoints/src/test/java/org/onap/policy/common/endpoints/http/server/test/YamlMessageBodyHandlerTest.java @@ -36,6 +36,7 @@ import javax.ws.rs.core.MediaType; import org.junit.Before; import org.junit.Test; import org.onap.policy.common.endpoints.http.server.YamlMessageBodyHandler; +import org.yaml.snakeyaml.error.YAMLException; public class YamlMessageBodyHandlerTest { private static final String EXPECTED_EXCEPTION = "expected exception"; @@ -168,7 +169,17 @@ public class YamlMessageBodyHandlerTest { }; assertThatThrownBy(() -> hdlr.readFrom(CLASS_OBJ, CLASS_OBJ, null, null, null, inpstr)) - .isInstanceOf(IOException.class); + .isInstanceOf(YAMLException.class); + + inpstr.close(); + } + + @Test + public void testReadFrom_Invalid() throws Exception { + InputStream inpstr = new ByteArrayInputStream("plain text".getBytes()); + + assertThatThrownBy(() -> hdlr.readFrom(CLASS_OBJ, CLASS_OBJ, null, null, null, inpstr)) + .isInstanceOf(YAMLException.class); inpstr.close(); } |