Skip to content

feat: Improve Artifactory handler log message #7838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 30, 2025
Merged

feat: Improve Artifactory handler log message #7838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -108,9 +115,13 @@ public List<MavenArtifact> 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) {
Expand Down
31 changes: 22 additions & 9 deletions ...ain/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,16 @@
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;
import java.util.Optional;
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<List<MavenArtifact>> {
/**
* Pattern to match the path returned by the Artifactory AQL API.
Expand All @@ -61,13 +64,20 @@ class ArtifactorySearchResponseHandler implements HttpClientResponseHandler<List
private final Dependency expectedDependency;

/**
* Creates a responsehandler for the response on a single dependency-search attempt.
* The search request URL i.e., the location at which to search artifacts with checksum information
*/
private final URL sourceUrl;

/**
* Creates a response handler for the response on a single dependency-search attempt.
*
* @param sourceUrl The search request URL
* @param dependency The dependency that is expected to be in the response when found (for validating the FileItem(s) in the response)
*/
ArtifactorySearchResponseHandler(Dependency dependency) {
ArtifactorySearchResponseHandler(URL sourceUrl, Dependency dependency) {
this.fileImplReader = new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false).readerFor(FileImpl.class);
this.expectedDependency = dependency;
this.sourceUrl = sourceUrl;
}

protected boolean init(JsonParser parser) throws IOException {
Expand Down Expand Up @@ -114,7 +124,7 @@ private boolean checkHashes(ChecksumsImpl checksums) {
match = false;
}
final String sha256sum = expectedDependency.getSha256sum();
/* For sha256 we need to validate that the checksum is non-null in the artifactory response.
/* For SHA-256, we need to validate that the checksum is non-null in the artifactory response.
* Extract from Artifactory documentation:
* New artifacts that are uploaded to Artifactory 5.5 and later will automatically have their SHA-256 checksum calculated.
* However, artifacts that were already hosted in Artifactory before the upgrade will not have their SHA-256 checksum in the database yet.
Expand All @@ -132,7 +142,7 @@ private boolean checkHashes(ChecksumsImpl checksums) {
* Process the Artifactory response.
*
* @param response the HTTP response
* @return a list of the Maven Artifact informations that match the searched dependency hash
* @return a list of the Maven Artifact information that matches the searched dependency hash
* @throws FileNotFoundException When a matching artifact is not found
* @throws IOException thrown if there is an I/O error
*/
Expand All @@ -149,8 +159,11 @@ public List<MavenArtifact> 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;
}

Expand All @@ -174,18 +187,18 @@ public List<MavenArtifact> 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;
}

/**
* Validate the FileImpl result for usability as a dependency.
* <br/>
* 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<Matcher> validateUsability(FileImpl file) {
Expand Down
59 changes: 37 additions & 22 deletions ...java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearchResponseHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 {
Expand Down Expand Up @@ -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<MavenArtifact> mavenArtifacts = handler.handleResponse(response);

// Then
Expand Down Expand Up @@ -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<MavenArtifact> mavenArtifacts = handler.handleResponse(response);

// Then
Expand Down Expand Up @@ -152,6 +166,7 @@ void shouldProcessCorrectlyForMissingXResultDetailHeader() throws IOException {
final ListAppender<ILoggingEvent> 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();
Expand All @@ -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<ILoggingEvent> 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);
Expand All @@ -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
Expand Down Expand Up @@ -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<MavenArtifact> mavenArtifacts = handler.handleResponse(response);

// Then
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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<MavenArtifact> result = handler.handleResponse(response);
// Then
assertEquals(1, result.size());
Expand Down