Skip to content

Commit 31dbddd

Browse files
Flossyclaude
andcommitted
fix: add error accumulation, timeouts, and size validation to MavenRepositoryClassSource
- Accumulate all errors instead of silently swallowing exceptions - Add configurable connect timeout (10s default) and read timeout (30s default) - Add MAX_JAR_SIZE validation (100MB) before downloading - Add MAX_CLASS_SIZE validation (10MB) when reading class entries - Improve error messages with detailed failure information from all artifacts - Add connectTimeout() and readTimeout() builder methods The TOCTOU race condition was already fixed (using atomic get() on line 71). Fixes #60 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 265f5a2 commit 31dbddd

1 file changed

Lines changed: 91 additions & 8 deletions

File tree

src/main/java/org/flossware/classloader/MavenRepositoryClassSource.java

Lines changed: 91 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,32 +26,61 @@
2626
* ConcurrentHashMap to support concurrent class loading operations.</p>
2727
*/
2828
public class MavenRepositoryClassSource implements ClassSource {
29+
private static final long MAX_JAR_SIZE = 100 * 1024 * 1024; // 100MB default max JAR size
30+
private static final long MAX_CLASS_SIZE = 10 * 1024 * 1024; // 10MB default max class size
31+
private static final int DEFAULT_CONNECT_TIMEOUT = 10000; // 10 seconds
32+
private static final int DEFAULT_READ_TIMEOUT = 30000; // 30 seconds
33+
2934
private final String repositoryUrl;
3035
private final List<MavenArtifact> artifacts;
3136
private final AuthConfig authConfig;
3237
private final Map<String, byte[]> classCache;
38+
private final int connectTimeout;
39+
private final int readTimeout;
3340

3441
/**
35-
* Creates a Maven repository class source with full configuration.
42+
* Creates a Maven repository class source with full configuration including timeouts.
3643
*
3744
* @param repositoryUrl The Maven repository URL (e.g., "https://repo1.maven.org/maven2/")
3845
* @param artifacts The list of Maven artifacts to load classes from
3946
* @param authConfig The authentication configuration
47+
* @param connectTimeout Connection timeout in milliseconds
48+
* @param readTimeout Read timeout in milliseconds
4049
* @throws NullPointerException if repositoryUrl or artifacts is null
41-
* @throws IllegalArgumentException if artifacts list is empty
50+
* @throws IllegalArgumentException if artifacts list is empty or timeouts are negative
4251
*/
43-
public MavenRepositoryClassSource(String repositoryUrl, List<MavenArtifact> artifacts, AuthConfig authConfig) {
52+
public MavenRepositoryClassSource(String repositoryUrl, List<MavenArtifact> artifacts, AuthConfig authConfig,
53+
int connectTimeout, int readTimeout) {
4454
Objects.requireNonNull(repositoryUrl, "repositoryUrl cannot be null");
4555
Objects.requireNonNull(artifacts, "artifacts cannot be null");
4656

4757
if (artifacts.isEmpty()) {
4858
throw new IllegalArgumentException("At least one Maven artifact must be specified");
4959
}
60+
if (connectTimeout < 0) {
61+
throw new IllegalArgumentException("connectTimeout must be >= 0");
62+
}
63+
if (readTimeout < 0) {
64+
throw new IllegalArgumentException("readTimeout must be >= 0");
65+
}
5066

5167
this.repositoryUrl = repositoryUrl.endsWith("/") ? repositoryUrl : repositoryUrl + "/";
5268
this.artifacts = new ArrayList<>(artifacts);
5369
this.authConfig = authConfig != null ? authConfig : AuthConfig.none();
5470
this.classCache = new ConcurrentHashMap<>();
71+
this.connectTimeout = connectTimeout;
72+
this.readTimeout = readTimeout;
73+
}
74+
75+
/**
76+
* Creates a Maven repository class source with default timeouts.
77+
*
78+
* @param repositoryUrl The Maven repository URL
79+
* @param artifacts The list of Maven artifacts to load classes from
80+
* @param authConfig The authentication configuration
81+
*/
82+
public MavenRepositoryClassSource(String repositoryUrl, List<MavenArtifact> artifacts, AuthConfig authConfig) {
83+
this(repositoryUrl, artifacts, authConfig, DEFAULT_CONNECT_TIMEOUT, DEFAULT_READ_TIMEOUT);
5584
}
5685

5786
/**
@@ -74,6 +103,7 @@ public byte[] loadClassData(String className) throws IOException {
74103
}
75104

76105
String classFileName = ClassNameUtil.toClassFilePath(className);
106+
List<String> errorMessages = new ArrayList<>();
77107

78108
for (MavenArtifact artifact : artifacts) {
79109
try {
@@ -82,11 +112,19 @@ public byte[] loadClassData(String className) throws IOException {
82112
classCache.put(cacheKey, classData);
83113
return classData;
84114
} catch (IOException e) {
85-
// Continue to next artifact if class not found in this one
115+
// Accumulate errors instead of silently swallowing
116+
String errorMsg = String.format("Artifact %s - %s",
117+
artifact.toString(), e.getMessage());
118+
errorMessages.add(errorMsg);
86119
}
87120
}
88121

89-
throw new IOException("Class not found in any configured Maven artifacts: " + className);
122+
// Throw with ALL error details
123+
String allErrors = String.join("\n - ", errorMessages);
124+
throw new IOException(
125+
"Class not found in any of " + artifacts.size() + " configured Maven artifacts: " +
126+
className + "\nAttempted artifacts:\n - " + allErrors
127+
);
90128
}
91129

92130
@Override
@@ -112,12 +150,22 @@ private String buildJarUrl(MavenArtifact artifact) {
112150
private byte[] extractClassFromJar(String jarUrl, String classFileName) throws IOException {
113151
URL url = new URL(jarUrl);
114152
HttpURLConnection connection = (HttpURLConnection) url.openConnection();
153+
connection.setConnectTimeout(connectTimeout);
154+
connection.setReadTimeout(readTimeout);
115155
AuthHelper.configureAuth(connection, authConfig);
116156
connection.setRequestMethod("GET");
117157

118158
int responseCode = connection.getResponseCode();
119159
if (responseCode != HttpURLConnection.HTTP_OK) {
120-
throw new IOException("HTTP error code: " + responseCode + " for JAR URL: " + jarUrl);
160+
throw new IOException("HTTP " + responseCode + " for JAR: " + jarUrl);
161+
}
162+
163+
// Check JAR size before downloading
164+
long contentLength = connection.getContentLengthLong();
165+
if (contentLength > MAX_JAR_SIZE) {
166+
throw new IOException(
167+
"JAR too large: " + contentLength + " bytes (max " + MAX_JAR_SIZE + ")"
168+
);
121169
}
122170

123171
try (InputStream in = connection.getInputStream();
@@ -126,18 +174,34 @@ private byte[] extractClassFromJar(String jarUrl, String classFileName) throws I
126174
JarEntry entry;
127175
while ((entry = jarIn.getNextJarEntry()) != null) {
128176
if (entry.getName().equals(classFileName) && !entry.isDirectory()) {
177+
long size = entry.getSize();
178+
179+
if (size > MAX_CLASS_SIZE) {
180+
throw new IOException(
181+
"Class too large: " + size + " bytes (max " + MAX_CLASS_SIZE + ")"
182+
);
183+
}
184+
185+
// Read with size enforcement
129186
ByteArrayOutputStream out = new ByteArrayOutputStream();
130187
byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
188+
long totalRead = 0;
131189
int bytesRead;
190+
132191
while ((bytesRead = jarIn.read(buffer)) != -1) {
192+
totalRead += bytesRead;
193+
if (totalRead > MAX_CLASS_SIZE) {
194+
throw new IOException("Class exceeded size limit: " + totalRead);
195+
}
133196
out.write(buffer, 0, bytesRead);
134197
}
198+
135199
return out.toByteArray();
136200
}
137201
}
138202
}
139203

140-
throw new IOException("Class file not found in JAR: " + classFileName);
204+
throw new IOException("Class not found in JAR: " + classFileName + " (URL: " + jarUrl + ")");
141205
}
142206

143207
public void addArtifact(MavenArtifact artifact) {
@@ -165,6 +229,8 @@ public static class Builder {
165229
private String repositoryUrl;
166230
private final List<MavenArtifact> artifacts = new ArrayList<>();
167231
private AuthConfig authConfig = AuthConfig.none();
232+
private int connectTimeout = DEFAULT_CONNECT_TIMEOUT;
233+
private int readTimeout = DEFAULT_READ_TIMEOUT;
168234

169235
public Builder repositoryUrl(String repositoryUrl) {
170236
this.repositoryUrl = Objects.requireNonNull(repositoryUrl, "repositoryUrl cannot be null");
@@ -194,9 +260,26 @@ public Builder auth(AuthConfig authConfig) {
194260
return this;
195261
}
196262

263+
public Builder connectTimeout(int timeoutMs) {
264+
if (timeoutMs < 0) {
265+
throw new IllegalArgumentException("connectTimeout must be >= 0");
266+
}
267+
this.connectTimeout = timeoutMs;
268+
return this;
269+
}
270+
271+
public Builder readTimeout(int timeoutMs) {
272+
if (timeoutMs < 0) {
273+
throw new IllegalArgumentException("readTimeout must be >= 0");
274+
}
275+
this.readTimeout = timeoutMs;
276+
return this;
277+
}
278+
197279
public MavenRepositoryClassSource build() {
198280
Objects.requireNonNull(repositoryUrl, "repositoryUrl must be set");
199-
return new MavenRepositoryClassSource(repositoryUrl, artifacts, authConfig);
281+
return new MavenRepositoryClassSource(repositoryUrl, artifacts, authConfig,
282+
connectTimeout, readTimeout);
200283
}
201284
}
202285

0 commit comments

Comments
 (0)