Skip to content

Commit b0fe146

Browse files
committed
style(appsec): address PR review feedback on GlassFish multipart instrumentation
- Use try-with-resources for OutputStream in GlassFishBlockingHelper - Cache Config limit values as static finals in GlassFishBlockingHelper to avoid per-request Config.get() calls (consistent with ParameterCollector) - Remove Config import from GlassFishMultipartInstrumentation (now reads limits from GlassFishBlockingHelper static fields) - Add comment to skipVersions explaining the jakarta namespace reason - Add assertInverse to glassfish muzzle block for defensive CI coverage
1 parent 6e7f129 commit b0fe146

3 files changed

Lines changed: 11 additions & 7 deletions

File tree

dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ muzzle {
2020
group = 'org.glassfish.main.extras'
2121
module = 'glassfish-embedded-all'
2222
versions = '[4.0,]'
23-
skipVersions += '6.1.0'
23+
skipVersions += '6.1.0' // GlassFish 6.1.0+ uses jakarta.* namespace; our advice uses javax.servlet.http.Part
24+
assertInverse = true
2425
}
2526
}
2627

dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelper.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package datadog.trace.instrumentation.tomcat7;
22

33
import datadog.appsec.api.blocking.BlockingContentType;
4+
import datadog.trace.api.Config;
45
import datadog.trace.api.gateway.Flow;
56
import datadog.trace.bootstrap.blocking.BlockingActionHelper;
67
import java.io.OutputStream;
@@ -10,6 +11,9 @@
1011

1112
public final class GlassFishBlockingHelper {
1213

14+
static final int MAX_FILE_CONTENT_COUNT = Config.get().getAppSecMaxFileContentCount();
15+
static final int MAX_FILE_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes();
16+
1317
public static boolean commitBlocking(
1418
HttpServletRequest request,
1519
HttpServletResponse response,
@@ -34,9 +38,9 @@ public static boolean commitBlocking(
3438
if (body != null) {
3539
response.setHeader("Content-Type", BlockingActionHelper.getContentType(type));
3640
response.setHeader("Content-Length", Integer.toString(body.length));
37-
OutputStream os = response.getOutputStream();
38-
os.write(body);
39-
os.close();
41+
try (OutputStream os = response.getOutputStream()) {
42+
os.write(body);
43+
}
4044
}
4145
}
4246
response.flushBuffer();

dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishMultipartInstrumentation.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import datadog.appsec.api.blocking.BlockingException;
1010
import datadog.trace.agent.tooling.Instrumenter;
1111
import datadog.trace.agent.tooling.InstrumenterModule;
12-
import datadog.trace.api.Config;
1312
import datadog.trace.api.gateway.BlockResponseFunction;
1413
import datadog.trace.api.gateway.CallbackProvider;
1514
import datadog.trace.api.gateway.Flow;
@@ -127,8 +126,8 @@ static void after(
127126
} catch (Exception ignored) {
128127
}
129128

130-
int maxFiles = Config.get().getAppSecMaxFileContentCount();
131-
int maxBytes = Config.get().getAppSecMaxFileContentBytes();
129+
int maxFiles = GlassFishBlockingHelper.MAX_FILE_CONTENT_COUNT;
130+
int maxBytes = GlassFishBlockingHelper.MAX_FILE_CONTENT_BYTES;
132131

133132
List<String> filenames = null;
134133
List<String> contents = null;

0 commit comments

Comments
 (0)