Report bad-request for invalid YAML

Added classes and modified code to report bad-request when a servlet
attempts to read invalid YAML.

Change-Id: Iacddee92a448fb69d5c778a3c3f3f2b5528983f7
Issue-ID: POLICY-2085
Signed-off-by: Jim Hahn <jrh3@att.com>

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();
     }