From 7f92ad943072ec6111432b6c5e327fa74e05023a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcel=20St=C3=B6r?= Date: Tue, 29 Jul 2025 22:43:02 +0200 Subject: [PATCH] Improve Artifactory handler log message Fixes #7837 --- .../data/artifactory/ArtifactorySearch.java | 17 +++++- .../ArtifactorySearchResponseHandler.java | 31 +++++++--- .../ArtifactorySearchResponseHandlerTest.java | 59 ++++++++++++------- 3 files changed, 73 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearch.java b/core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearch.java index 8674103c388..630e8af0d0b 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearch.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearch.java @@ -52,6 +52,13 @@ @ThreadSafe public class ArtifactorySearch { + /** + * Required to add all extra information of the found artifact. + * Source: https://jfrog.com/help/r/jfrog-rest-apis/property-search + */ + @SuppressWarnings("JavadocLinkAsPlainText") + public static final String X_RESULT_DETAIL_HEADER = "X-Result-Detail"; + /** * Used for logging. */ @@ -108,9 +115,13 @@ public List search(Dependency dependency) throws IOException { final StringBuilder msg = new StringBuilder("Could not connect to Artifactory at") .append(url); try { - final BasicHeader artifactoryResultDetail = new BasicHeader("X-Result-Detail", "info"); - return Downloader.getInstance().fetchAndHandle(url, new ArtifactorySearchResponseHandler(dependency), List.of(artifactoryResultDetail), - allowUsingProxy); + final BasicHeader artifactoryResultDetail = new BasicHeader(X_RESULT_DETAIL_HEADER, "info"); + return Downloader.getInstance().fetchAndHandle( + url, + new ArtifactorySearchResponseHandler(url, dependency), + List.of(artifactoryResultDetail), + allowUsingProxy + ); } catch (TooManyRequestsException e) { throw new IOException(msg.append(" (429): Too manu requests").toString(), e); } catch (URISyntaxException e) { diff --git a/core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandler.java b/core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandler.java index adaf1bc1ecc..711bfc657e0 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandler.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandler.java @@ -32,6 +32,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStreamReader; +import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; @@ -39,6 +40,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import static org.owasp.dependencycheck.data.artifactory.ArtifactorySearch.X_RESULT_DETAIL_HEADER; + class ArtifactorySearchResponseHandler implements HttpClientResponseHandler> { /** * Pattern to match the path returned by the Artifactory AQL API. @@ -61,13 +64,20 @@ class ArtifactorySearchResponseHandler implements HttpClientResponseHandler handleResponse(ClassicHttpResponse response) throws I final FileImpl file = fileImplReader.readValue(parser); if (file.getChecksums() == null) { - 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", - file.getUri()); + LOGGER.warn( + "No checksums found in Artifactory search result for '{}'. " + + "Specifically, the result set contains URI '{}' but it is missing the 'checksums' property. " + + "Please make sure that the '{}' header is retained on any (reverse-)proxy, load-balancer or Web Application Firewall in the network path to your Artifactory server.", + sourceUrl, file.getUri(), X_RESULT_DETAIL_HEADER); continue; } @@ -174,7 +187,7 @@ public List handleResponse(ClassicHttpResponse response) throws I } if (result.isEmpty()) { throw new FileNotFoundException("Artifact " + expectedDependency - + " not found in Artifactory; discovered sha1 hits not recognized as matching maven artifacts"); + + " not found in Artifactory; discovered SHA1 hits not recognized as matching Maven artifacts"); } return result; } @@ -182,10 +195,10 @@ public List handleResponse(ClassicHttpResponse response) throws I /** * Validate the FileImpl result for usability as a dependency. *
- * Checks that the actually matches all known hashes and the path appears to match a maven repository G/A/V pattern. + * Checks that the file actually matches all known hashes and the path appears to match a maven repository G/A/V pattern. * * @param file The FileImpl from an Artifactory search response - * @return An Optional with the Matcher for the file path to retrieve the Maven G/A/V coordinates in case result is usable for further + * @return An Optional with the Matcher for the file path to retrieve the Maven G/A/V coordinates in case the result is usable for further * processing, otherwise an empty Optional. */ private Optional validateUsability(FileImpl file) { diff --git a/core/src/test/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandlerTest.java b/core/src/test/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandlerTest.java index 00dada71ec8..861aaf1f9cb 100644 --- a/core/src/test/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandlerTest.java +++ b/core/src/test/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandlerTest.java @@ -33,6 +33,8 @@ import java.io.ByteArrayInputStream; import java.io.FileNotFoundException; import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.List; @@ -41,8 +43,20 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +@SuppressWarnings("resource") class ArtifactorySearchResponseHandlerTest extends BaseTest { + private static final URL TEST_URL; + private static final String EXCEPTION_MESSAGE = "Artifact Dependency{ fileName='null', actualFilePath='null', filePath='null', packagePath='null'} not found in Artifactory; discovered SHA1 hits not recognized as matching Maven artifacts"; + + static { + try { + TEST_URL = new URL("https://example.com/artifactory/api/search/checksum?sha1=43515aa4b2f4bce7c431145e8c0a7bcc441e0532"); + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } + } + @BeforeEach @Override public void setUp() throws Exception { @@ -88,7 +102,7 @@ void shouldProcessCorrectlyArtifactoryAnswerWithoutSha256() throws IOException { // When - final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency); + final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(TEST_URL, dependency); final List mavenArtifacts = handler.handleResponse(response); // Then @@ -118,7 +132,7 @@ void shouldProcessCorrectlyArtifactoryAnswerWithMultipleMatches() throws IOExcep // When - final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency); + final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(TEST_URL, dependency); final List mavenArtifacts = handler.handleResponse(response); // Then @@ -152,6 +166,7 @@ void shouldProcessCorrectlyForMissingXResultDetailHeader() throws IOException { final ListAppender listAppender = new ListAppender<>(); listAppender.start(); sutLogger.addAppender(listAppender); + final String logMessage = "No checksums found in Artifactory search result for '{}'. Specifically, the result set contains URI '{}' but it is missing the 'checksums' property. Please make sure that the '{}' header is retained on any (reverse-)proxy, load-balancer or Web Application Firewall in the network path to your Artifactory server."; // Given final Dependency dependency = new Dependency(); @@ -167,32 +182,32 @@ void shouldProcessCorrectlyForMissingXResultDetailHeader() throws IOException { // When - final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency); + final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(TEST_URL, dependency); FileNotFoundException e = assertThrows(FileNotFoundException.class, () -> handler.handleResponse(response), "Result with no details due to missing X-Result-Detail header, should throw an exception!"); // Then - 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", + 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", e.getMessage()); - // There should be a WARN-log for for each of the results regarding the absence of X-Result-Detail header driven attributes + // There should be a WARN-log for each of the results regarding the absence of X-Result-Detail header driven attributes final List logsList = listAppender.list; assertEquals(2, logsList.size(), "Number of log entries for the ArtifactorySearchResponseHandler"); ILoggingEvent logEvent = logsList.get(0); assertEquals(Level.WARN, logEvent.getLevel()); - 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()); + assertEquals(logMessage, logEvent.getMessage()); Object[] args = logEvent.getArgumentArray(); - assertEquals(1, args.length); - 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]); + assertEquals(3, args.length); + assertEquals("https://artifactory.example.com:443/artifactory/api/storage/maven-central-cache/org/freemarker/freemarker/2.3.33/freemarker-2.3.33.jar", args[1]); logEvent = logsList.get(1); assertEquals(Level.WARN, logEvent.getLevel()); - 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()); + assertEquals(logMessage, logEvent.getMessage()); args = logEvent.getArgumentArray(); - assertEquals(1, args.length); - 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]); + assertEquals(3, args.length); + 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[1]); // Remove our manually injected additional appender sutLogger.detachAppender(listAppender); @@ -212,7 +227,7 @@ void shouldHandleNoMatches() throws IOException { when(responseEntity.getContent()).thenReturn(new ByteArrayInputStream(payload)); // When - final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency); + final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(TEST_URL, dependency); FileNotFoundException e = assertThrows(FileNotFoundException.class, () -> handler.handleResponse(response), "No Match found, should throw an exception!"); // Then @@ -298,7 +313,7 @@ void shouldProcessCorrectlyArtifactoryAnswer() throws IOException { when(responseEntity.getContent()).thenReturn(new ByteArrayInputStream(payload)); // When - final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency); + final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(TEST_URL, dependency); final List mavenArtifacts = handler.handleResponse(response); // Then @@ -419,13 +434,13 @@ void shouldProcessCorrectlyArtifactoryAnswerMisMatchMd5() throws IOException { when(responseEntity.getContent()).thenReturn(new ByteArrayInputStream(payload)); // When - final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency); + final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(TEST_URL, dependency); FileNotFoundException e = assertThrows(FileNotFoundException.class, () -> handler.handleResponse(response), "MD5 mismatching should throw an exception!"); // Then assertEquals("Artifact " + dependency - + " not found in Artifactory; discovered sha1 hits not recognized as matching maven artifacts", e.getMessage()); + + " not found in Artifactory; discovered SHA1 hits not recognized as matching Maven artifacts", e.getMessage()); } @Test @@ -442,12 +457,12 @@ void shouldProcessCorrectlyArtifactoryAnswerMisMatchSha1() throws IOException { when(responseEntity.getContent()).thenReturn(new ByteArrayInputStream(payload)); // When - final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency); + final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(TEST_URL, dependency); FileNotFoundException e = assertThrows(FileNotFoundException.class, () -> handler.handleResponse(response), "SHA1 mismatching should throw an exception!"); // Then - assertEquals("Artifact Dependency{ fileName='null', actualFilePath='null', filePath='null', packagePath='null'} not found in Artifactory; discovered sha1 hits not recognized as matching maven artifacts", e.getMessage()); + assertEquals(EXCEPTION_MESSAGE, e.getMessage()); } @Test @@ -464,12 +479,12 @@ void shouldProcessCorrectlyArtifactoryAnswerMisMatchSha256() throws IOException when(responseEntity.getContent()).thenReturn(new ByteArrayInputStream(payload)); // When - final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency); + final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(TEST_URL, dependency); FileNotFoundException e = assertThrows(FileNotFoundException.class, () -> handler.handleResponse(response), "SHA256 mismatching should throw an exception!"); // Then - assertEquals("Artifact Dependency{ fileName='null', actualFilePath='null', filePath='null', packagePath='null'} not found in Artifactory; discovered sha1 hits not recognized as matching maven artifacts", e.getMessage()); + assertEquals(EXCEPTION_MESSAGE, e.getMessage()); } @Test @@ -487,12 +502,12 @@ void shouldThrowNotFoundWhenPatternCannotBeParsed() throws IOException { when(responseEntity.getContent()).thenReturn(new ByteArrayInputStream(payload)); // When - final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency); + final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(TEST_URL, dependency); FileNotFoundException e = assertThrows(FileNotFoundException.class, () -> handler.handleResponse(response), "Maven GAV pattern mismatch for filepath should throw a not found exception!"); // Then - assertEquals("Artifact Dependency{ fileName='null', actualFilePath='null', filePath='null', packagePath='null'} not found in Artifactory; discovered sha1 hits not recognized as matching maven artifacts", e.getMessage()); + assertEquals(EXCEPTION_MESSAGE, e.getMessage()); } @Test @@ -509,7 +524,7 @@ void shouldSkipResultsWherePatternCannotBeParsed() throws IOException { when(responseEntity.getContent()).thenReturn(new ByteArrayInputStream(payload)); // When - final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(dependency); + final ArtifactorySearchResponseHandler handler = new ArtifactorySearchResponseHandler(TEST_URL, dependency); List result = handler.handleResponse(response); // Then assertEquals(1, result.size());