Copilot commented on code in PR #808: URL: https://github.com/apache/maven-indexer/pull/808#discussion_r4172202488
########## indexer-core/src/main/java/org/apache/maven/index/PomInfo.java: ########## @@ -0,0 +1,201 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.index; + +import javax.xml.XMLConstants; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamConstants; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; + +import java.io.IOException; +import java.io.InputStream; +import java.util.HashMap; +import java.util.Map; + +/** + * The few POM fields the indexer reads: {@code name}, {@code description} and {@code packaging}, taken from the + * top-level elements of a POM without inheritance or interpolation. It is read with the JDK StAX parser and does not + * need the Maven model classes. + * + * @see ArtifactContext#getPomInfo() + * @since 7.2.0 + */ +public final class PomInfo { + + private final String name; + + private final String description; + + private final String packaging; + + public PomInfo(String name, String description, String packaging) { + this.name = name; + this.description = description; + this.packaging = packaging; + } + + /** + * Returns the trimmed {@code <name>} of the POM, or {@code null} if the POM declares none. + */ + public String getName() { + return name; + } + + /** + * Returns the trimmed {@code <description>} of the POM, or {@code null} if the POM declares none. + */ + public String getDescription() { + return description; + } + + /** + * Returns the trimmed {@code <packaging>} of the POM, or {@code jar} if the POM declares none, as the Maven + * model does. + */ + public String getPackaging() { + return packaging; + } + + /** + * Reads the three fields from the given POM stream. The stream is not closed. DTDs and external entities are not + * processed. + * + * @throws IOException if the stream cannot be read or is not well-formed XML + */ + public static PomInfo read(InputStream inputStream) throws IOException { + XMLInputFactory factory = XMLInputFactory.newFactory(); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + // POMs in the wild use HTML entities such as ø without declaring them; the Maven model reader accepts + // the XHTML ones in non-strict mode, so keep entity references as events and resolve them here + factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, false); Review Comment: Disabling entity replacement exposes references in element text, but the JDK parser still rejects undeclared references in attributes. For example, `<arg value="ø"/>` inside plugin configuration makes the whole POM read fail, although the Maven reader accepts it. This loses otherwise valid metadata and makes `ArtifactLocator` return null. Preserve XHTML entity support in attributes as well, without enabling external entity processing, and add an attribute parity test. ########## indexer-core/src/main/java/org/apache/maven/index/PomInfo.java: ########## @@ -0,0 +1,201 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.index; + +import javax.xml.XMLConstants; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamConstants; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; + +import java.io.IOException; +import java.io.InputStream; +import java.util.HashMap; +import java.util.Map; + +/** + * The few POM fields the indexer reads: {@code name}, {@code description} and {@code packaging}, taken from the + * top-level elements of a POM without inheritance or interpolation. It is read with the JDK StAX parser and does not + * need the Maven model classes. + * + * @see ArtifactContext#getPomInfo() + * @since 7.2.0 + */ +public final class PomInfo { + + private final String name; + + private final String description; + + private final String packaging; + + public PomInfo(String name, String description, String packaging) { + this.name = name; + this.description = description; + this.packaging = packaging; + } + + /** + * Returns the trimmed {@code <name>} of the POM, or {@code null} if the POM declares none. + */ + public String getName() { + return name; + } + + /** + * Returns the trimmed {@code <description>} of the POM, or {@code null} if the POM declares none. + */ + public String getDescription() { + return description; + } + + /** + * Returns the trimmed {@code <packaging>} of the POM, or {@code jar} if the POM declares none, as the Maven + * model does. + */ + public String getPackaging() { + return packaging; + } + + /** + * Reads the three fields from the given POM stream. The stream is not closed. DTDs and external entities are not + * processed. + * + * @throws IOException if the stream cannot be read or is not well-formed XML + */ + public static PomInfo read(InputStream inputStream) throws IOException { + XMLInputFactory factory = XMLInputFactory.newFactory(); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + // POMs in the wild use HTML entities such as ø without declaring them; the Maven model reader accepts + // the XHTML ones in non-strict mode, so keep entity references as events and resolve them here + factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, false); + if (factory.isPropertySupported(XMLConstants.ACCESS_EXTERNAL_DTD)) { + factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + } + XMLStreamReader reader = null; + try { + reader = factory.createXMLStreamReader(inputStream); Review Comment: The JDK StAX parser closes the supplied stream when it reaches the end of the document, despite `read()` promising to leave it open. A tracking stream around `<project><name>sample</name></project>` is already closed before `reader.close()` runs. Pass a non-closing wrapper to StAX so callers retain ownership of their stream. ########## indexer-core/src/main/java/org/apache/maven/index/PomInfo.java: ########## @@ -0,0 +1,201 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.index; + +import javax.xml.XMLConstants; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamConstants; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; + +import java.io.IOException; +import java.io.InputStream; +import java.util.HashMap; +import java.util.Map; + +/** + * The few POM fields the indexer reads: {@code name}, {@code description} and {@code packaging}, taken from the + * top-level elements of a POM without inheritance or interpolation. It is read with the JDK StAX parser and does not + * need the Maven model classes. + * + * @see ArtifactContext#getPomInfo() + * @since 7.2.0 + */ +public final class PomInfo { + + private final String name; + + private final String description; + + private final String packaging; + + public PomInfo(String name, String description, String packaging) { + this.name = name; + this.description = description; + this.packaging = packaging; + } + + /** + * Returns the trimmed {@code <name>} of the POM, or {@code null} if the POM declares none. + */ + public String getName() { + return name; + } + + /** + * Returns the trimmed {@code <description>} of the POM, or {@code null} if the POM declares none. + */ + public String getDescription() { + return description; + } + + /** + * Returns the trimmed {@code <packaging>} of the POM, or {@code jar} if the POM declares none, as the Maven + * model does. + */ + public String getPackaging() { + return packaging; + } + + /** + * Reads the three fields from the given POM stream. The stream is not closed. DTDs and external entities are not + * processed. + * + * @throws IOException if the stream cannot be read or is not well-formed XML + */ + public static PomInfo read(InputStream inputStream) throws IOException { + XMLInputFactory factory = XMLInputFactory.newFactory(); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + // POMs in the wild use HTML entities such as ø without declaring them; the Maven model reader accepts + // the XHTML ones in non-strict mode, so keep entity references as events and resolve them here + factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, false); + if (factory.isPropertySupported(XMLConstants.ACCESS_EXTERNAL_DTD)) { + factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + } + XMLStreamReader reader = null; + try { + reader = factory.createXMLStreamReader(inputStream); + String name = null; + String description = null; + String packaging = "jar"; + int depth = 0; + while (reader.hasNext()) { + int event = reader.next(); + if (event == XMLStreamConstants.START_ELEMENT) { + depth++; + if (depth == 2) { + String local = reader.getLocalName(); Review Comment: Matching only the local name lets foreign prefixed elements override POM fields. For example, `<packaging>jar</packaging><ext:packaging xmlns:ext="urn:custom">war</ext:packaging>` changes packaging to `war`, while the non-strict Maven reader skips `ext:packaging`. This can make artifact lookup select the wrong sibling file. Only extract unprefixed fields to preserve the previous tag matching. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
