Skip to content

build: break dependency so tests can be more reliable #8212

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

Closed
nhumblot wants to merge 1 commit into from
Closed

build: break dependency so tests can be more reliable #8212

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
4 changes: 2 additions & 2 deletions core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
*/
private void loadSuppressionData(Engine engine) throws SuppressionParseException {
final List<SuppressionRule> ruleList = new ArrayList<>();
final SuppressionParser parser = new SuppressionParser();
final SuppressionParser parser = SuppressionParser.newInstance();
final String[] suppressionFilePaths = getSettings().getArray(Settings.KEYS.SUPPRESSION_FILE);
final List<String> failedLoadingFiles = new ArrayList<>();
if (suppressionFilePaths != null && suppressionFilePaths.length > 0) {
Expand Down Expand Up @@ -188,7 +188,7 @@ private void loadSuppressionData(Engine engine) throws SuppressionParseException
* @throws SuppressionParseException thrown if the XML cannot be parsed.
*/
private void loadSuppressionBaseData(final Engine engine) throws SuppressionParseException {
final SuppressionParser parser = new SuppressionParser();
final SuppressionParser parser = SuppressionParser.newInstance();
loadPackagedSuppressionBaseData(parser, engine);
loadHostedSuppressionBaseData(parser, engine);
}
Expand Down
21 changes: 20 additions & 1 deletion core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,26 @@ public class SuppressionHandler extends DefaultHandler {

private Boolean groupBase = null;
private Calendar groupUntil = null;
private final Calendar now;

/**
* Constructs a new suppression handler.
*
* @deprecated use {@link SuppressionHandler#SuppressionHandler(Calendar)}
*/
@Deprecated(forRemoval = true)
public SuppressionHandler() {
this(Calendar.getInstance());
}

/**
* Constructs a new suppression handler.
*
* @param now represents current time. Used to compare suppression expiration dates set within the 'until' attribute.
*/
SuppressionHandler(Calendar now) {
Copy link
Collaborator

@chadlwilson chadlwilson Jan 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use something more modern like an Instant or ZonedDateTime (if TZ dependent) in API or does that require too many other changes/introduce inconsistency with existing public APIs already taking Calendars (🤮)?

Copy link
Collaborator Author

@nhumblot nhumblot Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java.util.Calendar seems to be exposed in our API in a limited amount of places, in the core, through org.owasp.dependencycheck.xml.suppression.SuppressionRule and org.owasp.dependencycheck.utils.DateUtil.

Removing these public elements will technically require a breaking change of the core at some point but we could start by providing alternative APIs and deprecating the old ones for a removal in the next major.

Copy link
Collaborator

@chadlwilson chadlwilson Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I am not sure how "seriously" we treat Java level visibility on these core APIs, especially given java's limited ability to really enforce visibility (short of the module system).

Do you know how many projects there are which depend on the core which are not official or "close" plugins (e.g jenkins, azure etc). I wonder if it is sufficient that we check we're not breaking the major ones when making changes to the core.

Copy link
Collaborator

@chadlwilson chadlwilson Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some adjacent discussion at dependency-check/dependency-check-gradle#491 (comment) where I express my preference that things like the Settings API are not directly exposed to end users via things like the Gradle plugin - but where your opinion would probably be valuable.

this.now = now;
}

/**
* Get the value of suppressionRules.
Expand Down Expand Up @@ -199,7 +218,7 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
if (null != qName) {
switch (qName) {
case SUPPRESS:
if (rule.getUntil() != null && rule.getUntil().before(Calendar.getInstance())) {
if (rule.getUntil() != null && rule.getUntil().before(now)) {
LOGGER.info("Suppression is expired for rule: {}", rule);
} else {
suppressionRules.add(rule);
Expand Down
28 changes: 27 additions & 1 deletion core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.io.InputStreamReader;
import java.io.Reader;
import java.nio.charset.StandardCharsets;
import java.util.Calendar;
import java.util.List;
import javax.annotation.concurrent.ThreadSafe;
import javax.xml.parsers.ParserConfigurationException;
Expand Down Expand Up @@ -76,6 +77,31 @@ public class SuppressionParser {
*/
private static final String SUPPRESSION_SCHEMA_1_0 = "schema/suppression.xsd";

private final Calendar now;

public static SuppressionParser newInstance() {
return new SuppressionParser(Calendar.getInstance());
}

/**
* Constructs a new SuppressionParser.
*
* @deprecated use {@link SuppressionParser#newInstance()}
*/
@Deprecated(forRemoval = true)
public SuppressionParser() {
this(Calendar.getInstance());
}

/**
* Constructs a new SuppressionParser.
*
* @param now represents current time. Used to compare suppression expiration dates set within the 'until' attribute.
*/
SuppressionParser(Calendar now) {
this.now = now;
}

/**
* Parses the given XML file and returns a list of the suppression rules
* contained.
Expand Down Expand Up @@ -117,7 +143,7 @@ public List<SuppressionRule> parseSuppressionRules(InputStream inputStream)
final String defaultEncoding = StandardCharsets.UTF_8.name();
final String charsetName = bom == null ? defaultEncoding : bom.getCharsetName();

final SuppressionHandler handler = new SuppressionHandler();
final SuppressionHandler handler = new SuppressionHandler(now);
final SAXParser saxParser = XmlUtils.buildSecureSaxParser(schemaStream14, schemaStream13, schemaStream12, schemaStream11, schemaStream10);
final XMLReader xmlReader = saxParser.getXMLReader();
xmlReader.setErrorHandler(new SuppressionErrorHandler());
Expand Down
3 changes: 2 additions & 1 deletion core/src/test/java/org/owasp/dependencycheck/xml/suppression/SuppressionHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.io.InputStreamReader;
import java.io.Reader;
import java.nio.charset.StandardCharsets;
import java.util.Calendar;
import java.util.List;

import static org.junit.jupiter.api.Assertions.assertTrue;
Expand All @@ -50,7 +51,7 @@ void testHandler() throws Exception {
File file = BaseTest.getResourceAsFile(this, "suppressions.xml");
InputStream schemaStream = BaseTest.getResourceAsStream(this, "schema/suppression.xsd");

SuppressionHandler handler = new SuppressionHandler();
SuppressionHandler handler = new SuppressionHandler(Calendar.getInstance());
SAXParser saxParser = XmlUtils.buildSecureSaxParser(schemaStream);
XMLReader xmlReader = saxParser.getXMLReader();
xmlReader.setErrorHandler(new SuppressionErrorHandler());
Expand Down
34 changes: 26 additions & 8 deletions core/src/test/java/org/owasp/dependencycheck/xml/suppression/SuppressionParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.time.Instant;
import java.time.LocalDate;
import java.time.ZoneOffset;
import java.util.Calendar;
import java.util.List;
import java.util.stream.Collectors;

Expand All @@ -36,6 +37,23 @@
*/
class SuppressionParserTest extends BaseTest {


private static final Calendar NOW_AS_OF_2013_11_13 = new Calendar.Builder()
.setDate(2013, 10, 13)
.build();

private static final Calendar NOW_AS_OF_2018_04_08 = new Calendar.Builder()
.setDate(2018, 3, 8)
.build();

private static final Calendar NOW_AS_OF_2019_07_21 = new Calendar.Builder()
.setDate(2019, 6, 21)
.build();

private static final Calendar NOW_AS_OF_2025_11_13 = new Calendar.Builder()
.setDate(2025, 10, 13)
.build();

/**
* Test of parseSuppressionRules method, of class SuppressionParser for the
* v1.0 suppression XML Schema.
Expand All @@ -44,7 +62,7 @@ class SuppressionParserTest extends BaseTest {
void testParseSuppressionRulesV1dot0() throws Exception {
//File file = new File(this.getClass().getClassLoader().getResource("suppressions.xml").getPath());
File file = BaseTest.getResourceAsFile(this, "suppressions.xml");
SuppressionParser instance = new SuppressionParser();
SuppressionParser instance = new SuppressionParser(NOW_AS_OF_2013_11_13);
List<SuppressionRule> result = instance.parseSuppressionRules(file);
assertEquals(5, result.size());
}
Expand All @@ -57,7 +75,7 @@ void testParseSuppressionRulesV1dot0() throws Exception {
void testParseSuppressionRulesV1dot1() throws Exception {
//File file = new File(this.getClass().getClassLoader().getResource("suppressions.xml").getPath());
File file = BaseTest.getResourceAsFile(this, "suppressions_1_1.xml");
SuppressionParser instance = new SuppressionParser();
SuppressionParser instance = new SuppressionParser(NOW_AS_OF_2018_04_08);
List<SuppressionRule> result = instance.parseSuppressionRules(file);
assertEquals(5, result.size());
}
Expand All @@ -70,7 +88,7 @@ void testParseSuppressionRulesV1dot1() throws Exception {
void testParseSuppressionRulesV1dot2() throws Exception {
//File file = new File(this.getClass().getClassLoader().getResource("suppressions.xml").getPath());
File file = BaseTest.getResourceAsFile(this, "suppressions_1_2.xml");
SuppressionParser instance = new SuppressionParser();
SuppressionParser instance = new SuppressionParser(NOW_AS_OF_2018_04_08);
List<SuppressionRule> result = instance.parseSuppressionRules(file);
assertEquals(4, result.size());
}
Expand All @@ -83,7 +101,7 @@ void testParseSuppressionRulesV1dot2() throws Exception {
void testParseSuppressionRulesV1dot3() throws Exception {
//File file = new File(this.getClass().getClassLoader().getResource("suppressions.xml").getPath());
File file = BaseTest.getResourceAsFile(this, "suppressions_1_3.xml");
SuppressionParser instance = new SuppressionParser();
SuppressionParser instance = new SuppressionParser(NOW_AS_OF_2019_07_21);
List<SuppressionRule> result = instance.parseSuppressionRules(file);
assertEquals(4, result.size());
}
Expand All @@ -95,7 +113,7 @@ void testParseSuppressionRulesV1dot3() throws Exception {
@Test
void testParseSuppressionRulesV1dot4() throws SuppressionParseException {
File file = BaseTest.getResourceAsFile(this, "suppressions_1_4.xml");
SuppressionParser instance = new SuppressionParser();
SuppressionParser instance = new SuppressionParser(NOW_AS_OF_2025_11_13);
List<SuppressionRule> suppressionRules = instance.parseSuppressionRules(file);

assertEquals(7, suppressionRules.size());
Expand All @@ -108,7 +126,7 @@ void testParseSuppressionRulesV1dot4() throws SuppressionParseException {
void testParseSuppressionRulesV1dot4BackwardsCompability() throws SuppressionParseException {
// 'suppressions_1_4_no_groups.xml' has the same content as 'suppressions_1_3.xml'. But follows schema 1.4
File file = BaseTest.getResourceAsFile(this, "suppressions_1_4_no_groups.xml");
SuppressionParser instance = new SuppressionParser();
SuppressionParser instance = new SuppressionParser(NOW_AS_OF_2025_11_13);
List<SuppressionRule> suppressionRules = instance.parseSuppressionRules(file);

assertEquals(4, suppressionRules.size());
Expand All @@ -121,7 +139,7 @@ void testParseSuppressionRulesV1dot4BackwardsCompability() throws SuppressionPar
@Test
void testParseSuppressionV1dot4Inherits() throws SuppressionParseException {
File file = BaseTest.getResourceAsFile(this, "suppressions_1_4.xml");
SuppressionParser instance = new SuppressionParser();
SuppressionParser instance = new SuppressionParser(NOW_AS_OF_2025_11_13);
List<SuppressionRule> suppressionRules = instance.parseSuppressionRules(file);

// CVE-2013-1338 in test xml has no attributes and should inherit the ones set on group level.
Expand All @@ -144,7 +162,7 @@ void testParseSuppressionV1dot4Inherits() throws SuppressionParseException {
@Test
void testParseSuppressionV1dot4AttributeOverrides() throws SuppressionParseException {
File file = BaseTest.getResourceAsFile(this, "suppressions_1_4.xml");
SuppressionParser instance = new SuppressionParser();
SuppressionParser instance = new SuppressionParser(NOW_AS_OF_2025_11_13);
List<SuppressionRule> suppressionRules = instance.parseSuppressionRules(file);

// CVE-2013-1339 in test xml has attribute {code (until="2027-01-01Z")} set and is present in suppressionGroup.
Expand Down