Skip to content

Commit

Permalink
[JENKINS-75157] Bitbucket client fail to retrieve resource with HTTP …
Browse files Browse the repository at this point in the history 
…404 for bitbucket server (#970)

Remove the HttpHost parameter when calling in Apache client as the request could be different than the host configured in jenkins, for example in Bitbucket Server request using mirror link.
Rethrow the FileNotFoundException in executeMethod instead of encapsulate into other exception to respect the SCMFile interface.
Rewrote the BitbucketSCMFile logic to not assume the SCMFile.type but resolve at runtime when needed.
  • Loading branch information
@nfalco79
nfalco79 committed Jan 18, 2025
1 parent c4dd43b commit 0caaac3
Show file tree
Hide file tree
Showing 9 changed files with 165 additions and 96 deletions.
15 changes: 14 additions & 1 deletion src/main/java/com/cloudbees/jenkins/plugins/bitbucket/api/BitbucketApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,17 +302,30 @@ List<? extends BitbucketRepository> getRepositories(@CheckForNull UserRoleInRepo
* @throws IOException if there was a network communications error.
* @throws InterruptedException if interrupted while waiting on remote communications.
*/
@NonNull
@Restricted(NoExternalUse.class)
Iterable<SCMFile> getDirectoryContent(BitbucketSCMFile parent) throws IOException, InterruptedException;

/**
* Return an input stream for the given file.
*
* @param file and instance of SCM file
* @param file an instance of SCM file
* @return the stream of the given {@link SCMFile}
* @throws IOException if there was a network communications error.
* @throws InterruptedException if interrupted while waiting on remote communications.
*/
@Restricted(NoExternalUse.class)
InputStream getFileContent(BitbucketSCMFile file) throws IOException, InterruptedException;

/**
* Return the metadata for the given file.
*
* @param file an instance of SCM file
* @return a {@link SCMFile} file with updated the metadata
* @throws IOException if there was a network communications error.
* @throws InterruptedException if interrupted while waiting on remote communications.
*/
@NonNull
@Restricted(NoExternalUse.class)
SCMFile getFile(@NonNull BitbucketSCMFile file) throws IOException, InterruptedException;
}
37 changes: 23 additions & 14 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -836,27 +836,23 @@ public Iterable<SCMFile> getDirectoryContent(final BitbucketSCMFile parent) thro
.set("path", parent.getPath())
.expand();
List<SCMFile> result = new ArrayList<>();
String response = getRequest(url);
BitbucketCloudPage<BitbucketRepositorySource> page = JsonParser.mapper.readValue(response,
new TypeReference<BitbucketCloudPage<BitbucketRepositorySource>>(){});

for(BitbucketRepositorySource source:page.getValues()){
result.add(source.toBitbucketScmFile(parent));
}
String pageURL = url;
BitbucketCloudPage<BitbucketRepositorySource> page;
do {
String response = getRequest(pageURL);
page = JsonParser.mapper.readValue(response, new TypeReference<BitbucketCloudPage<BitbucketRepositorySource>>(){});

while (!page.isLastPage()){
response = getRequest(page.getNext());
page = JsonParser.mapper.readValue(response,
new TypeReference<BitbucketCloudPage<BitbucketRepositorySource>>(){});
for(BitbucketRepositorySource source:page.getValues()){
result.add(source.toBitbucketScmFile(parent));
for(BitbucketRepositorySource source : page.getValues()){
result.add(source.toBitbucketSCMFile(parent));
}
}
pageURL = page.getNext();
} while (!page.isLastPage());
return result;
}

@Override
public InputStream getFileContent(BitbucketSCMFile file) throws IOException, InterruptedException {
public InputStream getFileContent(@NonNull BitbucketSCMFile file) throws IOException, InterruptedException {
String url = UriTemplate.fromTemplate(REPO_URL_TEMPLATE + "/src{/branchOrHash,path}")
.set("owner", owner)
.set("repo", repositoryName)
Expand All @@ -865,4 +861,17 @@ public InputStream getFileContent(BitbucketSCMFile file) throws IOException, Int
.expand();
return getRequestAsInputStream(url);
}

@Override
public SCMFile getFile(@NonNull BitbucketSCMFile file) throws IOException, InterruptedException {
String url = UriTemplate.fromTemplate(REPO_URL_TEMPLATE + "/src{/branchOrHash,path}?format=meta")
.set("owner", owner)
.set("repo", repositoryName)
.set("branchOrHash", file.getRef())
.set("path", file.getPath())
.expand();
String response = getRequest(url);
BitbucketRepositorySource src = JsonParser.mapper.readValue(response, BitbucketRepositorySource.class);
return src.toBitbucketSCMFile((BitbucketSCMFile) file.parent());

Check warning on line 875 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiClient.java

View check run for this annotation

ci.jenkins.io / SpotBugs

NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE 

NORMAL:
Possible null pointer dereference in com.cloudbees.jenkins.plugins.bitbucket.client.BitbucketCloudApiClient.getFile(BitbucketSCMFile) due to return value of called method
Raw output 
<p> The return value from a method is dereferenced without a null check, and the return value of that method is one that should generally be checked for null. This may lead to a <code>NullPointerException</code> when the code is executed. </p>
}
}
13 changes: 7 additions & 6 deletions .../com/cloudbees/jenkins/plugins/bitbucket/client/repository/BitbucketRepositorySource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,24 @@ public String getHash() {

@JsonIgnore
public boolean isDirectory() {
return type.equals("commit_directory");
return "commit_directory".equals(type);
}

public BitbucketSCMFile toBitbucketScmFile(BitbucketSCMFile parent){
public BitbucketSCMFile toBitbucketSCMFile(BitbucketSCMFile parent) {
SCMFile.Type fileType;
if(isDirectory()){
if (isDirectory()) {
fileType = SCMFile.Type.DIRECTORY;
} else {
fileType = SCMFile.Type.REGULAR_FILE;
for(String attribute: getAttributes()){
if(attribute.equals("link")){
for (String attribute : getAttributes()) {
if (attribute.equals("link")) {
fileType = SCMFile.Type.LINK;
} else if(attribute.equals("subrepository")){
} else if (attribute.equals("subrepository")) {
fileType = SCMFile.Type.OTHER; // sub-module or sub-repo
}
}
}
return new BitbucketSCMFile(parent, path, fileType, hash);
}

}
45 changes: 22 additions & 23 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/filesystem/BitbucketSCMFile.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.IOException;
import java.io.InputStream;
import java.util.Collections;
import jenkins.scm.api.SCMFile;

public class BitbucketSCMFile extends SCMFile {
public class BitbucketSCMFile extends SCMFile {

private final BitbucketApi api;
private String ref;
private final String hash;
private boolean resolved;

public String getRef() {
return ref;
Expand All @@ -44,26 +46,12 @@ public void setRef(String ref) {
this.ref = ref;
}

@Deprecated
public BitbucketSCMFile(BitbucketSCMFileSystem bitBucketSCMFileSystem,
BitbucketApi api,
String ref) {
this(bitBucketSCMFileSystem, api, ref, null);
}

public BitbucketSCMFile(BitbucketSCMFileSystem bitBucketSCMFileSystem,
BitbucketApi api,
String ref, String hash) {
super();
public BitbucketSCMFile(BitbucketApi api, String ref, String hash) {
type(Type.DIRECTORY);
this.api = api;
this.ref = ref;
this.hash = hash;
}

@Deprecated
public BitbucketSCMFile(@NonNull BitbucketSCMFile parent, String name, Type type) {
this(parent, name, type, null);
this.resolved = false;
}

public BitbucketSCMFile(@NonNull BitbucketSCMFile parent, String name, Type type, String hash) {
Expand All @@ -72,6 +60,7 @@ public BitbucketSCMFile(@NonNull BitbucketSCMFile parent, String name, Type type
this.ref = parent.ref;
this.hash = hash;
type(type);
this.resolved = true;
}

public String getHash() {
Expand All @@ -80,12 +69,12 @@ public String getHash() {

@Override
@NonNull
public Iterable<SCMFile> children() throws IOException,
InterruptedException {
public Iterable<SCMFile> children() throws IOException, InterruptedException {
if (this.isDirectory()) {
return api.getDirectoryContent(this);
} else {
throw new IOException("Cannot get children from a regular file");
// respect the interface javadoc
return Collections.emptyList();
}
}

Expand All @@ -101,19 +90,29 @@ public InputStream content() throws IOException, InterruptedException {

@Override
public long lastModified() throws IOException, InterruptedException {
// TODO: Return valid value when Tag support is implemented
return 0;
return 0L;
}

@Override
@NonNull
protected SCMFile newChild(String name, boolean assumeIsDirectory) {
return new BitbucketSCMFile(this, name, assumeIsDirectory?Type.DIRECTORY:Type.REGULAR_FILE, hash);
BitbucketSCMFile child = new BitbucketSCMFile(this, name, assumeIsDirectory ? Type.DIRECTORY : Type.REGULAR_FILE, hash);
child.resolved = false;
return child;
}

@Override
@NonNull
protected Type type() throws IOException, InterruptedException {
if (!resolved) {
try {
SCMFile metadata = api.getFile(this);
type(metadata.getType());
} catch(IOException e) {
type(Type.NONEXISTENT);
}
resolved = true;
}
return this.getType();
}

Expand Down
27 changes: 13 additions & 14 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/filesystem/BitbucketSCMFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@
import hudson.Util;
import hudson.model.Item;
import hudson.model.Queue;
import hudson.model.queue.Tasks;
import hudson.scm.SCM;
import hudson.scm.SCMDescriptor;
import hudson.security.ACL;
import java.io.IOException;
import java.lang.annotation.Inherited;
import jenkins.authentication.tokens.api.AuthenticationTokens;
import jenkins.scm.api.SCMFile;
import jenkins.scm.api.SCMFileSystem;
Expand All @@ -69,21 +69,18 @@ protected BitbucketSCMFileSystem(BitbucketApi api, String ref, SCMRevision rev)
}

/**
* Return timestamp of last commit or of tag if its annotated tag.
*
* @return timestamp of last commit or of tag if its annotated tag
* {@link Inherited}
*/
@Override
public long lastModified() throws IOException {
// TODO figure out how to implement this
return 0L;
}

@NonNull
@Override
public SCMFile getRoot() {
SCMRevision revision = getRevision();
return new BitbucketSCMFile(this, api, ref, revision == null ? null : revision.toString());
return new BitbucketSCMFile(api, ref, revision == null ? null : revision.toString());
}

@Extension
Expand Down Expand Up @@ -116,22 +113,24 @@ public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull
}

private static StandardCredentials lookupScanCredentials(@CheckForNull Item context,
@CheckForNull String scanCredentialsId, String serverUrl) {
if (Util.fixEmpty(scanCredentialsId) == null) {
@CheckForNull String scanCredentialsId,
String serverURL) {
scanCredentialsId = Util.fixEmpty(scanCredentialsId);
if (scanCredentialsId == null) {
return null;
} else {
return CredentialsMatchers.firstOrNull(
CredentialsProvider.lookupCredentials(
CredentialsProvider.lookupCredentialsInItem(
StandardCredentials.class,
context,
context instanceof Queue.Task
? Tasks.getDefaultAuthenticationOf((Queue.Task) context)
: ACL.SYSTEM,
URIRequirementBuilder.fromUri(serverUrl).build()
context instanceof Queue.Task task
? task.getDefaultAuthentication2()
: ACL.SYSTEM2,
URIRequirementBuilder.fromUri(serverURL).build()
),
CredentialsMatchers.allOf(
CredentialsMatchers.withId(scanCredentialsId),
AuthenticationTokens.matcher(BitbucketAuthenticator.authenticationContext(serverUrl))
AuthenticationTokens.matcher(BitbucketAuthenticator.authenticationContext(serverURL))
)
);
}
Expand Down
36 changes: 11 additions & 25 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/impl/client/AbstractBitbucketApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@
import java.io.InputStream;
import java.net.InetSocketAddress;
import java.net.Proxy;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -220,21 +218,21 @@ private void setClientProxyParams(String host, HttpClientBuilder builder) {
@NonNull
protected abstract CloseableHttpClient getClient();

protected CloseableHttpResponse executeMethod(HttpHost host,
HttpRequestBase httpMethod,
boolean requireAuthentication) throws IOException {
protected CloseableHttpResponse executeMethod(HttpRequestBase request, boolean requireAuthentication) throws IOException {
if (requireAuthentication && authenticator != null) {
authenticator.configureRequest(httpMethod);
authenticator.configureRequest(request);
}
return getClient().execute(host, httpMethod, context);
// the Apache client determinate the host from request.getURI()
// in some cases like requests to mirror or avatar, the host could not be the same of configured in Jenkins
return getClient().execute(request, context);
}

protected CloseableHttpResponse executeMethod(HttpHost host, HttpRequestBase httpMethod) throws IOException {
return executeMethod(host, httpMethod, true);
protected CloseableHttpResponse executeMethod(HttpRequestBase httpMethod) throws IOException {
return executeMethod(httpMethod, true);
}

protected String doRequest(HttpRequestBase request, boolean requireAuthentication) throws IOException {
try (CloseableHttpResponse response = executeMethod(getHost(), request, requireAuthentication)) {
try (CloseableHttpResponse response = executeMethod(request, requireAuthentication)) {
int statusCode = response.getStatusLine().getStatusCode();
if (statusCode == HttpStatus.SC_NOT_FOUND) {
throw new FileNotFoundException("URL: " + request.getURI());
Expand All @@ -250,7 +248,7 @@ protected String doRequest(HttpRequestBase request, boolean requireAuthenticatio
throw buildResponseException(response, content);
}
return content;
} catch (BitbucketRequestException e) {
} catch (FileNotFoundException | BitbucketRequestException e) {
throw e;
} catch (IOException e) {
throw new IOException("Communication error for url: " + request, e);
Expand All @@ -276,19 +274,7 @@ private void release(HttpRequestBase method) {
*/
protected InputStream getRequestAsInputStream(String path) throws IOException {
HttpGet httpget = new HttpGet(path);
HttpHost host = getHost();

// Extract host from URL, if present
try {
URI uri = new URI(host.toURI());
if (uri.isAbsolute() && ! uri.isOpaque()) {
host = HttpHost.create(uri.getScheme() + "://" + uri.getAuthority());
}
} catch (URISyntaxException ex) {
// use default
}

CloseableHttpResponse response = executeMethod(host, httpget);
CloseableHttpResponse response = executeMethod(httpget);
int statusCode = response.getStatusLine().getStatusCode();
if (statusCode == HttpStatus.SC_NOT_FOUND) {
EntityUtils.consume(response.getEntity());
Expand All @@ -303,7 +289,7 @@ protected InputStream getRequestAsInputStream(String path) throws IOException {

protected int headRequestStatus(String path) throws IOException {
HttpHead httpHead = new HttpHead(path);
try (CloseableHttpResponse response = executeMethod(getHost(), httpHead)) {
try (CloseableHttpResponse response = executeMethod(httpHead)) {
EntityUtils.consume(response.getEntity());
return response.getStatusLine().getStatusCode();
} catch (IOException e) {
Expand Down
Loading

0 comments on commit 0caaac3

Please sign in to comment.