diff --git a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/CommandChain.java b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/CommandChain.java index 9ae936c10..dc03a16ed 100644 --- a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/CommandChain.java +++ b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/CommandChain.java @@ -16,18 +16,7 @@ class CommandChain { private static final Logger LOGGER = LoggerFactory.getLogger(CommandChain.class); /** Internal ArrayList of the Commands */ - private List commandList; - - private static List COMMON_COMMANDS = - List.of( - // Validation commands - CheckDependencyPresent.getInstance(), - CheckParentPackaging.getInstance(), - // Format commands - new FormatCommand(), - new DiscardFormatCommand(), - // Multipom command - new CompositeDependencyManagement()); + private final List commandList; private CommandChain(List commands) { this.commandList = commands; @@ -93,7 +82,17 @@ public boolean execute(ProjectModel c) * @return A pre-configured Chain for modifying a POM. */ public static CommandChain modifyDependency() { - final List modifyCommands = new ArrayList<>(COMMON_COMMANDS); + final List modifyCommands = + new ArrayList<>( + List.of( + // Validation commands + CheckDependencyPresent.getInstance(), + CheckParentPackaging.getInstance(), + // Format commands + new FormatCommand(), + new DiscardFormatCommand(), + // Multipom command + new CompositeDependencyManagement())); modifyCommands.addAll( List.of( SimpleUpgrade.getInstance(), @@ -109,7 +108,17 @@ public static CommandChain modifyDependency() { * @return A pre-configured Chain. */ public static CommandChain insertDependency() { - final List insertCommands = new ArrayList<>(COMMON_COMMANDS); + final List insertCommands = + new ArrayList<>( + List.of( + // Validation commands + CheckDependencyPresent.getInstance(), + CheckParentPackaging.getInstance(), + // Format commands + new FormatCommand(), + new DiscardFormatCommand(), + // Multipom command + new CompositeDependencyManagement())); insertCommands.add(new SimpleInsert(true)); return new CommandChain(insertCommands); } @@ -120,7 +129,17 @@ public static CommandChain insertDependency() { * @return A pre-configured Chain. */ public static CommandChain updateDependency() { - final List insertCommands = new ArrayList<>(COMMON_COMMANDS); + final List insertCommands = + new ArrayList<>( + List.of( + // Validation commands + CheckDependencyPresent.getInstance(), + CheckParentPackaging.getInstance(), + // Format commands + new FormatCommand(), + new DiscardFormatCommand(), + // Multipom command + new CompositeDependencyManagement())); insertCommands.addAll( List.of(SimpleUpgrade.getInstance(), SimpleDependencyManagement.getInstance())); @@ -169,7 +188,8 @@ public static CommandChain createForDependencyQuery(QueryType queryType) { return filterByQueryType( AVAILABLE_DEPENDENCY_QUERY_COMMANDS, queryType, - Arrays.asList(CheckLocalRepositoryDirCommand.CheckParentDirCommand.getInstance()), + Collections.singletonList( + CheckLocalRepositoryDirCommand.CheckParentDirCommand.getInstance()), it -> it == queryType); } @@ -196,7 +216,7 @@ public static CommandChain createForVersionQuery(QueryType queryType) { * and report issues creating */ static final List> AVAILABLE_DEPENDENCY_QUERY_COMMANDS = - new ArrayList<>(Arrays.asList(new Pair<>(QueryType.SAFE, "QueryByParsing"))); + new ArrayList<>(List.of(new Pair<>(QueryType.SAFE, "QueryByParsing"))); /** List of Commands for Version Query */ private static final List> AVAILABLE_QUERY_VERSION_COMMANDS = diff --git a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java index 511ceebc6..4ada1c5a7 100644 --- a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java +++ b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java @@ -9,6 +9,7 @@ import java.net.URISyntaxException; import java.nio.charset.Charset; import java.util.*; +import java.util.function.UnaryOperator; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -137,22 +138,22 @@ private BitSet elementBitSet(byte[] doc) throws XMLStreamException { * A Slight variation on writeAsUnicode from stax which writes as a regex string so we could * rewrite its output */ - private String writeAsRegex(StartElement element) { + private String writeAsRegex(final StartElement element, final List orderedAttributes) { StringWriter writer = new StringWriter(); writer.write("<"); writer.write(Pattern.quote(element.getName().getLocalPart())); - Iterator attrIter = element.getAttributes(); - while (attrIter.hasNext()) { - Attribute attr = (Attribute) attrIter.next(); - + for (var attr : orderedAttributes) { writer.write("\\s+"); writer.write(Pattern.quote(attr.getName().getLocalPart())); - writer.write("=[\\\"\']"); + writer.write("\\s*"); + writer.write("="); + writer.write("\\s*"); + writer.write("[\\\"']"); writer.write(Pattern.quote(attr.getValue())); - writer.write("[\\\"\']"); + writer.write("[\\\"']"); } writer.write("\\s*\\/>"); @@ -335,7 +336,25 @@ private void parseXmlAndCharset(POMDocument pomFile) throws XMLStreamException, new IntRange( realElementStart, realElementStart + 1 + trimmedOriginalContent.length()); - String contentRe = writeAsRegex(getLastStartElement(prevEvents)); + var element = getLastStartElement(prevEvents); + + // order the attributes by the original ordering + // attributes names are unique, we can just order them by the index of the name + + // Remove attribute contents, just in case some they contain the name of an attribute + // TODO should we trim the element name beforehand? + String contentRemoved = untrimmedOriginalContent.replaceAll("[\\\"'].*[\\\"']", ""); + + var it = element.getAttributes(); + var orderedAttributes = + Stream.iterate(it, Iterator::hasNext, UnaryOperator.identity()) + .map(Iterator::next) + .map(a -> new Pair<>(a, contentRemoved.indexOf(a.getName().getLocalPart()))) + .sorted(Comparator.comparing(p -> p.getSecond())) + .map(p -> p.getFirst()) + .collect(Collectors.toList()); + + String contentRe = writeAsRegex(element, orderedAttributes); Regex modifiedContentRE = new Regex(contentRe); @@ -494,6 +513,8 @@ private byte[] serializePomFile(POMDocument pom) throws XMLStreamException { // Let's find out the original empty elements from the original pom and store into a stack List elementsToReplace = getElementsToReplace(originalElementMap, pom); + // DOM parsers don't guarantee attribute ordering, extract the original ordering for the regex + // Lets to the replacements backwards on the existing, current pom Map emptyElements = getEmptyElements(targetElementMap, xmlRepresentation); diff --git a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/AbstractTestBase.java b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/AbstractTestBase.java index da091ba81..17558ca09 100644 --- a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/AbstractTestBase.java +++ b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/AbstractTestBase.java @@ -59,7 +59,7 @@ protected ProjectModel performAndAssertPomOperation( String testName, ProjectModel context, final OperationType operationType) throws Exception { String resultFile = "pom-" + testName + "-result.xml"; - URL resource = AbstractTestBase.class.getClass().getResource(resultFile); + URL resource = AbstractTestBase.class.getResource(resultFile); if (resource != null) { Document outcome = new SAXReader().read(resource); diff --git a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java index a49b90793..4c8888ed8 100644 --- a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java +++ b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java @@ -581,6 +581,19 @@ void modify_adds_dependency_to_file_with_tabs() "\n\t\t\n\t\t\torg.dom4j\n\t\t\tdom4j\n\t\t\n"); } + @Test + void modify_adds_dependency_to_pom_with_empty_elements_with_multiple_attributes() + throws Exception { + Dependency dependencyToUpgrade = + new Dependency("io.github.pixee", "java-security-toolkit", "1.0.2", null, null, null); + + performAndAssertModifyPomOperation( + "trimmed-roller", + ProjectModelFactory.load(POMOperatorTest.class.getResource("pom-trimmed-roller.xml")) + .withDependency(dependencyToUpgrade) + .withUseProperties(true)); + } + /** * Tests a scenario with an empty element from a customer's POM file and validates the resultant * POM. diff --git a/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller-result.xml b/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller-result.xml new file mode 100644 index 000000000..9f346c3b7 --- /dev/null +++ b/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller-result.xml @@ -0,0 +1,113 @@ + + + + + + + org.instancio + instancio-junit + 5.0.1 + test + + + io.github.pixee + java-security-toolkit + + + + + + roller + + + + + maven-antrun-plugin + 3.1.0 + + + ant-contrib + ant-contrib + ${maven-antrun.version} + + + ant + ant + + + + + + org.apache.velocity + velocity + 1.7 + + + + + + gen-db-scripts + generate-resources + + run + + + + + + + + + + + + + + + + + + + + + + + + + + + io.github.pixee + java-security-toolkit + ${versions.java-security-toolkit} + + + + + 1.0.2 + + diff --git a/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller.xml b/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller.xml new file mode 100644 index 000000000..faa5fc696 --- /dev/null +++ b/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller.xml @@ -0,0 +1,98 @@ + + + + + + + org.instancio + instancio-junit + 5.0.1 + test + + + + + + roller + + + + + maven-antrun-plugin + 3.1.0 + + + ant-contrib + ant-contrib + ${maven-antrun.version} + + + ant + ant + + + + + + org.apache.velocity + velocity + 1.7 + + + + + + gen-db-scripts + generate-resources + + run + + + + + + + + + + + + + + + + + + + + + + + + +