Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
dfa0897
feat: Replace JClouds with AWS SDK and NIO [DHIS2-20648]
david-mackessy May 14, 2026
5715363
feat: Align impl contracts for consistency. Add comments [DHIS2-20648]
david-mackessy May 14, 2026
28c97bf
feat: Remove remaining JClouds refs [DHIS2-20648]
david-mackessy May 14, 2026
4d6bcb7
feat: Harden security and add more tests [DHIS2-20648]
david-mackessy May 14, 2026
2603559
feat: Clean logs [DHIS2-20648]
david-mackessy May 14, 2026
0ca8f4c
Merge branch 'master' into DHIS2-20648_4
david-mackessy May 15, 2026
9ccf9ad
Revert get content length
david-mackessy May 18, 2026
6db21f1
feat: Enable dir create/exists to keep existing behaviour
david-mackessy May 18, 2026
5abf6f1
clean up tests
david-mackessy May 18, 2026
de63752
feat: Use static of
david-mackessy May 19, 2026
34f5288
Merge branch 'master' into DHIS2-20648_4
david-mackessy May 19, 2026
6a8905e
feat: Craete bucket first without HEAD call
david-mackessy May 19, 2026
c64f5fd
feat: Address PR comments
david-mackessy May 19, 2026
7b9e7b8
remove redundant tests
david-mackessy May 19, 2026
0794e47
feat: Remove AppStorageSource, no longer required
david-mackessy May 19, 2026
67a3ed9
Merge branch 'master' into DHIS2-20648_4
david-mackessy May 20, 2026
a85f024
Merge branch 'master' into DHIS2-20648_4
david-mackessy May 21, 2026
cc27c0c
feat: Ensure streams are resetable when passing to s3 [DHIS2-20648]
david-mackessy May 21, 2026
9473b2c
Merge branch 'master' into DHIS2-20648_4
david-mackessy May 25, 2026
ea8041c
Merge branch 'master' into DHIS2-20648_4
david-mackessy May 25, 2026
3b44104
Merge branch 'master' into DHIS2-20648_4
david-mackessy May 27, 2026
a6a3d0e
merge master
david-mackessy May 28, 2026
4767550
resolve conflicts
david-mackessy May 28, 2026
4cd222d
remove test util method
david-mackessy May 28, 2026
cff096d
Merge branch 'master' into DHIS2-20648_4
stian-sandvold May 29, 2026
29be1a6
Merge branch 'master' into DHIS2-20648_4
david-mackessy Jun 2, 2026
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
12 changes: 0 additions & 12 deletions dhis-2/dhis-api/src/main/java/org/hisp/dhis/appmanager/App.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ public class App implements Serializable {

private String defaultLocale;

private AppStorageSource appStorageSource;

private String folderName;

/** Optional. */
Expand Down Expand Up @@ -414,22 +412,12 @@ public void setBaseUrl(String baseUrl) {
this.baseUrl = baseUrl;
}

@JsonProperty
@JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0)
public AppStorageSource getAppStorageSource() {
return appStorageSource;
}

@JsonProperty
@JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0)
public List<AppShortcut> getShortcuts() {
return shortcuts;
}

public void setAppStorageSource(AppStorageSource appStorageSource) {
this.appStorageSource = appStorageSource;
}

@JsonProperty
@JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0)
public Set<String> getAuthorities() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public BlobKey resolve(String resource) {
* org.hisp.dhis.storage.BlobStoreService#deleteDirectory}.
*/
public BlobKeyPrefix asPrefix() {
return new BlobKeyPrefix(path);
return BlobKeyPrefix.of(path);
}

@Nonnull
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public String getStorageKey() {
* rather than {@link String}.
*/
public BlobKey asBlobKey() {
return new BlobKey(storageKey);
return BlobKey.of(storageKey);
}

public void setStorageKey(String storageKey) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
* #resolve(BlobKey)} always produces a clean path without any slash-cleaning.
*
* <p>Configured via {@link org.hisp.dhis.external.conf.ConfigurationKey#FILESTORE_CONTAINER} and
* resolved once at startup by {@link org.hisp.dhis.jclouds.JCloudsStore}.
* resolved once at startup by the {@link org.hisp.dhis.storage.BlobStoreService} implementation
* selected for the configured {@code filestore.provider}.
*/
public record BlobContainerName(String value) {

Expand Down
10 changes: 10 additions & 0 deletions dhis-2/dhis-api/src/main/java/org/hisp/dhis/storage/BlobKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ public record BlobKey(String value) {
throw new IllegalArgumentException(
"BlobKey value must not exceed " + MAX_KEY_LENGTH + " characters: " + value);
}
// Reject `..` as a path segment so filesystem-backed stores can't be tricked into writing
// outside the container directory. Substrings like "a..b" are fine — only segments split by
// `/` count. No DHIS2 caller produces `..` segments today (zip uploads sanitise via
// ZipFileUtils, file-resource storage keys are UUID-shaped) so this is defense-in-depth.
for (String segment : value.split("/", -1)) {
if ("..".equals(segment)) {
throw new IllegalArgumentException(
"BlobKey value must not contain '..' as a path segment: " + value);
}
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ public record BlobKeyPrefix(@Nonnull String value) {
/**
* Creates a {@link BlobKeyPrefix} from {@code value}, stripping any leading or trailing {@code /}
* before construction. Prefer this over the canonical constructor when the input may carry
* store-native separators (e.g. JClouds appends a trailing {@code /} to folder names).
* store-native separators (e.g. S3's {@code CommonPrefix} entries arrive with a trailing {@code
* /}).
*
* <p>Examples:
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,26 @@ void ofSingleSegment() {
void ofJoinsSegmentsWithSlash() {
assertEquals("apps/my-app/index.html", BlobKey.of("apps", "my-app", "index.html").value());
}

@Test
void leadingDotDotSegmentIsRejected() {
assertThrows(IllegalArgumentException.class, () -> new BlobKey("../escape.txt"));
}

@Test
void interiorDotDotSegmentIsRejected() {
assertThrows(IllegalArgumentException.class, () -> new BlobKey("a/../b/c"));
}

@Test
void trailingDotDotSegmentIsRejected() {
assertThrows(IllegalArgumentException.class, () -> new BlobKey("a/b/.."));
}

@Test
void dotDotAsSubstringWithinSegmentIsAccepted() {
// "a..b" is a perfectly legitimate filename — only `..` as a *segment* is rejected.
BlobKey key = new BlobKey("apps/my..app/index.html");
assertEquals("apps/my..app/index.html", key.value());
}
}
25 changes: 2 additions & 23 deletions dhis-2/dhis-services/dhis-service-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -251,32 +251,11 @@
<artifactId>annotations</artifactId>
</dependency>

<!-- Apache JClouds -->

<dependency>
<groupId>org.apache.jclouds</groupId>
<artifactId>jclouds-core</artifactId>
</dependency>
<!-- AWS SDK for Java v2 -->
<dependency>
<groupId>org.apache.jclouds</groupId>
<artifactId>jclouds-blobstore</artifactId>
</dependency>
<dependency>
<groupId>org.apache.jclouds.api</groupId>
<artifactId>filesystem</artifactId>
</dependency>
<dependency>
<groupId>org.apache.jclouds.api</groupId>
<groupId>software.amazon.awssdk</groupId>
<artifactId>s3</artifactId>
</dependency>
<dependency>
<groupId>org.apache.jclouds.provider</groupId>
<artifactId>aws-s3</artifactId>
</dependency>
<dependency>
<groupId>org.apache.jclouds.driver</groupId>
<artifactId>jclouds-slf4j</artifactId>
</dependency>

<!-- Image Processing -->

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URI;
import java.util.Collections;
import java.util.Enumeration;
Expand Down Expand Up @@ -69,6 +68,7 @@
import org.hisp.dhis.util.ZipBombException;
import org.hisp.dhis.util.ZipFileUtils;
import org.hisp.dhis.util.ZipSlipException;
import org.springframework.core.io.ByteArrayResource;
import org.springframework.core.io.FileSystemResource;
import org.springframework.core.io.Resource;
import org.springframework.core.io.UrlResource;
Expand All @@ -79,8 +79,8 @@
*/
@Slf4j
@RequiredArgsConstructor
@Service("org.hisp.dhis.appmanager.JCloudsAppStorageService")
public class JCloudsAppStorageService implements AppStorageService {
@Service
public class BlobStoreAppStorageService implements AppStorageService {

private static final String BUNDLED_APP_INFO_FILENAME = "bundled-app-info.json";
public static final String MANIFEST_WEBAPP_FILENAME = "manifest.webapp";
Expand Down Expand Up @@ -118,7 +118,6 @@ private void handleAppManifest(
BiConsumer<App, BundledAppInfo> handler, BlobKeyPrefix folder, InputStream manifestStream) {
try (InputStream inputStream = manifestStream) {
App app = App.MAPPER.readValue(inputStream, App.class);
app.setAppStorageSource(AppStorageSource.JCLOUDS);
app.setFolderName(folder.value());
app.setManifestTranslations(
readAppManifestTranslations(
Expand Down Expand Up @@ -234,7 +233,6 @@ public App installApp(
app = AppManager.readAppManifest(file, this.jsonMapper, topLevelFolder);
folder = AppFolderName.ofKey(app.getKey());
app.setFolderName(folder.path());
app.setAppStorageSource(AppStorageSource.JCLOUDS);
} catch (IOException e) {
log.error("Failed to install app: Failure during reading manifest from zip file", e);
app = new App();
Expand Down Expand Up @@ -294,9 +292,15 @@ private void unzipFile(File file, AppFolderName folder, String topLevelFolder)
String filePath = getFilePath(folder.path(), topLevelFolder, zipEntry);
// If it's the root folder, skip
if (filePath == null) continue;
if (zipEntry.isDirectory()) {
// Materialise empty directories so the /api/apps/<app>/<dir> → /<dir>/ redirect
// contract holds even when the zip contains a directory with no files inside.
blobStore.createDirectory(BlobKeyPrefix.of(filePath));
continue;
}
try (InputStream zipInputStream = zipFile.getInputStream(zipEntry)) {
blobStore.putBlob(
new BlobKey(filePath), zipInputStream, zipEntry.getSize(), null, null, null);
BlobKey.of(filePath), zipInputStream, zipEntry.getSize(), null, null, null);
}
}
}
Expand Down Expand Up @@ -338,30 +342,17 @@ public void deleteApp(@Nonnull App app) {
AppFolderName folder = app.appFolder();
blobStore.deleteBlob(folder.resolve(MANIFEST_WEBAPP_FILENAME));

// TODO(DHIS2-20648) Once the replacement BlobStoreService implementation does recursive
// deleteDirectory on every backend (see contract test), this branch can collapse to a
// single blobStore.deleteDirectory(folder.asPrefix()) call.
if (blobStore.isFilesystem()) {
// Delete all files related to app (works for local filestore)
blobStore.deleteDirectory(folder.asPrefix());
} else {
// S3: deleteDirectory is not recursive, so enumerate and delete individually
for (BlobKey key : blobStore.listKeys(folder.asPrefix())) {
log.debug("Deleting app file: {}", key);
blobStore.deleteBlob(key);
}
}
blobStore.deleteDirectory(folder.asPrefix());
log.info("Deleted app {}", app.getName());
}

@Override
@Nonnull
public ResourceResult getAppResource(@CheckForNull App app, @Nonnull String resource)
throws IOException {
if (app == null || !app.getAppStorageSource().equals(AppStorageSource.JCLOUDS)) {
if (app == null) {
log.warn(
"Can't look up resource {}. The specified app was not found in JClouds storage.",
resource);
"Can't look up resource {}. The specified app was not found in blob storage.", resource);
return new ResourceNotFound(resource);
}
if (resource.isBlank()) {
Expand All @@ -374,25 +365,28 @@ public ResourceResult getAppResource(@CheckForNull App app, @Nonnull String reso
if (blobStore.blobExists(key)) {
return new ResourceFound(getResource(key));
}
if (keyExistsAsDirectory(key)) {
if (blobStore.directoryExists(BlobKeyPrefix.of(key.value()))) {
return new Redirect(resource + "/");
}
log.debug("ResourceNotFound {} for App {}", key, app.getName());
return new ResourceNotFound(resource);
}

private boolean keyExistsAsDirectory(BlobKey key) {
return blobStore.listKeys(BlobKeyPrefix.of(key.value())).iterator().hasNext();
}

private Resource getResource(@Nonnull BlobKey key) throws MalformedURLException {
private Resource getResource(@Nonnull BlobKey key) throws IOException {
if (blobStore.isFilesystem()) {
return new FileSystemResource(
locationManager.getFileForReading(blobStore.container().resolve(key)));
} else {
URI uri = fileResourceContentStore.getSignedGetContentUri(key);
}
URI uri = fileResourceContentStore.getSignedGetContentUri(key);
if (uri != null) {
return new UrlResource(uri);
}
// When backend supports neither filesystem access nor signed URLs (e.g. transient): buffer
// the blob bytes so Spring can serve them directly.
try (InputStream in = blobStore.openStream(key)) {
if (in == null) throw new IOException("Blob not found: " + key);
return new ByteArrayResource(in.readAllBytes());
}
}

/**
Expand Down Expand Up @@ -429,12 +423,11 @@ private static void logInstallSuccess(App app, String appFolder) {

private static void logDiscoveredApps(Map<String, Pair<App, BundledAppInfo>> apps) {
if (apps.isEmpty()) {
log.info("No apps found during JClouds discovery.");
log.info("No apps found during blob store discovery");
} else {
apps.values()
.forEach(
pair ->
log.info("Discovered app '{}' from JClouds storage ", pair.getLeft().getName()));
pair -> log.info("Discovered app '{}' from blob storage ", pair.getLeft().getName()));
}
}
}
Loading
Loading