sonar/sonarqube-community-branch-plugin
1
0
Fork 0
mirror of https://github.com/mc1arke/sonarqube-community-branch-plugin.git synced 2025-02-21 19:20:09 +02:00
Code Issues Releases Activity

#445: Specify encoding when creating Gitlab decoration requests

Browse Source 
When calling the Gitlab API with content that contains accented or cyrillic characters, the API returns an error response and fails to complete the decoration. Explicitly setting the encoding for all body content to UTF-8 on Gitlab API requests results in the content being encoded in a way for the Gitlab API handles correctly.
This commit is contained in:
Michael Clarke 2021-10-09 19:20:36 +01:00 committed by Michael Clarke
parent caf0eeec84
commit 8acdd69ef9
6 changed files with 114 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/azuredevops/AzureDevopsRestClient.java
View File

@ -35,7 +35,6 @@ import org.apache.http.client.methods.RequestBuilder;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;
import org.apache.http.util.EntityUtils;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
@ -59,10 +58,6 @@ public class AzureDevopsRestClient implements AzureDevopsClient {
private final ObjectMapper objectMapper;
private final Supplier<CloseableHttpClient> httpClientFactory;
public AzureDevopsRestClient(String apiUrl, String authToken, ObjectMapper objectMapper) {
this(apiUrl, authToken, objectMapper, HttpClients::createSystem);
}
AzureDevopsRestClient(String apiUrl, String authToken, ObjectMapper objectMapper, Supplier<CloseableHttpClient> httpClientFactory) {
super();
this.apiUrl = apiUrl;

3
src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/azuredevops/DefaultAzureDevopsClientFactory.java
View File

@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.lang3.StringUtils;
import org.apache.http.impl.client.HttpClients;
import org.sonar.api.ce.ComputeEngineSide;
import org.sonar.api.config.internal.Settings;
import org.sonar.api.server.ServerSide;
@ -54,6 +55,6 @@ public class DefaultAzureDevopsClientFactory implements AzureDevopsClientFactory
public AzureDevopsClient createClient(ProjectAlmSettingDto projectAlmSettingDto, AlmSettingDto almSettingDto) {
String apiUrl = Optional.ofNullable(almSettingDto.getUrl()).map(StringUtils::trimToNull).orElseThrow(() -> new IllegalStateException("ALM URL must be provided"));
String accessToken = Optional.ofNullable(almSettingDto.getDecryptedPersonalAccessToken(settings.getEncryption())).map(StringUtils::trimToNull).orElseThrow(() -> new IllegalStateException("Personal Access Token must be provided"));
return new AzureDevopsRestClient(apiUrl, Base64.getEncoder().encodeToString((":" + accessToken).getBytes(StandardCharsets.UTF_8)), objectMapper);
return new AzureDevopsRestClient(apiUrl, Base64.getEncoder().encodeToString((":" + accessToken).getBytes(StandardCharsets.UTF_8)), objectMapper, HttpClients::createSystem);
}
}

3
src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/DefaultGitlabClientFactory.java
View File

@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.mc1arke.sonarqube.plugin.InvalidConfigurationException;
import com.github.mc1arke.sonarqube.plugin.almclient.LinkHeaderReader;
import org.apache.commons.lang.StringUtils;
import org.apache.http.impl.client.HttpClients;
import org.sonar.api.ce.ComputeEngineSide;
import org.sonar.api.config.internal.Settings;
import org.sonar.api.server.ServerSide;
@ -55,6 +56,6 @@ public class DefaultGitlabClientFactory implements GitlabClientFactory {
.orElseThrow(() -> new InvalidConfigurationException(InvalidConfigurationException.Scope.GLOBAL, "ALM URL must be specified"));
String apiToken = almSettingDto.getDecryptedPersonalAccessToken(settings.getEncryption());
return new GitlabRestClient(apiURL, apiToken, linkHeaderReader, objectMapper);
return new GitlabRestClient(apiURL, apiToken, linkHeaderReader, objectMapper, HttpClients::createSystem);
}
}

35
src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/GitlabRestClient.java
View File

@ -37,7 +37,6 @@ import org.apache.http.client.methods.HttpPut;
import org.apache.http.client.methods.HttpRequestBase;
import org.apache.http.entity.ContentType;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;
import org.apache.http.message.BasicNameValuePair;
import org.apache.http.util.EntityUtils;
import org.sonar.api.utils.log.Logger;
@ -52,6 +51,7 @@ import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Supplier;
class GitlabRestClient implements GitlabClient {
@ -61,12 +61,14 @@ class GitlabRestClient implements GitlabClient {
private final String authToken;
private final ObjectMapper objectMapper;
private final LinkHeaderReader linkHeaderReader;
private final Supplier<CloseableHttpClient> httpClientFactory;
GitlabRestClient(String baseGitlabApiUrl, String authToken, LinkHeaderReader linkHeaderReader, ObjectMapper objectMapper) {
GitlabRestClient(String baseGitlabApiUrl, String authToken, LinkHeaderReader linkHeaderReader, ObjectMapper objectMapper, Supplier<CloseableHttpClient> httpClientFactory) {
this.baseGitlabApiUrl = baseGitlabApiUrl;
this.authToken = authToken;
this.linkHeaderReader = linkHeaderReader;
this.objectMapper = objectMapper;
this.httpClientFactory = httpClientFactory;
}
@Override
@ -111,7 +113,7 @@ class GitlabRestClient implements GitlabClient {
HttpPost httpPost = new HttpPost(targetUrl);
httpPost.addHeader("Content-type", ContentType.APPLICATION_FORM_URLENCODED.getMimeType());
httpPost.setEntity(new UrlEncodedFormEntity(requestContent));
httpPost.setEntity(new UrlEncodedFormEntity(requestContent, StandardCharsets.UTF_8));
return entity(httpPost, Discussion.class, httpResponse -> validateResponse(httpResponse, 201, "Discussion successfully created"));
}
@ -120,7 +122,7 @@ class GitlabRestClient implements GitlabClient {
String targetUrl = String.format("%s/projects/%s/merge_requests/%s/discussions/%s/notes", baseGitlabApiUrl, projectId, mergeRequestIid, discussionId);
HttpPost httpPost = new HttpPost(targetUrl);
httpPost.setEntity(new UrlEncodedFormEntity(Collections.singletonList(new BasicNameValuePair("body", noteContent))));
httpPost.setEntity(new UrlEncodedFormEntity(Collections.singletonList(new BasicNameValuePair("body", noteContent)), StandardCharsets.UTF_8));
entity(httpPost, null, httpResponse -> validateResponse(httpResponse, 201, "Commit discussions note added"));
}
@ -146,7 +148,7 @@ class GitlabRestClient implements GitlabClient {
HttpPost httpPost = new HttpPost(statusUrl);
httpPost.addHeader("Content-type", ContentType.APPLICATION_FORM_URLENCODED.getMimeType());
httpPost.setEntity(new UrlEncodedFormEntity(entityFields));
httpPost.setEntity(new UrlEncodedFormEntity(entityFields, StandardCharsets.UTF_8));
entity(httpPost, null, httpResponse -> {
if (httpResponse.toString().contains("Cannot transition status")) {
// Workaround for https://gitlab.com/gitlab-org/gitlab-ce/issues/25807
@ -169,7 +171,7 @@ class GitlabRestClient implements GitlabClient {
private <X> X entity(HttpRequestBase httpRequest, Class<X> type, Consumer<HttpResponse> responseValidator) throws IOException {
httpRequest.addHeader("PRIVATE-TOKEN", authToken);
try (CloseableHttpClient httpClient = HttpClients.createSystem()) {
try (CloseableHttpClient httpClient = httpClientFactory.get()) {
HttpResponse httpResponse = httpClient.execute(httpRequest);
responseValidator.accept(httpResponse);
@ -188,7 +190,7 @@ class GitlabRestClient implements GitlabClient {
private <X> List<X> entities(HttpGet httpRequest, Class<X> type, Consumer<HttpResponse> responseValidator) throws IOException {
httpRequest.addHeader("PRIVATE-TOKEN", authToken);
try (CloseableHttpClient httpClient = HttpClients.createSystem()) {
try (CloseableHttpClient httpClient = httpClientFactory.get()) {
HttpResponse httpResponse = httpClient.execute(httpRequest);
responseValidator.accept(httpResponse);
@ -209,21 +211,22 @@ class GitlabRestClient implements GitlabClient {
private static void validateResponse(HttpResponse httpResponse, int expectedStatus, String successLogMessage) {
if (httpResponse.getStatusLine().getStatusCode() == expectedStatus) {
LOGGER.debug(Optional.ofNullable(successLogMessage).map(v -> v + System.lineSeparator()).orElse("") + httpResponse.toString());
LOGGER.debug(Optional.ofNullable(successLogMessage).map(v -> v + System.lineSeparator()).orElse("") + httpResponse);
return;
}
String responseContent;
try {
responseContent = EntityUtils.toString(httpResponse.getEntity(), StandardCharsets.UTF_8);
} catch (IOException ex) {
LOGGER.warn("Could not decode response entity", ex);
responseContent = "";
}
String responseContent = Optional.ofNullable(httpResponse.getEntity()).map(entity -> {
try {
return EntityUtils.toString(entity, StandardCharsets.UTF_8);
} catch (IOException ex) {
LOGGER.warn("Could not decode response entity", ex);
return "";
}
}).orElse("");
LOGGER.error("Gitlab response status did not match expected value. Expected: " + expectedStatus
+ System.lineSeparator()
+ httpResponse.toString()
+ httpResponse
+ System.lineSeparator()
+ responseContent);

87
src/test/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/GitlabRestClientTest.java Normal file
View File

@ -0,0 +1,87 @@
package com.github.mc1arke.sonarqube.plugin.almclient.gitlab;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.mc1arke.sonarqube.plugin.almclient.LinkHeaderReader;
import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.MergeRequestNote;
import org.apache.http.HttpEntity;
import org.apache.http.HttpEntityEnclosingRequest;
import org.apache.http.StatusLine;
import org.apache.http.client.entity.UrlEncodedFormEntity;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.message.BasicNameValuePair;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.List;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
class GitlabRestClientTest {
private final CloseableHttpClient closeableHttpClient = mock(CloseableHttpClient.class);
private final LinkHeaderReader linkHeaderReader = mock(LinkHeaderReader.class);
private final ObjectMapper objectMapper = mock(ObjectMapper.class);
@Test
void checkErrorThrownOnNonSuccessResponseStatus() throws IOException {
GitlabRestClient underTest = new GitlabRestClient("http://url.test/api", "token", linkHeaderReader, objectMapper, () -> closeableHttpClient);
CloseableHttpResponse closeableHttpResponse = mock(CloseableHttpResponse.class);
StatusLine statusLine = mock(StatusLine.class);
when(statusLine.getStatusCode()).thenReturn(500);
when(closeableHttpResponse.getStatusLine()).thenReturn(statusLine);
when(closeableHttpClient.execute(any())).thenReturn(closeableHttpResponse);
MergeRequestNote mergeRequestNote = mock(MergeRequestNote.class);
when(mergeRequestNote.getContent()).thenReturn("note");
assertThatThrownBy(() -> underTest.addMergeRequestDiscussion(101, 99, mergeRequestNote))
.isExactlyInstanceOf(IllegalStateException.class)
.hasMessage("An unexpected response code was returned from the Gitlab API - Expected: 201, Got: 500")
.hasNoCause();
ArgumentCaptor<HttpUriRequest> requestArgumentCaptor = ArgumentCaptor.forClass(HttpUriRequest.class);
verify(closeableHttpClient).execute(requestArgumentCaptor.capture());
HttpEntityEnclosingRequest request = (HttpEntityEnclosingRequest) requestArgumentCaptor.getValue();
assertThat(request.getRequestLine().getMethod()).isEqualTo("POST");
assertThat(request.getRequestLine().getUri()).isEqualTo("http://url.test/api/projects/101/merge_requests/99/discussions");
assertThat(request.getEntity()).usingRecursiveComparison().isEqualTo(new UrlEncodedFormEntity(List.of(new BasicNameValuePair("body", "note")), StandardCharsets.UTF_8));
}
@Test
void checkCorrectEncodingUsedOnMergeRequestDiscussion() throws IOException {
CloseableHttpResponse closeableHttpResponse = mock(CloseableHttpResponse.class);
StatusLine statusLine = mock(StatusLine.class);
when(statusLine.getStatusCode()).thenReturn(201);
when(closeableHttpResponse.getStatusLine()).thenReturn(statusLine);
HttpEntity httpEntity = mock(HttpEntity.class);
when(closeableHttpResponse.getEntity()).thenReturn(httpEntity);
when(closeableHttpClient.execute(any())).thenReturn(closeableHttpResponse);
MergeRequestNote mergeRequestNote = new MergeRequestNote("Merge request note");
GitlabRestClient underTest = new GitlabRestClient("http://api.url", "token", linkHeaderReader, objectMapper, () -> closeableHttpClient);
underTest.addMergeRequestDiscussion(123, 321, mergeRequestNote);
ArgumentCaptor<HttpUriRequest> requestArgumentCaptor = ArgumentCaptor.forClass(HttpUriRequest.class);
verify(closeableHttpClient).execute(requestArgumentCaptor.capture());
HttpEntityEnclosingRequest request = (HttpEntityEnclosingRequest) requestArgumentCaptor.getValue();
assertThat(request.getRequestLine().getMethod()).isEqualTo("POST");
assertThat(request.getRequestLine().getUri()).isEqualTo("http://api.url/projects/123/merge_requests/321/discussions");
assertThat(request.getEntity()).usingRecursiveComparison().isEqualTo(new UrlEncodedFormEntity(List.of(new BasicNameValuePair("body", "Merge request note")), StandardCharsets.UTF_8));
}
}

8
src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabMergeRequestDecoratorIntegrationTest.java
View File

@ -130,8 +130,8 @@ public class GitlabMergeRequestDecoratorIntegrationTest {
}
when(issueVisitor.getIssues()).thenReturn(issues);
when(analysisDetails.getPostAnalysisIssueVisitor()).thenReturn(issueVisitor);
when(analysisDetails.createAnalysisSummary(any())).thenReturn("summary comment\n\n[link text]");
when(analysisDetails.createAnalysisIssueSummary(any(), any())).thenReturn("issue");
when(analysisDetails.createAnalysisSummary(any())).thenReturn("summary commént\n\n[link text]");
when(analysisDetails.createAnalysisIssueSummary(any(), any())).thenReturn("issué");
when(analysisDetails.parseIssueIdFromUrl(any())).thenCallRealMethod();
wireMockRule.stubFor(get(urlPathEqualTo("/api/v4/user")).withHeader("PRIVATE-TOKEN", equalTo("token")).willReturn(okJson("{\n" +
@ -202,11 +202,11 @@ public class GitlabMergeRequestDecoratorIntegrationTest {
.willReturn(created()));
wireMockRule.stubFor(post(urlPathEqualTo("/api/v4/projects/" + sourceProjectId + "/merge_requests/" + mergeRequestIid + "/discussions"))
.withRequestBody(equalTo("body=summary+comment%0A%0A%5Blink+text%5D"))
.withRequestBody(equalTo("body=summary+comm%C3%A9nt%0A%0A%5Blink+text%5D"))
.willReturn(created().withBody(discussionPostResponseBody(discussionId, discussionNote(noteId, user, "summary comment", true, false)))));
wireMockRule.stubFor(post(urlPathEqualTo("/api/v4/projects/" + sourceProjectId + "/merge_requests/" + mergeRequestIid + "/discussions"))
.withRequestBody(equalTo("body=issue&" +
.withRequestBody(equalTo("body=issu%C3%A9&" +
urlEncode("position[base_sha]") + "=d6a420d043dfe85e7c240fd136fc6e197998b10a&" +
urlEncode("position[start_sha]") + "=d6a420d043dfe85e7c240fd136fc6e197998b10a&" +
urlEncode("position[head_sha]") + "=" + commitSHA + "&" +