Skip to content

Commit

Permalink
JS-185 Do not crash for protobuf exceptions (#4740)
Browse files Browse the repository at this point in the history
  • Loading branch information
quentin-jaquier-sonarsource authored Jun 17, 2024
1 parent fd2dcd0 commit 7adf07d
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -99,16 +105,16 @@ 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"));
when(mockResponse.headers()).thenReturn(HttpHeaders.of(values, (_a, _b) -> true));
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
Expand Down

0 comments on commit 7adf07d

Please sign in to comment.