From 764adcf0d0fd921b19230b4cf82393756785f7c8 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Mon, 5 Aug 2024 10:04:53 +0530 Subject: [PATCH 1/6] Adding Sast scan issue recommended fixes --- .../tools/langserver/lemminx/util/DocumentUtil.java | 6 ++++++ .../io/openliberty/tools/langserver/utils/XmlReader.java | 9 ++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java index 695d4877..3a0dd593 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java @@ -26,6 +26,7 @@ import org.w3c.dom.Node; import org.w3c.dom.NodeList; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.transform.OutputKeys; @@ -41,8 +42,13 @@ public class DocumentUtil { public static Document getDocument(File inputFile) throws Exception { try { DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); + docFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING,true); docFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); docFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + docFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl",true); + docFactory.setFeature("http://xml.org/sax/features/external-parameter-entities",false); + docFactory.setFeature("http://xml.org/sax/features/external-general-entities",false); + docFactory.setXIncludeAware(false); docFactory.setNamespaceAware(true); docFactory.setExpandEntityReferences(false); DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java index fc36eb26..9b09cf95 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java @@ -21,6 +21,7 @@ import java.util.Set; import java.util.logging.Logger; +import javax.xml.XMLConstants; import javax.xml.stream.XMLEventReader; import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; @@ -55,9 +56,11 @@ public static Map getElementValues(File file, Set elemen XMLInputFactory factory = XMLInputFactory.newInstance(); try { - factory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE); - factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); - factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE); + factory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE); + factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES,Boolean.FALSE); + } catch (Exception e) { LOGGER.warning("Could not set properties on XMLInputFactory."); } From b4e339558ab3115c919cba9290d5b7271bd64314 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Mon, 5 Aug 2024 10:27:30 +0530 Subject: [PATCH 2/6] Adding Sast scan issue recommended fixes Signed-off-by: Arun Venmany --- .../openliberty/tools/langserver/lemminx/util/DocumentUtil.java | 1 + 1 file changed, 1 insertion(+) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java index 3a0dd593..77dd0157 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java @@ -48,6 +48,7 @@ public static Document getDocument(File inputFile) throws Exception { docFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl",true); docFactory.setFeature("http://xml.org/sax/features/external-parameter-entities",false); docFactory.setFeature("http://xml.org/sax/features/external-general-entities",false); + docFactory.setXIncludeAware(false); docFactory.setNamespaceAware(true); docFactory.setExpandEntityReferences(false); From 031dc0b3f31baa67f96271ed160cf7d5bc95793b Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Mon, 5 Aug 2024 18:23:19 +0530 Subject: [PATCH 3/6] Incorparating Review Comments Signed-off-by: Arun Venmany --- .../tools/langserver/lemminx/util/DocumentUtil.java | 11 +++++------ .../openliberty/tools/langserver/utils/XmlReader.java | 10 +++++----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java index 77dd0157..0b0bf242 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java @@ -1,5 +1,5 @@ /******************************************************************************* -* Copyright (c) 2023 IBM Corporation and others. +* Copyright (c) 2024 IBM Corporation and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0 which is available at @@ -42,13 +42,12 @@ public class DocumentUtil { public static Document getDocument(File inputFile) throws Exception { try { DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); - docFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING,true); docFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); docFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - docFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl",true); - docFactory.setFeature("http://xml.org/sax/features/external-parameter-entities",false); - docFactory.setFeature("http://xml.org/sax/features/external-general-entities",false); - + docFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + docFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + docFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); + docFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); docFactory.setXIncludeAware(false); docFactory.setNamespaceAware(true); docFactory.setExpandEntityReferences(false); diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java index 9b09cf95..c4e84366 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java @@ -1,5 +1,5 @@ /******************************************************************************* -* Copyright (c) 2023 IBM Corporation and others. +* Copyright (c) 2024 IBM Corporation and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0 which is available at @@ -56,10 +56,10 @@ public static Map getElementValues(File file, Set elemen XMLInputFactory factory = XMLInputFactory.newInstance(); try { - factory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE); - factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); - factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE); - factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES,Boolean.FALSE); + factory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE); + factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, Boolean.FALSE); } catch (Exception e) { LOGGER.warning("Could not set properties on XMLInputFactory."); From ef304c844fa5ee6db8fc9fde78666a98f0830d03 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Mon, 5 Aug 2024 18:28:28 +0530 Subject: [PATCH 4/6] Incorparating Review Comments Signed-off-by: Arun Venmany --- .../openliberty/tools/langserver/lemminx/util/DocumentUtil.java | 2 +- .../java/io/openliberty/tools/langserver/utils/XmlReader.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java index 0b0bf242..7cc46054 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java @@ -1,5 +1,5 @@ /******************************************************************************* -* Copyright (c) 2024 IBM Corporation and others. +* Copyright (c) 2023, 2024 IBM Corporation and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0 which is available at diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java index c4e84366..acf08a61 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java @@ -1,5 +1,5 @@ /******************************************************************************* -* Copyright (c) 2024 IBM Corporation and others. +* Copyright (c) 2023, 2024 IBM Corporation and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0 which is available at From d6cb74a7fa8f9c04fd4742909dff0ea5aceffe13 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Tue, 6 Aug 2024 12:36:49 +0530 Subject: [PATCH 5/6] adding more sast checks and fixes Signed-off-by: Arun Venmany --- .../langserver/lemminx/util/DocumentUtil.java | 38 ++++++++++++------- .../langserver/lemminx/util/XmlReader.java | 23 ++++++----- .../tools/langserver/utils/XmlReader.java | 25 +++++++----- 3 files changed, 54 insertions(+), 32 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java index 7cc46054..7a63335e 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/DocumentUtil.java @@ -29,8 +29,10 @@ import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerConfigurationException; import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; @@ -41,16 +43,7 @@ public class DocumentUtil { public static Document getDocument(File inputFile) throws Exception { try { - DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); - docFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); - docFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - docFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - docFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - docFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); - docFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - docFactory.setXIncludeAware(false); - docFactory.setNamespaceAware(true); - docFactory.setExpandEntityReferences(false); + DocumentBuilderFactory docFactory = getDocumentBuilderFactory(); DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); return docBuilder.parse(inputFile); } catch (Exception e) { @@ -59,11 +52,25 @@ public static Document getDocument(File inputFile) throws Exception { } } + private static DocumentBuilderFactory getDocumentBuilderFactory() throws ParserConfigurationException { + DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); + docFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); + docFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + docFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + docFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + docFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); + docFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + docFactory.setXIncludeAware(false); + docFactory.setNamespaceAware(true); + docFactory.setExpandEntityReferences(false); + return docFactory; + } + public static void writeDocToXmlFile(Document doc, File inputFile) throws Exception { - TransformerFactory transformerFactory = TransformerFactory.newInstance(); + TransformerFactory transformerFactory = getTransformerFactory(); // Need to use this xsl to prevent extra lines in the updated xsd file. It is a known issue in Java 9 and up that is not going to be fixed. // It was a design decision. Ref link: https://bugs.openjdk.org/browse/JDK-8262285?attachmentViewMode=list - InputStream is = DocumentUtil.class.getClassLoader().getResourceAsStream("formatxsd.xsl"); + InputStream is = DocumentUtil.class.getClassLoader().getResourceAsStream("formatxsd.xsl"); Transformer transformer = transformerFactory.newTransformer(new StreamSource(is)); transformer.setOutputProperty(OutputKeys.METHOD, "xml"); transformer.setOutputProperty(OutputKeys.ENCODING, "UTF-8"); @@ -71,13 +78,18 @@ public static void writeDocToXmlFile(Document doc, File inputFile) throws Except transformer.setOutputProperty(OutputKeys.STANDALONE, "yes"); transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "4"); transformer.setOutputProperty(OutputKeys.DOCTYPE_PUBLIC, "yes"); - doc.setXmlStandalone(true); DOMSource source = new DOMSource(doc); StreamResult file = new StreamResult(new OutputStreamWriter(new FileOutputStream(inputFile), "UTF-8")); transformer.transform(source, file); } + private static TransformerFactory getTransformerFactory() throws TransformerConfigurationException { + TransformerFactory transformerFactory = TransformerFactory.newInstance(); + transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, Boolean.TRUE); + return transformerFactory; + } + public static void removeExtraneousAnyAttributeElements(File schemaFile) { try { Document doc = getDocument(schemaFile); diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/XmlReader.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/XmlReader.java index fb5c33ef..7213b4f5 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/XmlReader.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/XmlReader.java @@ -1,5 +1,5 @@ /******************************************************************************* -* Copyright (c) 2023 IBM Corporation and others. +* Copyright (c) 2023, 2024 IBM Corporation and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0 which is available at @@ -56,14 +56,7 @@ public static boolean hasServerRoot(File xmlFile) { } try { - XMLInputFactory factory = XMLInputFactory.newInstance(); - try { - factory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE); - factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); - factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE); - } catch (Exception e) { - LOGGER.warning("Could not set properties on XMLInputFactory."); - } + XMLInputFactory factory = getXmlInputFactory(); XMLEventReader reader = null; @@ -92,6 +85,18 @@ public static boolean hasServerRoot(File xmlFile) { return false; } + private static XMLInputFactory getXmlInputFactory() { + XMLInputFactory factory = XMLInputFactory.newInstance(); + try { + factory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE); + } catch (Exception e) { + LOGGER.warning("Could not set properties on XMLInputFactory."); + } + return factory; + } + public static String getElementValue(Path file, String elementName) { Set names = new HashSet (); names.add(elementName); diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java index acf08a61..d119dad0 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java @@ -54,16 +54,7 @@ public static Map getElementValues(File file, Set elemen return returnValues; } - XMLInputFactory factory = XMLInputFactory.newInstance(); - try { - factory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE); - factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); - factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE); - factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, Boolean.FALSE); - - } catch (Exception e) { - LOGGER.warning("Could not set properties on XMLInputFactory."); - } + XMLInputFactory factory = getXmlInputFactory(); XMLEventReader reader = null; try { @@ -101,6 +92,20 @@ public static Map getElementValues(File file, Set elemen return returnValues; } + private static XMLInputFactory getXmlInputFactory() { + XMLInputFactory factory = XMLInputFactory.newInstance(); + try { + factory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE); + factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, Boolean.FALSE); + + } catch (Exception e) { + LOGGER.warning("Could not set properties on XMLInputFactory."); + } + return factory; + } + protected static String getElementName(XMLEvent event) { return event.asStartElement().getName().getLocalPart(); } From 45d99192f3684ee64e652deca037aefc616b0ce0 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Tue, 6 Aug 2024 18:08:00 +0530 Subject: [PATCH 6/6] adding property to xmlreader of lemminx Signed-off-by: Arun Venmany --- .../io/openliberty/tools/langserver/lemminx/util/XmlReader.java | 1 + 1 file changed, 1 insertion(+) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/XmlReader.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/XmlReader.java index 7213b4f5..c730ab88 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/XmlReader.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/XmlReader.java @@ -91,6 +91,7 @@ private static XMLInputFactory getXmlInputFactory() { factory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE); factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE); + factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, Boolean.FALSE); } catch (Exception e) { LOGGER.warning("Could not set properties on XMLInputFactory."); }