Skip to content

Commit 02889c9

Browse files
committed
fix: Make ArtifactorySearchResponseHandler gracefully handle absence of detail
Resolves the NPE reported in jeremylong/DependencyCheck#7254 (comment) Logs a warning for each entry in the response that does not have the checksums block that is to be expected for any request that has the `X-Result-Detail: info` HTTP header.
1 parent 242be38 commit 02889c9

File tree

2 files changed

+84
-0
lines changed

2 files changed

+84
-0
lines changed

core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandler.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,12 @@ public List<MavenArtifact> handleResponse(ClassicHttpResponse response) throws I
148148
do {
149149
final FileImpl file = fileImplReader.readValue(parser);
150150

151+
if (file.getChecksums() == null) {
152+
LOGGER.warn("No checksums found in artifactory search result of uri {}. Please make sure that header X-Result-Detail is retained on any (reverse)-proxy, loadbalancer or WebApplicationFirewall in the network path to your Artifactory Server",
153+
file.getUri());
154+
continue;
155+
}
156+
151157
final Optional<Matcher> validationResult = validateUsability(file);
152158
if (validationResult.isEmpty()) {
153159
continue;

core/src/test/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandlerTest.java

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,18 @@
1717
*/
1818
package org.owasp.dependencycheck.data.artifactory;
1919

20+
import ch.qos.logback.classic.Level;
21+
import ch.qos.logback.classic.Logger;
22+
import ch.qos.logback.classic.spi.ILoggingEvent;
23+
import ch.qos.logback.core.read.ListAppender;
2024
import org.apache.hc.core5.http.ClassicHttpResponse;
2125
import org.apache.hc.core5.http.HttpEntity;
2226
import org.junit.Before;
2327
import org.junit.Test;
2428
import org.owasp.dependencycheck.BaseTest;
2529
import org.owasp.dependencycheck.data.nexus.MavenArtifact;
2630
import org.owasp.dependencycheck.dependency.Dependency;
31+
import org.slf4j.LoggerFactory;
2732

2833
import java.io.ByteArrayInputStream;
2934
import java.io.FileNotFoundException;
@@ -135,6 +140,66 @@ public void shouldProcessCorrectlyArtifactoryAnswerWithMultipleMatches() throws
135140
artifact2.getPomUrl());
136141
}
137142

143+
/**
144+
* Tests the correct working for responses that are sent by Artifactory when the {@code X-Result-Detail} HTTP-Header is missing (e.g. when it
145+
* gets stripped by an intermediate WebApplicationFirewall configured to only allow a defined subset of HTTP-headers).
146+
* @throws IOException
147+
*/
148+
@Test
149+
public void shouldProcessCorrectlyForMissingXResultDetailHeader() throws IOException {
150+
// Inject logback ListAppender to capture test-logs from ArtifactorySearchResponseHandler
151+
final Logger sutLogger = (Logger) LoggerFactory.getLogger(ArtifactorySearchResponseHandler.class);
152+
final ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
153+
listAppender.start();
154+
sutLogger.addAppender(listAppender);
155+
156+
// Given
157+
final Dependency dependency = new Dependency();
158+
dependency.setFileName("freemarker-2.3.33.jar");
159+
dependency.setSha256sum("ab829182363e747a1530a368436242f4cca7ff309dd29bfca638a1fdc7b6771d");
160+
dependency.setSha1sum("fecaeb606993fc9fd0f95fe5a644048a69c39474");
161+
dependency.setMd5sum("4ec135628fd640a201c1d4f8670cc020");
162+
final ClassicHttpResponse response = mock(ClassicHttpResponse.class);
163+
final HttpEntity responseEntity = mock(HttpEntity.class);
164+
final byte[] payload = noXResultDetailHeaderResponse();
165+
when(response.getEntity()).thenReturn(responseEntity);
166+
when(responseEntity.getContent()).thenReturn(new ByteArrayInputStream(payload));
167+
168+
169+
// When
170+
final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency);
171+
try {
172+
handler.handleResponse(response);
173+
fail("Result with no details due to missing X-Result-Detail header, should throw an exception!");
174+
} catch (FileNotFoundException e) {
175+
// Then
176+
assertEquals("Artifact Dependency{ fileName='freemarker-2.3.33.jar', actualFilePath='null', filePath='null', packagePath='null'} not found in Artifactory; discovered sha1 hits not recognized as matching maven artifacts",
177+
e.getMessage());
178+
179+
// There should be a WARN-log for for each of the results regarding the absence of X-Result-Detail header driven attributes
180+
final List<ILoggingEvent> logsList = listAppender.list;
181+
assertEquals("Number of log entries for the ArtifactorySearchResponseHandler", 2, logsList.size());
182+
183+
ILoggingEvent logEvent = logsList.get(0);
184+
assertEquals(Level.WARN, logEvent.getLevel());
185+
assertEquals("No checksums found in artifactory search result of uri {}. Please make sure that header X-Result-Detail is retained on any (reverse)-proxy, loadbalancer or WebApplicationFirewall in the network path to your Artifactory Server", logEvent.getMessage());
186+
Object[] args = logEvent.getArgumentArray();
187+
assertEquals(1, args.length);
188+
assertEquals("https://artifactory.example.com:443/artifactory/api/storage/maven-central-cache/org/freemarker/freemarker/2.3.33/freemarker-2.3.33.jar", args[0]);
189+
190+
logEvent = logsList.get(1);
191+
assertEquals(Level.WARN, logEvent.getLevel());
192+
assertEquals("No checksums found in artifactory search result of uri {}. Please make sure that header X-Result-Detail is retained on any (reverse)-proxy, loadbalancer or WebApplicationFirewall in the network path to your Artifactory Server", logEvent.getMessage());
193+
args = logEvent.getArgumentArray();
194+
assertEquals(1, args.length);
195+
assertEquals("https://artifactory.example.com:443/artifactory/api/storage/gradle-plugins-extended-cache/org/freemarker/freemarker/2.3.33/freemarker-2.3.33.jar", args[0]);
196+
197+
// Remove our manually injected additional appender
198+
sutLogger.detachAppender(listAppender);
199+
listAppender.stop();
200+
}
201+
}
202+
138203
@Test
139204
public void shouldHandleNoMatches() throws IOException {
140205
// Given
@@ -210,6 +275,19 @@ private byte[] multipleMatchesPayload() {
210275
" } ]}").getBytes(StandardCharsets.UTF_8);
211276
}
212277

278+
private byte[] noXResultDetailHeaderResponse() {
279+
return ("{\n" +
280+
" \"results\": [\n" +
281+
" {\n" +
282+
" \"uri\": \"https://artifactory.example.com:443/artifactory/api/storage/maven-central-cache/org/freemarker/freemarker/2.3.33/freemarker-2.3.33.jar\"\n" +
283+
" },\n" +
284+
" {\n" +
285+
" \"uri\": \"https://artifactory.example.com:443/artifactory/api/storage/gradle-plugins-extended-cache/org/freemarker/freemarker/2.3.33/freemarker-2.3.33.jar\"\n" +
286+
" }\n" +
287+
" ]\n" +
288+
"}").getBytes(StandardCharsets.UTF_8);
289+
}
290+
213291
@Test
214292
public void shouldProcessCorrectlyArtifactoryAnswer() throws IOException {
215293
// Given

0 commit comments

Comments
 (0)