Skip to content

fix(s3): use EmulatorConfig for FLOCI_HOSTNAME to fix native image crash #4

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

Open
Meemaw wants to merge 1 commit into
base: main
Choose a base branch
from
+43 −20
Open

fix(s3): use EmulatorConfig for FLOCI_HOSTNAME to fix native image crash #4

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
44 changes: 27 additions & 17 deletions src/main/java/io/github/hectorvent/floci/services/s3/S3VirtualHostFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
package io.github.hectorvent.floci.services.s3;

import jakarta.inject.Inject;
import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.container.ContainerRequestFilter;
import jakarta.ws.rs.container.PreMatching;
import jakarta.ws.rs.core.UriBuilder;
import jakarta.ws.rs.ext.Provider;
import org.eclipse.microprofile.config.inject.ConfigProperty;
import org.eclipse.microprofile.config.ConfigProvider;

import java.net.URI;
import java.util.Optional;
Expand All @@ -17,10 +16,13 @@ public class S3VirtualHostFilter implements ContainerRequestFilter {

private final String baseHostname;

@Inject
public S3VirtualHostFilter(
@ConfigProperty(name = "floci.base-url", defaultValue = "http://localhost:4566") String baseUrl,
@ConfigProperty(name = "floci.hostname") Optional<String> hostname) {
public S3VirtualHostFilter() {
var config = ConfigProvider.getConfig();
String baseUrl = config
.getOptionalValue("floci.base-url", String.class)
.orElse("http://localhost:4566");
Optional<String> hostname = config
.getOptionalValue("floci.hostname", String.class);
Comment on lines +19 to +25
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot Apr 8, 2026

Choose a reason for hiding this comment

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

🔴 Violation of AGENT.md "Use constructor injection" rule: replaces @Inject with manual ConfigProvider

The PR removes @Inject constructor injection and replaces it with ConfigProvider.getConfig(), directly violating the mandatory AGENT.md Code Style rule at line 268: "Use constructor injection". This is the only file in the entire codebase that uses ConfigProvider.getConfig() — every other class that needs configuration uses CDI injection (typically via EmulatorConfig or @ConfigProperty), as seen in ServiceEnabledFilter.java:29-32, PreSignedUrlFilter.java:16-19, AwsJsonMessageBodyWriter.java:15-18, and dozens of service classes. This also violates the AGENT.md rule "Follow existing project patterns" (line 272).

Prompt for agents 
The S3VirtualHostFilter constructor was changed from CDI constructor injection (@Inject + @ConfigProperty) to manual ConfigProvider.getConfig() usage. This violates the AGENT.md mandatory rules: 'Use constructor injection' and 'Follow existing project patterns'.

The entire rest of the codebase uses CDI injection for configuration, either via EmulatorConfig (the type-safe config interface) or @ConfigProperty annotations. This is the only class using ConfigProvider directly.

To fix this, restore the @Inject-based constructor injection. The original approach was:

@Inject
public S3VirtualHostFilter(
        @ConfigProperty(name = "floci.base-url", defaultValue = "http://localhost:4566") String baseUrl,
        @ConfigProperty(name = "floci.hostname") Optional<String> hostname) {
    // ... same body
}

Alternatively, consider injecting EmulatorConfig instead of individual @ConfigProperty values, which is the more common pattern in this codebase (used by S3Service, LambdaService, etc.).

If there was a genuine technical issue with CDI injection in @PreMatching filters, it should be documented and an exception to the rule should be noted in a code comment.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

String effectiveUrl = hostname
.map(h -> baseUrl.replaceFirst("://[^:/]+(:\\d+)?", "://" + h + "$1"))
.orElse(baseUrl);
Expand Down Expand Up @@ -71,19 +73,21 @@ public void filter(ContainerRequestContext requestContext) {
* matches a well-known AWS S3 domain pattern (for DNS-redirect setups).
*
* Examples with baseHostname="localhost":
* my-bucket.localhost:4566 → "my-bucket"
* my-bucket.localhost → "my-bucket"
* floci.svc.cluster.local → null (no bucket prefix, path-style)
* my-svc.floci.svc.cluster.local → null (remainder doesn't match "localhost")
* my-bucket.localhost:4566 -> "my-bucket"
* my-bucket.localhost -> "my-bucket"
* floci.svc.cluster.local -> null (no bucket prefix, path-style)
* my-svc.floci.svc.cluster.local -> null (remainder doesn't match "localhost")
*
* Examples with baseHostname="floci.svc.cluster.local":
* my-bucket.floci.svc.cluster.local → "my-bucket"
* floci.svc.cluster.local → null (no bucket prefix, path-style)
* my-bucket.floci.svc.cluster.local -> "my-bucket"
* floci.svc.cluster.local -> null (no bucket prefix, path-style)
*
* Returns null if the host does not match a virtual-hosted pattern.
*/
static String extractBucket(String host, String baseHostname) {
if (host == null) return null;
if (host == null) {
return null;
}

// Strip port if present
String hostname = stripPort(host);
Expand Down Expand Up @@ -117,7 +121,9 @@ static String extractBucket(String host, String baseHostname) {

/** Extracts the hostname (without scheme or port) from a URL string. */
static String extractHostnameFromUrl(String url) {
if (url == null) return null;
if (url == null) {
return null;
}
try {
URI uri = URI.create(url);
return uri.getHost();
Expand Down Expand Up @@ -147,11 +153,15 @@ private static boolean isIpv4Address(String hostname) {
return true;
}

/** Returns true for *.s3.amazonaws.com and *.s3.<region>.amazonaws.com domains. */
/** Returns true for *.s3.amazonaws.com and *.s3.region.amazonaws.com domains. */
private static boolean isAwsS3Domain(String remainder) {
if ("s3.amazonaws.com".equals(remainder)) return true;
if ("s3.amazonaws.com".equals(remainder)) {
return true;
}
// s3.<region>.amazonaws.com
if (remainder.startsWith("s3.") && remainder.endsWith(".amazonaws.com")) return true;
if (remainder.startsWith("s3.") && remainder.endsWith(".amazonaws.com")) {
return true;
}
return false;
}
}
19 changes: 16 additions & 3 deletions src/test/java/io/github/hectorvent/floci/services/s3/S3VirtualHostFilterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.github.hectorvent.floci.services.s3;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.NullSource;
Expand All @@ -9,7 +10,7 @@

class S3VirtualHostFilterTest {

// ── Virtual-hosted style: bucket prefix + matching baseHostname ─────────
// --- extractBucket with baseHostname ---

@ParameterizedTest
@CsvSource({
Expand All @@ -33,7 +34,7 @@ void extractsBucketFromVirtualHostedStyle(String host, String baseHostname, Stri
assertEquals(expectedBucket, S3VirtualHostFilter.extractBucket(host, baseHostname));
}

// ── Path-style: service hostname alone — must NOT extract a bucket ───────
// --- Path-style: service hostname alone — must NOT extract a bucket ---

@ParameterizedTest
@CsvSource({
Expand Down Expand Up @@ -70,7 +71,14 @@ void returnsNullForNullHost(String host) {
assertNull(S3VirtualHostFilter.extractBucket(host, "localhost"));
}

// ── Hostname extraction from URL ─────────────────────────────────────────
@Test
void returnsNullForNullBaseHostname() {
// Without a baseHostname, only AWS S3 domains should match
assertNull(S3VirtualHostFilter.extractBucket("my-bucket.localhost:4566", null));
assertEquals("my-bucket", S3VirtualHostFilter.extractBucket("my-bucket.s3.amazonaws.com", null));
}

// --- Hostname extraction from URL ---

@ParameterizedTest
@CsvSource({
Expand All @@ -83,4 +91,9 @@ void returnsNullForNullHost(String host) {
void extractsHostnameFromUrl(String url, String expectedHostname) {
assertEquals(expectedHostname, S3VirtualHostFilter.extractHostnameFromUrl(url));
}

@Test
void extractHostnameFromUrlReturnsNullForNull() {
assertNull(S3VirtualHostFilter.extractHostnameFromUrl(null));
}
}