Skip to content

Commit

Permalink
Fix the API build notification state when the sent payload contains a…
Browse files Browse the repository at this point in the history
… longer than allowed attribute value.

For bitbucket server and cloud:
- name must be less than 255 characters;
- URL must be less than 450 characters, in this case we can not trim otherwise the URL is invalid.
  • Loading branch information
nfalco79 committed Nov 27, 2024
1 parent a71d0e0 commit eca80b1
Show file tree
Hide file tree
Showing 12 changed files with 244 additions and 37 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ work

# VSCode
.factorypath
META-INF/
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@
<version>3.26.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>net.javacrumbs.json-unit</groupId>
<artifactId>json-unit-assertj</artifactId>
<version>4.0.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>git</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ private static void createStatus(@NonNull Run<?, ?> build, @NonNull TaskListener

private static @CheckForNull BitbucketSCMSource findBitbucketSCMSource(Run<?, ?> build) {
SCMSource s = SCMSource.SourceByItem.findSource(build.getParent());
return s instanceof BitbucketSCMSource ? (BitbucketSCMSource) s : null;
return s instanceof BitbucketSCMSource scm ? scm : null;
}

private static void sendNotifications(BitbucketSCMSource source, Run<?, ?> build, TaskListener listener)
Expand Down Expand Up @@ -211,12 +211,11 @@ private static void sendNotifications(BitbucketSCMSource source, Run<?, ?> build

@CheckForNull
private static String getHash(@CheckForNull SCMRevision revision) {
if (revision instanceof PullRequestSCMRevision) {
// unwrap
revision = ((PullRequestSCMRevision) revision).getPull();
if (revision instanceof PullRequestSCMRevision prRevision) {

Check warning on line 214 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketBuildStatusNotifications.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 214 is only partially covered, one branch is missing
revision = prRevision.getPull();

Check warning on line 215 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketBuildStatusNotifications.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 215 is not covered by tests
}
if (revision instanceof AbstractGitSCMSource.SCMRevisionImpl) {
return ((AbstractGitSCMSource.SCMRevisionImpl) revision).getHash();
if (revision instanceof AbstractGitSCMSource.SCMRevisionImpl scmRevision) {

Check warning on line 217 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketBuildStatusNotifications.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 217 is only partially covered, one branch is missing
return scmRevision.getHash();
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
package com.cloudbees.jenkins.plugins.bitbucket.api;

import com.fasterxml.jackson.annotation.JsonIgnore;
import edu.umd.cs.findbugs.annotations.NonNull;
import org.apache.commons.codec.digest.DigestUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
Expand Down Expand Up @@ -99,6 +100,20 @@ public BitbucketBuildStatus(String hash, String description, Status state, Strin
this.name = name;
}

/**
* Copy constructor.
*
* @param other from copy to.
*/
public BitbucketBuildStatus(@NonNull BitbucketBuildStatus other) {
this.hash = other.hash;
this.description = other.description;
this.state = other.state;
this.url = other.url;
this.key = other.key;
this.name = other.name;
}

public String getHash() {
return hash;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -657,12 +657,15 @@ public List<BitbucketRepositoryHook> getWebHooks() throws IOException, Interrupt
*/
@Override
public void postBuildStatus(@NonNull BitbucketBuildStatus status) throws IOException, InterruptedException {
BitbucketBuildStatus newStatus = new BitbucketBuildStatus(status);
newStatus.setName(truncateMiddle(newStatus.getName(), 255));

String url = UriTemplate.fromTemplate(REPO_URL_TEMPLATE + "/commit/{hash}/statuses/build")
.set("owner", owner)
.set("repo", repositoryName)
.set("hash", status.getHash())
.set("hash", newStatus.getHash())
.expand();
postRequest(url, JsonParser.toJson(status));
postRequest(url, JsonParser.toJson(newStatus));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
package com.cloudbees.jenkins.plugins.bitbucket.internal.api;

import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketRequestException;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import hudson.ProxyConfiguration;
import java.io.IOException;
import java.io.InputStream;
Expand All @@ -50,12 +51,21 @@
import org.kohsuke.accmod.restrictions.ProtectedExternally;

@Restricted(ProtectedExternally.class)
public class AbstractBitbucketApi {
public abstract class AbstractBitbucketApi {
protected static final int API_RATE_LIMIT_STATUS_CODE = 429;

protected final Logger logger = Logger.getLogger(this.getClass().getName());
protected HttpClientContext context;

protected String truncateMiddle(@CheckForNull String value, int maxLength) {
int length = StringUtils.length(value);
if (length > maxLength) {

Check warning on line 62 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/internal/api/AbstractBitbucketApi.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 62 is only partially covered, one branch is missing
int halfLength = (maxLength - 3) / 2;
return StringUtils.left(value, halfLength) + "..." + StringUtils.substring(value, -halfLength);
} else {
return value;

Check warning on line 66 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/internal/api/AbstractBitbucketApi.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 66 is not covered by tests
}
}

protected BitbucketRequestException buildResponseException(CloseableHttpResponse response, String errorMessage) {
String headers = StringUtils.join(response.getAllHeaders(), "\n");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,13 +509,13 @@ public void postCommitComment(@NonNull String hash, @NonNull String comment) thr
*/
@Override
public void postBuildStatus(@NonNull BitbucketBuildStatus status) throws IOException, InterruptedException {
postRequest(
UriTemplate
.fromTemplate(API_COMMIT_STATUS_PATH)
.set("hash", status.getHash())
.expand(),
JsonParser.toJson(status)
);
BitbucketBuildStatus newStatus = new BitbucketBuildStatus(status);
newStatus.setName(truncateMiddle(newStatus.getName(), 255));

String url = UriTemplate.fromTemplate(API_COMMIT_STATUS_PATH)
.set("hash", newStatus.getHash())
.expand();
postRequest(url, JsonParser.toJson(newStatus));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,52 +25,87 @@

import com.cloudbees.jenkins.plugins.bitbucket.JsonParser;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketApi;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketBuildStatus;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketBuildStatus.Status;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketWebHook;
import com.cloudbees.jenkins.plugins.bitbucket.client.BitbucketIntegrationClientFactory.IRequestAudit;
import com.cloudbees.jenkins.plugins.bitbucket.client.repository.BitbucketCloudRepository;
import com.cloudbees.jenkins.plugins.bitbucket.endpoints.BitbucketCloudEndpoint;
import io.jenkins.cli.shaded.org.apache.commons.lang.RandomStringUtils;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.Date;
import java.util.Optional;
import org.apache.commons.io.IOUtils;
import org.hamcrest.CoreMatchers;
import org.junit.Test;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
import org.apache.http.client.methods.HttpRequestBase;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;

public class BitbucketCloudApiClientTest {
class BitbucketCloudApiClientTest {

public String loadPayload(String api) throws IOException {
private String loadPayload(String api) throws IOException {
try (InputStream is = getClass().getResourceAsStream(getClass().getSimpleName() + "/" + api + "Payload.json")) {
return IOUtils.toString(is, "UTF-8");
}
}

@Test
public void get_repository_parse_correctly_date_from_cloud() throws Exception {
void verify_status_notitication_name_max_length() throws Exception {
BitbucketApi client = BitbucketIntegrationClientFactory.getApiMockClient(BitbucketCloudEndpoint.SERVER_URL);
BitbucketBuildStatus status = new BitbucketBuildStatus();
status.setName(RandomStringUtils.randomAlphanumeric(300));
status.setState(Status.INPROGRESS);
status.setHash("046d9a3c1532acf4cf08fe93235c00e4d673c1d3");

client.postBuildStatus(status);

IRequestAudit clientAudit = ((IRequestAudit) client).getAudit();
HttpRequestBase request = extractRequest(clientAudit);
assertThat(request).isNotNull()
.isInstanceOf(HttpPost.class);
try (InputStream content = ((HttpPost) request).getEntity().getContent()) {
String json = IOUtils.toString(content, StandardCharsets.UTF_8);
assertThatJson(json).node("name").isString().hasSize(255);
}
}

private HttpRequestBase extractRequest(IRequestAudit clientAudit) {
ArgumentCaptor<HttpRequestBase> captor = ArgumentCaptor.forClass(HttpRequestBase.class);
verify(clientAudit).request(captor.capture());
return captor.getValue();
}

@Test
void get_repository_parse_correctly_date_from_cloud() throws Exception {
BitbucketCloudRepository repository = JsonParser.toJava(loadPayload("getRepository"), BitbucketCloudRepository.class);
assertNotNull("update on date is null", repository.getUpdatedOn());
Date date = DateUtils.getDate(2018, 4, 27, 15, 32, 8, 356);
assertThat(repository.getUpdatedOn().getTime(), CoreMatchers.is(date.getTime()));
assertThat(repository.getUpdatedOn()).describedAs("update on date is null").isNotNull();
Date expectedDate = DateUtils.getDate(2018, 4, 27, 15, 32, 8, 356);
assertThat(repository.getUpdatedOn()).isEqualTo(expectedDate);
}

@Test
public void verifyUpdateWebhookURL() throws Exception {
void verifyUpdateWebhookURL() throws Exception {
BitbucketApi client = BitbucketIntegrationClientFactory.getApiMockClient(BitbucketCloudEndpoint.SERVER_URL);
IRequestAudit audit = ((IRequestAudit) client).getAudit();
Optional<? extends BitbucketWebHook> webHook = client.getWebHooks().stream()
.filter(h -> h.getDescription().contains("Jenkins"))
.findFirst();
assertTrue(webHook.isPresent());
assertThat(webHook).isPresent();

reset(audit);
client.updateCommitWebHook(webHook.get());
verify(audit).request("https://api.bitbucket.org/2.0/repositories/amuniz/test-repos/hooks/%7B202cf34e-7ccf-44b7-ba6b-8827a14d5324%7D");
HttpRequestBase request = extractRequest(audit);
assertThat(request).isNotNull()
.isInstanceOfSatisfying(HttpPut.class, put ->
assertThat(put.getURI()).hasToString("https://api.bitbucket.org/2.0/repositories/amuniz/test-repos/hooks/%7B202cf34e-7ccf-44b7-ba6b-8827a14d5324%7D"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
public class BitbucketIntegrationClientFactory {

public interface IRequestAudit {
default void request(String url) {
default void request(HttpRequestBase request) {
// mockito audit
}

Expand Down Expand Up @@ -118,10 +118,12 @@ protected CloseableHttpResponse executeMethodNoRetry(CloseableHttpClient client,
return createRateLimitResponse();
}
String path = httpMethod.getURI().toString();
path = path.substring(path.indexOf("/rest/api/"));
audit.request(path);
audit.request(httpMethod);

String payloadPath = path.replace("/rest/api/", "").replace('/', '-').replaceAll("[=%&?]", "_");
String payloadPath = path.substring(path.indexOf("/rest/"))
.replace("/rest/api/", "")
.replace("/rest/", "")
.replace('/', '-').replaceAll("[=%&?]", "_");
payloadPath = payloadRootPath + payloadPath + ".json";

return loadResponseFromResources(getClass(), path, payloadPath);
Expand Down Expand Up @@ -160,7 +162,7 @@ private BitbucketClouldIntegrationClient(String payloadRootPath, String owner, S
@Override
protected CloseableHttpResponse executeMethod(HttpHost host, HttpRequestBase httpMethod) throws InterruptedException, IOException {
String path = httpMethod.getURI().toString();
audit.request(path);
audit.request(httpMethod);

String payloadPath = path.replace(API_ENDPOINT, "").replace('/', '-').replaceAll("[=%&?]", "_");
payloadPath = payloadRootPath + payloadPath + ".json";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,37 @@
package com.cloudbees.jenkins.plugins.bitbucket.server.client;

import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketApi;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketBuildStatus;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketBuildStatus.Status;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketRepository;
import com.cloudbees.jenkins.plugins.bitbucket.client.BitbucketIntegrationClientFactory;
import com.cloudbees.jenkins.plugins.bitbucket.client.BitbucketIntegrationClientFactory.BitbucketServerIntegrationClient;
import com.cloudbees.jenkins.plugins.bitbucket.client.BitbucketIntegrationClientFactory.IRequestAudit;
import com.damnhandy.uri.template.UriTemplate;
import com.damnhandy.uri.template.impl.Operator;
import io.jenkins.cli.shaded.org.apache.commons.lang.RandomStringUtils;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.logging.Level;
import org.apache.commons.io.IOUtils;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpRequestBase;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.junit.Assert;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.LoggerRule;
import org.jvnet.hudson.test.WithoutJenkins;
import org.mockito.ArgumentCaptor;
import org.mockito.MockedStatic;

import static com.cloudbees.jenkins.plugins.bitbucket.server.client.BitbucketServerAPIClient.API_BROWSE_PATH;
import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson;
import static org.assertj.core.api.Assertions.assertThat;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
Expand All @@ -32,11 +45,38 @@

public class BitbucketServerAPIClientTest {

@Rule
public JenkinsRule r = new JenkinsRule();
@ClassRule
public static JenkinsRule r = new JenkinsRule();
@Rule
public LoggerRule logger = new LoggerRule().record(BitbucketServerIntegrationClient.class, Level.FINE);

@Test
@WithoutJenkins
public void verify_status_notitication_name_max_length() throws Exception {
BitbucketApi client = BitbucketIntegrationClientFactory.getApiMockClient("https://acme.bitbucket.org");
BitbucketBuildStatus status = new BitbucketBuildStatus();
status.setName(RandomStringUtils.randomAlphanumeric(300));
status.setState(Status.INPROGRESS);
status.setHash("046d9a3c1532acf4cf08fe93235c00e4d673c1d3");

client.postBuildStatus(status);

IRequestAudit clientAudit = ((IRequestAudit) client).getAudit();
HttpRequestBase request = extractRequest(clientAudit);
assertThat(request).isNotNull()
.isInstanceOf(HttpPost.class);
try (InputStream content = ((HttpPost) request).getEntity().getContent()) {
String json = IOUtils.toString(content, StandardCharsets.UTF_8);
assertThatJson(json).node("name").isString().hasSize(255);
}
}

private HttpRequestBase extractRequest(IRequestAudit clientAudit) {
ArgumentCaptor<HttpRequestBase> captor = ArgumentCaptor.forClass(HttpRequestBase.class);
verify(clientAudit).request(captor.capture());
return captor.getValue();
}

@Test
@WithoutJenkins
public void repoBrowsePathFolder() {
Expand Down
Loading

0 comments on commit eca80b1

Please sign in to comment.