Skip to content

Allow special characters in the url #1590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 26, 2025
Merged

Allow special characters in the url #1590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ...controller-api/src/main/java/org/cloudfoundry/multiapps/controller/api/model/FileUrl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import org.cloudfoundry.multiapps.common.Nullable;
import org.immutables.value.Value;

@Value.Immutable
Expand All @@ -14,5 +15,9 @@ public interface FileUrl {
@JsonProperty("file_url")
String getFileUrl();

@Nullable
@Value.Parameter
@JsonProperty("credentials")
UserCredentials getUserCredentials();
//this could potentially contain a TLS certificate as well, if the remote endpoint is a custom registry/repository
}
25 changes: 25 additions & 0 deletions ...er-api/src/main/java/org/cloudfoundry/multiapps/controller/api/model/UserCredentials.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.cloudfoundry.multiapps.controller.api.model;

import org.cloudfoundry.multiapps.common.Nullable;
import org.immutables.value.Value;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;

@Value.Immutable
@JsonSerialize(as = ImmutableUserCredentials.class)
@JsonDeserialize(as = ImmutableUserCredentials.class)
public interface UserCredentials {

@Nullable
@Value.Parameter
@JsonProperty("username")
String getUsername();

@Nullable
@Value.Parameter
@JsonProperty("password")
String getPassword();
//this could potentially contain a TLS certificate as well, if the remote endpoint is a custom registry/repository
}
75 changes: 53 additions & 22 deletions ...src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
import java.io.InputStream;
import java.math.BigInteger;
import java.net.URI;
import java.net.URLDecoder;
import java.net.http.HttpClient;
import java.net.http.HttpClient.Redirect;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.net.http.HttpResponse.BodyHandlers;
import java.nio.charset.StandardCharsets;
import java.text.MessageFormat;
import java.time.Duration;
import java.time.LocalDateTime;
Expand All @@ -24,9 +26,6 @@
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;

import jakarta.inject.Inject;
import jakarta.inject.Named;

import org.apache.commons.io.IOUtils;
import org.apache.commons.io.input.ProxyInputStream;
import org.cloudfoundry.multiapps.common.SLException;
Expand All @@ -36,6 +35,7 @@
import org.cloudfoundry.multiapps.controller.api.model.FileUrl;
import org.cloudfoundry.multiapps.controller.api.model.ImmutableAsyncUploadResult;
import org.cloudfoundry.multiapps.controller.api.model.ImmutableFileMetadata;
import org.cloudfoundry.multiapps.controller.api.model.UserCredentials;
import org.cloudfoundry.multiapps.controller.client.util.CheckedSupplier;
import org.cloudfoundry.multiapps.controller.client.util.ResilientOperationExecutor;
import org.cloudfoundry.multiapps.controller.core.auditlogging.FilesApiServiceAuditLog;
Expand Down Expand Up @@ -69,6 +69,9 @@
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.multipart.MultipartHttpServletRequest;

import jakarta.inject.Inject;
import jakarta.inject.Named;

@Named
public class FilesApiServiceImpl implements FilesApiService {

Expand All @@ -77,6 +80,11 @@ public class FilesApiServiceImpl implements FilesApiService {
private static final int INPUT_STREAM_BUFFER_SIZE = 16 * 1024;
private static final Duration HTTP_CONNECT_TIMEOUT = Duration.ofMinutes(10);
private static final String RETRY_AFTER_SECONDS = "30";
private static final String USERNAME_PASSWORD_URL_FORMAT = "{0}:{1}";
static {
System.setProperty(Constants.RETRY_LIMIT_PROPERTY, "0");
}

private final CachedMap<String, AtomicLong> jobCounters = new CachedMap<>(Duration.ofHours(1));
private final CachedMap<String, Future<?>> runningTasks = new CachedMap<>(Duration.ofHours(1));
private final ResilientOperationExecutor resilientOperationExecutor = getResilientOperationExecutor();
Expand All @@ -97,10 +105,6 @@ public class FilesApiServiceImpl implements FilesApiService {
@Inject
private ExecutorService fileStorageThreadPool;

static {
System.setProperty(Constants.RETRY_LIMIT_PROPERTY, "0");
}

@Override
public ResponseEntity<List<FileMetadata>> getFiles(String spaceGuid, String namespace) {
try {
Expand Down Expand Up @@ -156,7 +160,7 @@ public ResponseEntity<Void> startUploadFromUrl(String spaceGuid, String namespac
deleteAsyncJobEntry(existingJob);
}
}
return triggerUploadFromUrl(spaceGuid, namespace, urlWithoutUserInfo, decodedUrl);
return triggerUploadFromUrl(spaceGuid, namespace, urlWithoutUserInfo, decodedUrl, fileUrl.getUserCredentials());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UserCredentials is passed through multiple layers just to reach buildFetchFileRequest. Just a small suggestion—consider encapsulating related parameters into a FileDownloadRequestContext to reduce method parameter lists and improve readability. Not a change request, just optimization proposal.

}

private String getLocationHeader(String spaceGuid, String jobId) {
Expand Down Expand Up @@ -289,12 +293,14 @@ private void deleteAsyncJobEntry(AsyncUploadJobEntry entry) {
}
}

private ResponseEntity<Void> triggerUploadFromUrl(String spaceGuid, String namespace, String urlWithoutUserInfo, String decodedUrl) {
private ResponseEntity<Void> triggerUploadFromUrl(String spaceGuid, String namespace, String urlWithoutUserInfo, String decodedUrl,
UserCredentials userCredentials) {
var entry = createJobEntry(spaceGuid, namespace, urlWithoutUserInfo);
LOGGER.debug(Messages.CREATING_ASYNC_UPLOAD_JOB, urlWithoutUserInfo, entry.getId());
uploadJobService.add(entry);
try {
Future<?> runningTask = deployFromUrlExecutor.submit(() -> uploadFileFromUrl(entry, spaceGuid, namespace, decodedUrl));
Future<?> runningTask = deployFromUrlExecutor.submit(() -> uploadFileFromUrl(entry, spaceGuid, namespace, decodedUrl,
userCredentials));
runningTasks.put(entry.getId(), runningTask);
} catch (RejectedExecutionException ignored) {
LOGGER.debug(Messages.ASYNC_UPLOAD_JOB_REJECTED, entry.getId());
Expand Down Expand Up @@ -345,7 +351,8 @@ private AsyncUploadResult createErrorResult(String error, AsyncUploadResult.Clie
.build();
}

private void uploadFileFromUrl(AsyncUploadJobEntry jobEntry, String spaceGuid, String namespace, String fileUrl) {
private void uploadFileFromUrl(AsyncUploadJobEntry jobEntry, String spaceGuid, String namespace, String fileUrl,
UserCredentials userCredentials) {
var counter = new AtomicLong(0);
jobCounters.put(jobEntry.getId(), counter);
LOGGER.info(Messages.STARTING_DOWNLOAD_OF_MTAR, jobEntry.getUrl());
Expand All @@ -358,7 +365,8 @@ private void uploadFileFromUrl(AsyncUploadJobEntry jobEntry, String spaceGuid, S
FileEntry fileEntry = resilientOperationExecutor.execute((CheckedSupplier<FileEntry>) () -> doUploadFileFromUrl(spaceGuid,
namespace,
fileUrl,
counter));
counter,
userCredentials));
LOGGER.trace(Messages.UPLOADED_MTAR_FROM_REMOTE_ENDPOINT_AND_JOB_ID, jobEntry.getUrl(), jobEntry.getId(),
ChronoUnit.MILLIS.between(startTime, LocalDateTime.now()));
var descriptor = fileService.processFileContent(spaceGuid, fileEntry.getId(), this::extractDeploymentDescriptor);
Expand All @@ -376,14 +384,16 @@ private void uploadFileFromUrl(AsyncUploadJobEntry jobEntry, String spaceGuid, S
}
}

private FileEntry doUploadFileFromUrl(String spaceGuid, String namespace, String fileUrl, AtomicLong counter) throws Exception {
private FileEntry doUploadFileFromUrl(String spaceGuid, String namespace, String fileUrl, AtomicLong counter,
UserCredentials userCredentials)
throws Exception {
if (!UriUtil.isUrlSecure(fileUrl)) {
throw new SLException(Messages.MTAR_ENDPOINT_NOT_SECURE);
}
UriUtil.validateUrl(fileUrl);
HttpClient client = buildHttpClient(fileUrl);

HttpResponse<InputStream> response = callRemoteEndpointWithRetry(client, fileUrl);
HttpResponse<InputStream> response = callRemoteEndpointWithRetry(client, fileUrl, userCredentials);
long fileSize = response.headers()
.firstValueAsLong(Constants.CONTENT_LENGTH)
.orElseThrow(() -> new SLException(Messages.FILE_URL_RESPONSE_DID_NOT_RETURN_CONTENT_LENGTH));
Expand Down Expand Up @@ -411,10 +421,11 @@ private FileEntry doUploadFileFromUrl(String spaceGuid, String namespace, String
}
}

private HttpResponse<InputStream> callRemoteEndpointWithRetry(HttpClient client, String decodedUrl) throws Exception {
public HttpResponse<InputStream> callRemoteEndpointWithRetry(HttpClient client, String decodedUrl, UserCredentials userCredentials)
throws Exception {
return resilientOperationExecutor.execute((CheckedSupplier<HttpResponse<InputStream>>) () -> {
var request = buildFetchFileRequest(decodedUrl);
LOGGER.debug(Messages.CALLING_REMOTE_MTAR_ENDPOINT, request.uri());
var request = buildFetchFileRequest(decodedUrl, userCredentials);
LOGGER.debug(Messages.CALLING_REMOTE_MTAR_ENDPOINT, getMaskedUri(urlDecodeUrl(decodedUrl)));
var response = client.send(request, BodyHandlers.ofInputStream());
if (response.statusCode() / 100 != 2) {
String error = readErrorBodyFromResponse(response);
Expand All @@ -424,13 +435,26 @@ private HttpResponse<InputStream> callRemoteEndpointWithRetry(HttpClient client,
UriUtil.stripUserInfo(decodedUrl));
throw new SLException(errorMessage);
}
throw new SLException(MessageFormat.format(Messages.ERROR_FROM_REMOTE_MTAR_ENDPOINT, request.uri(), response.statusCode(),
error));
throw new SLException(MessageFormat.format(Messages.ERROR_FROM_REMOTE_MTAR_ENDPOINT, getMaskedUri(urlDecodeUrl(decodedUrl)),
response.statusCode(), error));
}
return response;
});
}

private String getMaskedUri(String url) {
if (url.contains("@")) {
return url.substring(url.lastIndexOf("@"))
.replace("@", "...");
} else {
return url;
}
}

private String urlDecodeUrl(String url) {
return URLDecoder.decode(url, StandardCharsets.UTF_8);
}

private void resetCounterOnRetry(AtomicLong counter) {
counter.set(0);
}
Expand All @@ -443,14 +467,21 @@ protected HttpClient buildHttpClient(String decodedUrl) {
.build();
}

private HttpRequest buildFetchFileRequest(String decodedUrl) {
private HttpRequest buildFetchFileRequest(String decodedUrl, UserCredentials userCredentials) {
var builder = HttpRequest.newBuilder()
.GET()
.timeout(Duration.ofMinutes(15));
var uri = URI.create(decodedUrl);
var userInfo = uri.getUserInfo();
if (userInfo != null) {
builder.uri(URI.create(decodedUrl.replace(userInfo + "@", "")));
if (userCredentials != null) {
builder.uri(uri);
String userCredentialsUrlFormat = MessageFormat.format(USERNAME_PASSWORD_URL_FORMAT, userCredentials.getUsername(),
userCredentials.getPassword());
String encodedAuth = Base64.getEncoder()
.encodeToString(userCredentialsUrlFormat.getBytes());
builder.header(HttpHeaders.AUTHORIZATION, "Basic " + encodedAuth);
} else if (userInfo != null) {
builder.uri(URI.create(decodedUrl.replace(uri.getRawUserInfo() + "@", "")));
String encodedAuth = Base64.getEncoder()
.encodeToString(userInfo.getBytes());
builder.header(HttpHeaders.AUTHORIZATION, "Basic " + encodedAuth);
Expand Down
Loading