Escape double quotes in all strings, not just top level model strings

changelog/unreleased/pr-19951.toml

@@ -1,4 +1,4 @@
 type = "fixed"
 message = "Fixed issue where unescaped quotes in Custom HTTP notification JSON payloads breaks the notifications."
 
-pulls = ["19951"]
+pulls = ["19951", "20318"]

graylog2-server/src/main/java/org/graylog/events/notifications/types/HTTPEventNotificationV2.java

@@ -19,9 +19,11 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.floreysoft.jmte.Engine;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import jakarta.inject.Inject;
+import jakarta.inject.Named;
 import okhttp3.HttpUrl;
 import okhttp3.MediaType;
 import okhttp3.OkHttpClient;
@@ -54,7 +56,6 @@
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.stream.Collectors;
 
@@ -97,6 +98,7 @@ public interface Factory extends EventNotification.Factory<HTTPEventNotification
     private final EventsConfigurationProvider configurationProvider;
     private final ParameterizedHttpClientProvider parameterizedHttpClientProvider;
     private final Engine templateEngine;
+    private final Engine jsonTemplateEngine;
     private final NotificationService notificationService;
     private final NodeId nodeId;
 
@@ -107,6 +109,7 @@ public HTTPEventNotificationV2(EventNotificationService notificationCallbackServ
                                    EncryptedValueService encryptedValueService,
                                    EventsConfigurationProvider configurationProvider,
                                    Engine templateEngine,
+                                   @Named("JsonSafe") Engine jsonTemplateEngine,
                                    NotificationService notificationService,
                                    NodeId nodeId,
                                    final ParameterizedHttpClientProvider parameterizedHttpClientProvider) {
@@ -116,6 +119,7 @@ public HTTPEventNotificationV2(EventNotificationService notificationCallbackServ
         this.configurationProvider = configurationProvider;
         this.parameterizedHttpClientProvider = parameterizedHttpClientProvider;
         this.templateEngine = templateEngine;
+        this.jsonTemplateEngine = jsonTemplateEngine;
         this.notificationService = notificationService;
         this.nodeId = nodeId;
     }
@@ -215,28 +219,7 @@ private String buildRequestBody(EventNotificationModelData modelData, HTTPEventN
         final ObjectMapper objectMapper = objectMapperProvider.getForTimeZone(config.timeZone());
         final Map<String, Object> modelMap = objectMapper.convertValue(modelData, TypeReferences.MAP_STRING_OBJECT);
         if (!Strings.isNullOrEmpty(bodyTemplate)) {
-            if (config.contentType().equals(HTTPEventNotificationConfigV2.ContentType.FORM_DATA)) {
-                final String[] parts = bodyTemplate.split("&");
-                body = Arrays.stream(parts)
-                        .map(part -> {
-                            final int equalsIndex = part.indexOf("=");
-                            final String encodedKey = urlEncode(part.substring(0, equalsIndex));
-                            final String encodedValue = equalsIndex < part.length() - 1 ?
-                                    urlEncode(templateEngine.transform(part.substring(equalsIndex + 1), modelMap)) : "";
-                            return encodedKey + "=" + encodedValue;
-                        })
-                        .collect(Collectors.joining("&"));
-            } else {
-                Map<String, Object> escapedModelMap = new HashMap<>();
-                modelMap.forEach((k, v) -> {
-                    if (v instanceof String str) {
-                        escapedModelMap.put(k, str.replace("\"", "\\\""));
-                    } else {
-                        escapedModelMap.put(k, v);
-                    }
-                });
-                body = templateEngine.transform(bodyTemplate, escapedModelMap);
-            }
+            body = transformBody(bodyTemplate, config.contentType(), modelMap);
         } else {
             if (config.contentType().equals(HTTPEventNotificationConfigV2.ContentType.FORM_DATA)) {
                 final Map<String, Object> eventMap = objectMapper.convertValue(modelData.event(), TypeReferences.MAP_STRING_OBJECT);
@@ -248,6 +231,29 @@ private String buildRequestBody(EventNotificationModelData modelData, HTTPEventN
         return body;
     }
 
+    @VisibleForTesting
+    String transformBody(String bodyTemplate, HTTPEventNotificationConfigV2.ContentType contentType, Map<String, Object> modelMap) {
+        final String body;
+        if (contentType.equals(HTTPEventNotificationConfigV2.ContentType.FORM_DATA)) {
+            final String[] parts = bodyTemplate.split("&");
+            body = Arrays.stream(parts)
+                    .map(part -> {
+                        final int equalsIndex = part.indexOf("=");
+                        final String encodedKey = urlEncode(part.substring(0, equalsIndex));
+                        final String encodedValue = equalsIndex < part.length() - 1 ?
+                                urlEncode(templateEngine.transform(part.substring(equalsIndex + 1), modelMap)) : "";
+                        return encodedKey + "=" + encodedValue;
+                    })
+                    .collect(Collectors.joining("&"));
+        } else if (contentType.equals(HTTPEventNotificationConfigV2.ContentType.JSON)) {
+            body = jsonTemplateEngine.transform(bodyTemplate, modelMap);
+        } else {
+            body = templateEngine.transform(bodyTemplate, modelMap);
+        }
+
+        return body;
+    }
+
     private String getUrlEncodedEvent(Map<String, Object> modelMap, Map<String, Object> eventMap) {
         return StringUtils.chop(urlEncodedKeyValue(FIELD_EVENT_DEFINITION_ID, modelMap.get(FIELD_EVENT_DEFINITION_ID)) +
                 urlEncodedKeyValue(FIELD_EVENT_DEFINITION_TYPE, modelMap.get(FIELD_EVENT_DEFINITION_TYPE)) +

graylog2-server/src/main/java/org/graylog2/bindings/ServerBindings.java

@@ -35,6 +35,7 @@
 import org.graylog2.bindings.providers.DefaultSecurityManagerProvider;
 import org.graylog2.bindings.providers.DefaultStreamProvider;
 import org.graylog2.bindings.providers.HtmlSafeJmteEngineProvider;
+import org.graylog2.bindings.providers.JsonSafeEngineProvider;
 import org.graylog2.bindings.providers.SecureFreemarkerConfigProvider;
 import org.graylog2.bindings.providers.SystemJobFactoryProvider;
 import org.graylog2.bindings.providers.SystemJobManagerProvider;
@@ -191,6 +192,7 @@ private void bindSingletons() {
         bind(GrokPatternRegistry.class).in(Scopes.SINGLETON);
         bind(Engine.class).toInstance(Engine.createEngine());
         bind(Engine.class).annotatedWith(Names.named("HtmlSafe")).toProvider(HtmlSafeJmteEngineProvider.class).asEagerSingleton();
+        bind(Engine.class).annotatedWith(Names.named("JsonSafe")).toProvider(JsonSafeEngineProvider.class).asEagerSingleton();
         bind(ErrorPageGenerator.class).to(GraylogErrorPageGenerator.class).asEagerSingleton();
     }
 

graylog2-server/src/main/java/org/graylog2/bindings/providers/JsonSafeEngineProvider.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2020 Graylog, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the Server Side Public License, version 1,
+ * as published by MongoDB, Inc.
+ *
+ * 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
+ * Server Side Public License for more details.
+ *
+ * You should have received a copy of the Server Side Public License
+ * along with this program. If not, see
+ * <http://www.mongodb.com/licensing/server-side-public-license>.
+ */
+package org.graylog2.bindings.providers;
+
+import com.floreysoft.jmte.Engine;
+import com.floreysoft.jmte.Renderer;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.inject.Singleton;
+import org.apache.commons.lang.StringEscapeUtils;
+
+import java.util.Locale;
+import java.util.Map;
+
+@Singleton
+public class JsonSafeEngineProvider implements Provider<Engine> {
+    private final Engine engine;
+
+    @Inject
+    public JsonSafeEngineProvider() {
+        engine = Engine.createEngine();
+        engine.registerRenderer(String.class, new JsonSafeRenderer());
+    }
+    @Override
+    public Engine get() {
+        return engine;
+    }
+
+    private static class JsonSafeRenderer implements Renderer<String> {
+
+        @Override
+        public String render(String s, Locale locale, Map<String, Object> map) {
+            // Current version of Apache Commons does not have native support for escapeJson. However,
+            // https://commons.apache.org/proper/commons-text/javadocs/api-release/org/apache/commons/text/StringEscapeUtils.html#escapeJson(java.lang.String)
+            // current Apache Commons docs states:
+            // 'The only difference between Java strings and Json strings is that in Json, forward-slash (/) is escaped.'
+            // So we use escapeJava and tack on an extra String.replace() call to escape forward slashes.
+            return StringEscapeUtils.escapeJava(s).replace("/", "\\/");
+        }
+    }
+}

graylog2-server/src/test/java/org/graylog/events/notifications/types/HTTPEventNotificationV2Test.java

@@ -0,0 +1,94 @@
+/*
+ * Copyright (C) 2020 Graylog, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the Server Side Public License, version 1,
+ * as published by MongoDB, Inc.
+ *
+ * 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
+ * Server Side Public License for more details.
+ *
+ * You should have received a copy of the Server Side Public License
+ * along with this program. If not, see
+ * <http://www.mongodb.com/licensing/server-side-public-license>.
+ */
+package org.graylog.events.notifications.types;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.floreysoft.jmte.Engine;
+import com.google.common.collect.ImmutableList;
+import org.graylog.events.configuration.EventsConfigurationProvider;
+import org.graylog.events.notifications.EventNotificationService;
+import org.graylog2.bindings.providers.JsonSafeEngineProvider;
+import org.graylog2.notifications.NotificationService;
+import org.graylog2.plugin.Message;
+import org.graylog2.plugin.MessageSummary;
+import org.graylog2.plugin.TestMessageFactory;
+import org.graylog2.plugin.system.NodeId;
+import org.graylog2.security.encryption.EncryptedValueService;
+import org.graylog2.shared.bindings.providers.ObjectMapperProvider;
+import org.graylog2.shared.bindings.providers.ParameterizedHttpClientProvider;
+import org.graylog2.system.urlwhitelist.UrlWhitelistNotificationService;
+import org.graylog2.system.urlwhitelist.UrlWhitelistService;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mock;
+
+import java.io.UnsupportedEncodingException;
+import java.util.Map;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class HTTPEventNotificationV2Test {
+    @Mock
+    private EventNotificationService notificationCallbackService;
+    @Mock
+    private ObjectMapperProvider objectMapperProvider;
+    @Mock
+    private UrlWhitelistService whitelistService;
+    @Mock
+    private UrlWhitelistNotificationService urlWhitelistNotificationService;
+    @Mock
+    private EncryptedValueService encryptedValueService;
+    @Mock
+    private EventsConfigurationProvider configurationProvider;
+    @Mock
+    private ParameterizedHttpClientProvider parameterizedHttpClientProvider;
+    @Mock
+    private NotificationService notificationService;
+    @Mock
+    private NodeId nodeId;
+
+    private HTTPEventNotificationV2 notification;
+
+    @BeforeEach
+    void setUp() {
+        notification = new HTTPEventNotificationV2(notificationCallbackService, objectMapperProvider,
+                whitelistService, urlWhitelistNotificationService, encryptedValueService, configurationProvider,
+                new Engine(), new JsonSafeEngineProvider().get(), notificationService, nodeId,
+                parameterizedHttpClientProvider);
+    }
+
+    @Test
+    public void testEscapedQuotesInBacklog() throws UnsupportedEncodingException, JsonProcessingException {
+        Map<String, Object> model = Map.of(
+                "event_definition_title", "<<Test Event Title>>",
+                "event", Map.of("message", "Event Message & Whatnot"),
+                "backlog", createBacklog()
+        );
+        String bodyTemplate = "${if backlog}{\"backlog\": [${foreach backlog message}{ \"title\": \"Message\", \"value\": \"${message.message}\" }${if last_message}${else},${end}${end}]}${end}";
+        String body = notification.transformBody(bodyTemplate, HTTPEventNotificationConfigV2.ContentType.JSON, model);
+        assertThat(body).contains("\"value\": \"Message with \\\"Double Quotes\\\"");
+    }
+
+    private ImmutableList<MessageSummary> createBacklog() {
+        Message message = new TestMessageFactory().createMessage("Message with \"Double Quotes\"", "Unit Test", DateTime.now(DateTimeZone.UTC));
+        MessageSummary summary = new MessageSummary("index1", message);
+        return ImmutableList.of(summary);
+    }
+
+}