Fix NPE thrown by MultiValueMap endpoints (#63)

* Implement tests for the HECRestController

- Expects same behavior for sendEvents endpoints

* Initial design for MultiValueMapRequest

- Will replace RequestBodyCleaner

* Implement AcknowledgementResponse

- Aligns the response creation with event endpoints
- Changes the MultiValueMap parameters from raw types to "String, String"
- Sadly the ResponseEntities use the builder pattern
- Removes the old RequestBodyCleaner.java

* Add JMeter test for application/x-www-form-urlencoded events

* Update JavaDoc for MultiValueMapRequest

* Fix variable names in HECRestControllerTest

- "variable" renamed to "responseEntity"
- "jsonNodeResponseEntity" renamed to "expectedResponseEntity"

* Make sure that MultiValueMapRequest.asCleanedJsonString does not modify the underlying field

- Added unit test for this, tested before and after the copying of MultiValueMapRequest.multiValueMap

* Fix variable names in HECRestControllerTest

Author: MoonBow-1

src/main/java/com/teragrep/cfe_16/MultiValueMapRequest.java

@@ -0,0 +1,106 @@
+/*
+ * HTTP Event Capture to RFC5424 CFE_16
+ * Copyright (C) 2021-2025 Suomen Kanuuna Oy
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ *
+ * Additional permission under GNU Affero General Public License version 3
+ * section 7
+ *
+ * If you modify this Program, or any covered work, by linking or combining it
+ * with other code, such other code is not for that reason alone subject to any
+ * of the requirements of the GNU Affero GPL version 3 as long as this Program
+ * is the same Program as licensed from Suomen Kanuuna Oy without any additional
+ * modifications.
+ *
+ * Supplemented terms under GNU Affero General Public License version 3
+ * section 7
+ *
+ * Origin of the software must be attributed to Suomen Kanuuna Oy. Any modified
+ * versions must be marked as "Modified version of" The Program.
+ *
+ * Names of the licensors and authors may not be used for publicity purposes.
+ *
+ * No rights are granted for use of trade names, trademarks, or service marks
+ * which are in The Program if any.
+ *
+ * Licensee must indemnify licensors and authors for any liability that these
+ * contractual assumptions impose on licensors and authors.
+ *
+ * To the extent this program is licensed as part of the Commercial versions of
+ * Teragrep, the applicable Commercial License may apply to this file if you as
+ * a licensee so wish it.
+ */
+package com.teragrep.cfe_16;
+
+import java.util.Objects;
+import java.util.Set;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
+
+/**
+ * Cleans the request body so, that only the body of the request is left in the string. This is needed when calling the
+ * endpoint that consumes MediaType.APPLICATION_FORM_URLENCODED_VALUE <b>The actual body needed is stored as a key, and
+ * not a value for a key</b> Example of body sent as a parameter: {channel=[CHANNEL_11111], {"sourcetype":
+ * "mysourcetype", "event": "Hello, world!"}=[]}
+ */
+public final class MultiValueMapRequest {
+
+    private final MultiValueMap<String, String> multiValueMap;
+
+    public MultiValueMapRequest(final MultiValueMap<String, String> multiValueMap) {
+        this.multiValueMap = multiValueMap;
+    }
+
+    /**
+     * @return Example of cleaned body: {"sourcetype": "mysourcetype", "event": "Hello, world!"}
+     * @throws IllegalStateException if the MultiValueMap contains more than one key after the channel has been removed
+     */
+    public String asCleanedJsonString() throws IllegalStateException {
+        final String valueToReturn;
+        //
+        final MultiValueMap<String, String> copyOfMultiValueMap = new LinkedMultiValueMap<>(multiValueMap);
+        // Remove the channel, if it is present
+        copyOfMultiValueMap.remove("channel");
+        final Set<String> keys = copyOfMultiValueMap.keySet();
+
+        // Check if the parameters contains more entries than expected from the request
+        if (keys.size() != 1) {
+            throw new IllegalStateException(
+                    "application/x-www-form-urlencoded request contains more parameters than expected"
+            );
+        }
+        else {
+            // Get the value of the first key
+            valueToReturn = keys.iterator().next();
+        }
+
+        return valueToReturn;
+    }
+
+    @Override
+    public boolean equals(final Object o) {
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        final MultiValueMapRequest that = (MultiValueMapRequest) o;
+        return Objects.equals(multiValueMap, that.multiValueMap);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hashCode(multiValueMap);
+    }
+}

…/teragrep/cfe_16/RequestBodyCleaner.java …16/response/AcknowledgementResponse.javasrc/main/java/com/teragrep/cfe_16/RequestBodyCleaner.java renamed to src/main/java/com/teragrep/cfe_16/response/AcknowledgementResponse.java src/main/java/com/teragrep/cfe_16/RequestBodyCleaner.java renamed to src/main/java/com/teragrep/cfe_16/response/AcknowledgementResponse.java

@@ -43,39 +43,47 @@
  * Teragrep, the applicable Commercial License may apply to this file if you as
  * a licensee so wish it.
  */
-package com.teragrep.cfe_16;
+package com.teragrep.cfe_16.response;
 
-import org.springframework.stereotype.Component;
+import java.util.Objects;
+import org.springframework.http.MediaType;
+import org.springframework.http.ResponseEntity;
+import tools.jackson.databind.JsonNode;
+import tools.jackson.databind.ObjectMapper;
+import tools.jackson.databind.node.ObjectNode;
 
-import java.util.regex.Pattern;
+public final class AcknowledgementResponse implements Response {
 
-/*
- * Cleans the request body so, that only the body of the request is left in the string.
- * This is needed when calling the endpoint that consumes MediaType.APPLICATION_FORM_URLENCODED_VALUE
- * Example of body sent as a parameter: {channel=[CHANNEL_11111], {"sourcetype": "mysourcetype", "event": "Hello, world!"}=[]}
- * Example of cleaned body returned by the cleanAckRequestBody(): {"sourcetype": "mysourcetype", "event": "Hello, world!"}
- * TODO: Try to implement a better way to get the body of the request.
- *
- */
-@Component
-public class RequestBodyCleaner {
+    /**
+     * This field is expected to be in a format provided by
+     * {@link com.teragrep.cfe_16.Acknowledgements#getRequestedAckStatuses(String, String, JsonNode)}
+     */
+    private final JsonNode acknowledgementsJsonNode;
 
-    public String cleanAckRequestBody(String body, String channel) {
-        String bodyWithoutChannel = body.replaceAll("channel=\\[" + Pattern.quote(channel) + "\\]\\, ", "");
-        String bodywithoutChannelLastCharRemoved = removeLastChar(bodyWithoutChannel);
-
-        String cleanedBody = bodywithoutChannelLastCharRemoved.substring(1);
+    public AcknowledgementResponse(final JsonNode acknowledgementsJsonNode) {
+        this.acknowledgementsJsonNode = acknowledgementsJsonNode;
+    }
 
-        cleanedBody = removeEqualsArrayFromEnd(cleanedBody);
+    @Override
+    public ResponseEntity<JsonNode> asJsonNodeResponseEntity() {
+        final ObjectMapper objectMapper = new ObjectMapper();
+        final ObjectNode jsonNode = objectMapper.createObjectNode();
+        jsonNode.set("acks", this.acknowledgementsJsonNode);
 
-        return cleanedBody;
+        return ResponseEntity.ok().contentType(MediaType.APPLICATION_JSON).body(jsonNode);
     }
 
-    private String removeLastChar(String str) {
-        return str.substring(0, str.length() - 1);
+    @Override
+    public boolean equals(final Object o) {
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        final AcknowledgementResponse that = (AcknowledgementResponse) o;
+        return Objects.equals(acknowledgementsJsonNode, that.acknowledgementsJsonNode);
     }
 
-    private String removeEqualsArrayFromEnd(String str) {
-        return str.replace("=[]", "");
+    @Override
+    public int hashCode() {
+        return Objects.hashCode(acknowledgementsJsonNode);
     }
 }

src/main/java/com/teragrep/cfe_16/rest/HECRestController.java

@@ -45,9 +45,16 @@
  */
 package com.teragrep.cfe_16.rest;
 
+import com.teragrep.cfe_16.MultiValueMapRequest;
+import com.teragrep.cfe_16.bo.HeaderInfo;
+import com.teragrep.cfe_16.response.ExceptionEvent;
+import com.teragrep.cfe_16.response.ExceptionEventContext;
+import com.teragrep.cfe_16.response.ExceptionJsonResponse;
+import com.teragrep.cfe_16.response.JsonResponse;
+import java.util.UUID;
+import tools.jackson.core.JacksonException;
 import tools.jackson.databind.JsonNode;
 import tools.jackson.databind.ObjectMapper;
-import com.teragrep.cfe_16.RequestBodyCleaner;
 import com.teragrep.cfe_16.config.Configuration;
 import com.teragrep.cfe_16.response.Response;
 import com.teragrep.cfe_16.service.HECService;
@@ -69,37 +76,52 @@ public class HECRestController {
     @Autowired
     private HECService service;
 
-    @Autowired
-    private RequestBodyCleaner requestBodyCleaner;
-
     @Autowired
     private Configuration configuration;
 
-    @SuppressWarnings("rawtypes")
     @RequestMapping(
             value = "services/collector",
             method = RequestMethod.POST,
             consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE,
             produces = MediaType.APPLICATION_JSON_VALUE
     )
     public ResponseEntity<JsonNode> sendEvents(
-            HttpServletRequest request,
-            @RequestBody MultiValueMap body,
-            @RequestParam(required = false) String channel
+            final HttpServletRequest request,
+            @RequestBody final MultiValueMap<String, String> body,
+            @RequestParam(required = false) final String channel
     ) {
-        // TODO: Try to think an alternative way to implement getting the body of the
-        // call
-        final String eventInJson = requestBodyCleaner.cleanAckRequestBody(body.toString(), channel);
-
-        long t1 = System.nanoTime();
-        final Response response = service.sendEvents(request, channel, eventInJson);
-        long t2 = System.nanoTime();
-        long dt = t2 - t1;
-        double us = (double) dt / 1000.0;
-        if (this.configuration.getPrintTimes()) {
-            LOGGER.info("sendEvents took <{}> nanoseconds, that is <{}> microseconds", dt, us);
+        ResponseEntity<JsonNode> responseEntity;
+        try {
+            final MultiValueMapRequest eventInJson = new MultiValueMapRequest(body);
+            final long t1 = System.nanoTime();
+            final Response response = service.sendEvents(request, channel, eventInJson.asCleanedJsonString());
+            final long t2 = System.nanoTime();
+            final long dt = t2 - t1;
+            final double us = (double) dt / 1000.0;
+            if (this.configuration.getPrintTimes()) {
+                LOGGER.info("sendEvents took <{}> nanoseconds, that is <{}> microseconds", dt, us);
+            }
+            responseEntity = response.asJsonNodeResponseEntity();
         }
-        return response.asJsonNodeResponseEntity();
+        catch (final IllegalStateException illegalStateException) {
+            final HeaderInfo headerInfo = new HeaderInfo(request);
+            final ExceptionEventContext exceptionEventContext = new ExceptionEventContext(
+                    headerInfo,
+                    request.getHeader("user-agent"),
+                    request.getRequestURI(),
+                    request.getRemoteHost()
+            );
+            final ExceptionEvent event = new ExceptionEvent(
+                    exceptionEventContext,
+                    UUID.randomUUID(),
+                    illegalStateException
+            );
+            event.logException();
+            final Response response = new ExceptionJsonResponse(event);
+            responseEntity = response.asJsonNodeResponseEntity();
+        }
+
+        return responseEntity;
     }
 
     @RequestMapping(
@@ -132,79 +154,84 @@ public ResponseEntity<JsonNode> sendEvents(
             },
             consumes = MediaType.APPLICATION_JSON_VALUE
     )
-    public JsonNode getAcksWithPostMethod(
+    public ResponseEntity<JsonNode> getAcksWithPostMethod(
             @RequestBody JsonNode requestedAcksInJson,
             HttpServletRequest request,
             @RequestParam(required = false) String channel
     ) {
 
         long t1 = System.nanoTime();
-        JsonNode response = service.getAcks(request, channel, requestedAcksInJson);
+        final Response response = service.getAcks(request, channel, requestedAcksInJson);
         long t2 = System.nanoTime();
         long dt = t2 - t1;
         double us = (double) dt / 1000.0;
         if (this.configuration.getPrintTimes()) {
             LOGGER.info("getAcks took <{}> nanoseconds, that is <{}> microseconds", dt, us);
         }
-        return response;
+        return response.asJsonNodeResponseEntity();
     }
 
-    // @LogAnnotation(type = LogType.METRIC_DURATION)
-    @SuppressWarnings("rawtypes")
     @RequestMapping(
             value = "services/collector/ack",
             method = {
                     RequestMethod.POST, RequestMethod.GET
             },
             consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE
     )
-    public @ResponseBody JsonNode getAcks(
-            @RequestBody MultiValueMap body,
-            HttpServletRequest request,
-            @RequestParam(required = false) String channel
+    public ResponseEntity<JsonNode> getAcks(
+            @RequestBody final MultiValueMap<String, String> body,
+            final HttpServletRequest request,
+            @RequestParam(required = false) final String channel
     ) {
-        // TODO: Try to think an alternative way to implement getting the body of the
-        // call
-        String bodyString = requestBodyCleaner.cleanAckRequestBody(body.toString(), channel);
+        ResponseEntity<JsonNode> responseEntity;
 
-        JsonNode requestedAcksInJson = null;
         try {
-            requestedAcksInJson = objectMapper.readValue(bodyString, JsonNode.class);
+            final MultiValueMapRequest multiValueMapRequest = new MultiValueMapRequest(body);
+
+            final JsonNode requestedAcksInJson = objectMapper
+                    .readValue(multiValueMapRequest.asCleanedJsonString(), JsonNode.class);
+
+            final long t1 = System.nanoTime();
+            final Response response = service.getAcks(request, channel, requestedAcksInJson);
+            final long t2 = System.nanoTime();
+            final long dt = t2 - t1;
+            final double us = (double) dt / 1000.0;
+            if (this.configuration.getPrintTimes()) {
+                LOGGER.info("getAcks took <{}> nanoseconds, that is <{}> microseconds", dt, us);
+            }
+            responseEntity = new JsonResponse(response.toString()).asJsonNodeResponseEntity();
         }
-        catch (Exception e) {
-            // TODO: handle the error in a proper way
-            LOGGER.warn("Failed to handle response: ", e);
+        catch (final IllegalStateException | JacksonException exception) {
+            final HeaderInfo headerInfo = new HeaderInfo(request);
+            final ExceptionEventContext exceptionEventContext = new ExceptionEventContext(
+                    headerInfo,
+                    request.getHeader("user-agent"),
+                    request.getRequestURI(),
+                    request.getRemoteHost()
+            );
+            final ExceptionEvent event = new ExceptionEvent(exceptionEventContext, UUID.randomUUID(), exception);
+            event.logException();
+            final Response response = new ExceptionJsonResponse(event);
+            responseEntity = response.asJsonNodeResponseEntity();
         }
 
-        long t1 = System.nanoTime();
-        JsonNode response = service.getAcks(request, channel, requestedAcksInJson);
-        long t2 = System.nanoTime();
-        long dt = t2 - t1;
-        double us = (double) dt / 1000.0;
-        if (this.configuration.getPrintTimes()) {
-            LOGGER.info("getAcks took <{}> nanoseconds, that is <{}> microseconds", dt, us);
-        }
-        return response;
+        return responseEntity;
     }
 
-    @SuppressWarnings("rawtypes")
     @RequestMapping(
             value = "services/collector/event",
             method = RequestMethod.POST,
             consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE
     )
     public ResponseEntity<JsonNode> sendEventsWithFormatOption(
-            HttpServletRequest request,
-            @RequestBody MultiValueMap body,
-            @RequestParam(required = false) String channel
+            final HttpServletRequest request,
+            @RequestBody final MultiValueMap<String, String> body,
+            @RequestParam(required = false) final String channel
     ) {
-
-        // TODO: Try to think an alternative way to implement getting the body of the
-        // call
-        String eventInJson = requestBodyCleaner.cleanAckRequestBody(body.toString(), channel);
+        final MultiValueMapRequest multiValueMapRequest = new MultiValueMapRequest(body);
 
         long t1 = System.nanoTime();
-        final Response response = service.sendEvents(request, channel, eventInJson);
+        final Response response = service.sendEvents(request, channel, multiValueMapRequest.asCleanedJsonString());
         long t2 = System.nanoTime();
         long dt = t2 - t1;
         double us = (double) dt / 1000.0;

src/main/java/com/teragrep/cfe_16/service/HECService.java

@@ -71,7 +71,7 @@ public interface HECService {
      * @param requestedAcksInJson
      * @return
      */
-    public JsonNode getAcks(HttpServletRequest request, String channel, JsonNode requestedAcksInJson);
+    public Response getAcks(HttpServletRequest request, String channel, JsonNode requestedAcksInJson);
 
     /**
      * Ping.