CRITICAL: MinioClassSource has 7+ production-breaking issues

[sfloess]

Severity: CRITICAL

File: MinioClassSource.java

Problem

MinioClassSource has multiple critical bugs that will cause hangs, OOM crashes, and production failures. This code is NOT production-ready.

Bugs

1. Lines 42-46: No timeout configuration - infinite hangs

try (InputStream stream = minioClient.getObject(
        GetObjectArgs.builder()
            .bucket(bucketName)
            .object(objectName)
            .build());

Problem: No connection timeout, no read timeout

• Slow network? Thread hangs FOREVER
• Network partition? Thread hangs FOREVER
• Dead server? Thread hangs FOREVER

Impact: Under load, ALL threads hang waiting for MinIO responses. Application becomes unresponsive. Requires kill -9 to recover.

Fix: MinIO SDK supports timeouts in MinioClient builder:

MinioClient client = MinioClient.builder()
    .endpoint(endpoint, port, secure)
    .credentials(accessKey, secretKey)
    .connectTimeout(10, TimeUnit.SECONDS)      // ← Add this
    .writeTimeout(30, TimeUnit.SECONDS)        // ← Add this
    .readTimeout(30, TimeUnit.SECONDS)         // ← Add this
    .build();

2. Lines 49-53: No size validation - OOM vulnerability

byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
int bytesRead;
while ((bytesRead = stream.read(buffer)) != -1) {
    out.write(buffer, 0, bytesRead);
}

Problem: Downloads entire object into memory with no size check

Attack scenario:

1. Attacker uploads 2GB class file to MinIO bucket
2. JClassLoader tries to load it
3. ByteArrayOutputStream allocates 2GB
4. OOM crash - entire JVM dies

Fix: Check object size BEFORE downloading:

@Override
public byte[] loadClassData(String className) throws IOException {
    String objectName = buildObjectName(className);
    
    // Check size first
    StatObjectResponse stat;
    try {
        stat = minioClient.statObject(
            StatObjectArgs.builder()
                .bucket(bucketName)
                .object(objectName)
                .build());
    } catch (Exception e) {
        throw new IOException("Failed to stat object: " + objectName, e);
    }
    
    long size = stat.size();
    if (size > MAX_CLASS_SIZE) {  // e.g., 10MB
        throw new IOException("Class file too large: " + size + " bytes (max " + MAX_CLASS_SIZE + ")");
    }
    if (size > Integer.MAX_VALUE) {
        throw new IOException("Class file exceeds Java array limit: " + size);
    }
    
    // Now safe to download
    try (InputStream stream = minioClient.getObject(...)) {
        byte[] data = new byte[(int)size];
        int totalRead = 0;
        while (totalRead < size) {
            int n = stream.read(data, totalRead, (int)size - totalRead);
            if (n == -1) break;
            totalRead += n;
        }
        return data;
    }
}

3. Line 57: Catches generic Exception - masks programming errors

} catch (Exception e) {
    throw new IOException("Failed to load class from MinIO: " + objectName, e);
}

Problem: Catches EVERYTHING including:

• NullPointerException (programming error)
• IllegalStateException (programming error)
• OutOfMemoryError (critical JVM state)
• ThreadDeath (MUST NOT be caught!)

Fix: Only catch specific exceptions:

} catch (MinioException e) {
    throw new IOException("MinIO error loading class: " + objectName, e);
} catch (IOException e) {
    throw e;
}
// Let RuntimeExceptions and Errors propagate

4. Line 74: canLoad() catches generic Exception and returns false

try {
    minioClient.statObject(...);
    return true;
} catch (Exception e) {
    return false;
}

Problem: Cannot distinguish error types

• Object doesn't exist? Returns false
• Network timeout? Returns false
• Auth failure? Returns false
• Programming error (NPE)? Returns false

All errors look the same. Debugging impossible.

Consequence: If MinIO credentials are wrong, canLoad() silently returns false for EVERY class. JClassLoader tries ALL sources, finds nothing, fails. Error message: "ClassNotFoundException". No indication the problem is MinIO auth.

5. Line 164: Port hardcoded to 443

MinioClient.Builder clientBuilder = MinioClient.builder()
    .endpoint(endpoint, 443, secure)  // ← HARDCODED!

Problem: Cannot connect to MinIO on non-standard ports

Reality:

• Local dev MinIO runs on port 9000
• Docker MinIO often uses port 9000
• Corporate environments use custom ports

Impact: MinioClassSource is UNUSABLE for 90% of MinIO deployments

Fix:

public static class Builder {
    private int port = 443;  // Default for HTTPS
    
    public Builder port(int port) {
        if (port < 1 || port > 65535) {
            throw new IllegalArgumentException("Port must be 1-65535");
        }
        this.port = port;
        return this;
    }
    
    public MinioClassSource build() {
        // ...
        MinioClient client = MinioClient.builder()
            .endpoint(endpoint, port, secure)  // Use configured port
            .credentials(accessKey, secretKey)
            .build();
    }
}

6. Lines 94-97: Misleading close() implementation

@Override
public void close() throws IOException {
    // MinioClient does not provide a close() method in the current SDK
    // Connection pools are managed internally and cleaned up on JVM shutdown
}

Problem: Implements AutoCloseable but close() does nothing

Users write:

try (MinioClassSource source = MinioClassSource.builder()...build()) {
    loader.addClassSource(source);
    // Use loader
}  // ← User expects connections to close here. THEY DON'T.

Impact: Connection pool leaks if users create many MinioClassSource instances

Fix: Either:

1. Don't implement AutoCloseable if you can't close
2. Document clearly that close() is a no-op
3. Add connection pooling control to the API

7. Lines 158-161: Builder validation happens too late

public MinioClassSource build() {
    Objects.requireNonNull(endpoint, "endpoint must be set");
    Objects.requireNonNull(accessKey, "accessKey must be set");
    // ...
}

Problem: Errors happen at build() time, not setter time

Bad UX:

MinioClassSource.Builder builder = MinioClassSource.builder();
builder.endpoint("http://localhost:9000");
builder.accessKey("mykey");
// Forgot secretKey
// ... 100 lines of code ...
MinioClassSource source = builder.build();  // ← Error HERE, far from the mistake

Fix: Validate in setters:

public Builder endpoint(String endpoint) {
    this.endpoint = Objects.requireNonNull(endpoint, "endpoint cannot be null");
    return this;
}

public Builder accessKey(String accessKey) {
    this.accessKey = Objects.requireNonNull(accessKey, "accessKey cannot be null");
    return this;
}

Required Actions

1. Add timeout configuration to MinioClient builder
2. Add MAX_CLASS_SIZE validation (default 10MB, configurable)
3. Replace generic Exception catches with specific exception types
4. Add port configuration to Builder
5. Either remove AutoCloseable or implement close() properly
6. Move null checks to setter methods
7. Add comprehensive error messages with troubleshooting hints

Impact

Without these fixes:

• Hangs: Production systems will hang under network issues
• OOM crashes: Malicious or corrupted data causes JVM crashes
• Cannot deploy: Hardcoded port 443 makes this unusable for most deployments
• Impossible to debug: Generic exception handling hides root causes

This class needs a complete rewrite before it can be used in production.

[sfloess]

Fixed. Added size validation (10MB max, configurable), configurable port (default 443 HTTPS, auto-adjusts to 9000 for HTTP), and improved exception handling. Timeout configuration note added for SDK version compatibility.