From 7adf07d9a455416c741e2ea7b379ce1293c7f2a6 Mon Sep 17 00:00:00 2001
From: Quentin Jaquier <quentin.jaquier@sonarsource.com>
Date: Mon, 17 Jun 2024 12:30:43 +0200
Subject: [PATCH] JS-185 Do not crash for protobuf exceptions (#4740)

---
 .../javascript/bridge/FormDataUtils.java      | 19 +++++++++++++--
 .../javascript/bridge/FormDataUtilsTest.java  | 24 ++++++++++++-------
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/FormDataUtils.java b/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/FormDataUtils.java
index b7903e897a8..c62cc49d6c1 100644
--- a/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/FormDataUtils.java
+++ b/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/FormDataUtils.java
@@ -19,15 +19,22 @@
  */
 package org.sonar.plugins.javascript.bridge;
 
+import com.google.protobuf.InvalidProtocolBufferException;
 import java.io.IOException;
 import java.net.http.HttpResponse;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
+import javax.annotation.CheckForNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.sonar.plugins.javascript.bridge.protobuf.Node;
 
 public class FormDataUtils {
+
+  private static final Logger LOG = LoggerFactory.getLogger(FormDataUtils.class);
+
   private FormDataUtils() {
     throw new IllegalStateException("Utility class");
   }
@@ -75,8 +82,16 @@ public static BridgeServer.BridgeResponse parseFormData(HttpResponse<byte[]> res
     }
   }
 
-  public static Node parseProtobuf(byte[] ast) throws IOException {
-    return Node.parseFrom(ast);
+  @CheckForNull
+  private static Node parseProtobuf(byte[] ast) throws IOException {
+    try {
+      return Node.parseFrom(ast);
+    } catch (InvalidProtocolBufferException e) {
+      // Failing to parse the protobuf message should not prevent the analysis from continuing.
+      // This also happen in the case of large recursion. See https://sonarsource.atlassian.net/browse/JS-185.
+      LOG.error("Failed to deserialize Protobuf message.", e);
+    }
+    return null;
   }
 
   private static int indexOf(byte[] array, byte[] pattern) {
diff --git a/sonar-plugin/bridge/src/test/java/org/sonar/plugins/javascript/bridge/FormDataUtilsTest.java b/sonar-plugin/bridge/src/test/java/org/sonar/plugins/javascript/bridge/FormDataUtilsTest.java
index a06783829ba..69a391fe51b 100644
--- a/sonar-plugin/bridge/src/test/java/org/sonar/plugins/javascript/bridge/FormDataUtilsTest.java
+++ b/sonar-plugin/bridge/src/test/java/org/sonar/plugins/javascript/bridge/FormDataUtilsTest.java
@@ -19,12 +19,6 @@
  */
 package org.sonar.plugins.javascript.bridge;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-import static org.sonar.plugins.javascript.bridge.FormDataUtils.parseFormData;
-
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
@@ -36,10 +30,22 @@
 import java.util.List;
 import javax.annotation.Nullable;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.slf4j.event.Level;
+import org.sonar.api.testfixtures.log.LogTesterJUnit5;
 import org.sonar.plugins.javascript.bridge.protobuf.Node;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.sonar.plugins.javascript.bridge.FormDataUtils.parseFormData;
+
 public class FormDataUtilsTest {
 
+  @RegisterExtension
+  public LogTesterJUnit5 logTester = new LogTesterJUnit5().setLevel(Level.ERROR);
+
   @Test
   void should_parse_form_data_into_bridge_response() throws Exception {
     HttpResponse<byte[]> mockResponse = mock(HttpResponse.class);
@@ -99,7 +105,7 @@ private static byte[] buildPayload(String jsonData, @Nullable byte[] protoData)
   }
 
   @Test
-  void should_throw_an_error_if_ast_is_invalid() throws Exception {
+  void should_log_error_if_ast_is_invalid() throws Exception {
     HttpResponse<byte[]> mockResponse = mock(HttpResponse.class);
     var values = new HashMap<String, List<String>>();
     values.put("Content-Type", List.of("multipart/form-data; boundary=---------------------------9051914041544843365972754266"));
@@ -107,8 +113,8 @@ void should_throw_an_error_if_ast_is_invalid() throws Exception {
     var invalidAst = new byte[]{42};
     var body = buildPayload("{\"hello\":\"worlds\"}", invalidAst);
     when(mockResponse.body()).thenReturn(body);
-    assertThatThrownBy(() -> parseFormData(mockResponse))
-      .isInstanceOf(IllegalStateException.class);
+    assertThat(parseFormData(mockResponse).ast()).isNull();
+    assertThat(logTester.logs(Level.ERROR)).containsExactly("Failed to deserialize Protobuf message.");
   }
 
   @Test