From eecac2550f691910910ea3feee287b78a97631b8 Mon Sep 17 00:00:00 2001 From: David Thompson Date: Mon, 3 Nov 2025 16:45:00 -0500 Subject: [PATCH] Do not create document links with zero length range Ultimately, a zero length document link doesn't make sense. VS Code throws an error when attempting to consume a zero length document link. To test this change, try the reproducer example that I added in the linked issue. Fixes redhat-developer/vscode-xml#1110 Signed-off-by: David Thompson --- .../ContentModelDocumentLinkParticipant.java | 11 ++-- .../org/eclipse/lemminx/utils/DOMUtils.java | 15 ++++++ .../lemminx/utils/XMLPositionUtility.java | 9 +++- .../catalog/XMLCatalogDocumentLinkTest.java | 46 +++++++++++++++++ .../contentmodel/DTDDocumentLinkTest.java | 20 ++++++++ .../XMLModelDocumentLinkTest.java | 30 +++++++++++ .../extensions/dtd/DTDDocumentLinkTest.java | 6 +++ .../rng/RNGDocumentLinkingExtensionsTest.java | 51 +++++++++++++++++++ .../xinclude/XIncludeDocumentLinkTest.java | 11 ++++ 9 files changed, 192 insertions(+), 7 deletions(-) diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/ContentModelDocumentLinkParticipant.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/ContentModelDocumentLinkParticipant.java index 444a4522a..0fc5d950c 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/ContentModelDocumentLinkParticipant.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/ContentModelDocumentLinkParticipant.java @@ -28,6 +28,7 @@ import org.eclipse.lemminx.dom.XMLModel; import org.eclipse.lemminx.services.extensions.IDocumentLinkParticipant; import org.eclipse.lemminx.uriresolver.URIResolverExtensionManager; +import org.eclipse.lemminx.utils.DOMUtils; import org.eclipse.lsp4j.DocumentLink; import org.w3c.dom.NamedNodeMap; @@ -64,7 +65,7 @@ public void findDocumentLinks(DOMDocument document, List links) { if (location != null) { try { DOMRange systemIdRange = docType.getSystemIdNode(); - if (systemIdRange != null) { + if (DOMUtils.isNonEmptyRange(systemIdRange, true)) { links.add(createDocumentLink(systemIdRange, location, true)); } } catch (BadLocationException e) { @@ -81,7 +82,7 @@ public void findDocumentLinks(DOMDocument document, List links) { if (location != null) { try { DOMRange systemIdRange = entity.getSystemIdNode(); - if (systemIdRange != null) { + if (DOMUtils.isNonEmptyRange(systemIdRange, true)) { links.add(createDocumentLink(systemIdRange, location, true)); } } catch (BadLocationException e) { @@ -98,7 +99,7 @@ public void findDocumentLinks(DOMDocument document, List links) { if (location != null) { try { DOMRange hrefRange = xmlModel.getHrefNode(); - if (hrefRange != null) { + if (DOMUtils.isNonEmptyRange(hrefRange, true)) { links.add(createDocumentLink(hrefRange, location, true)); } } catch (BadLocationException e) { @@ -115,7 +116,7 @@ public void findDocumentLinks(DOMDocument document, List links) { noNamespaceSchemaLocation.getLocation()); if (location != null) { DOMRange attrValue = noNamespaceSchemaLocation.getAttr().getNodeAttrValue(); - if (attrValue != null) { + if (DOMUtils.isNonEmptyRange(attrValue, true)) { links.add(createDocumentLink(attrValue, location, true)); } } @@ -132,7 +133,7 @@ public void findDocumentLinks(DOMDocument document, List links) { String location; for (SchemaLocationHint schemaLocationHint : schemaLocationHints) { location = resolverManager.resolve(document.getDocumentURI(), null, schemaLocationHint.getHint()); - if (location != null) { + if (DOMUtils.isNonEmptyRange(schemaLocationHint, false)) { links.add(createDocumentLink(schemaLocationHint, location, false)); } } diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/DOMUtils.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/DOMUtils.java index cd9120fe3..80df91a8f 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/DOMUtils.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/DOMUtils.java @@ -21,6 +21,7 @@ import org.eclipse.lemminx.dom.DOMElement; import org.eclipse.lemminx.dom.DOMNode; import org.eclipse.lemminx.dom.DOMParser; +import org.eclipse.lemminx.dom.DOMRange; import org.eclipse.lemminx.uriresolver.URIResolverExtensionManager; import org.xml.sax.InputSource; import org.xml.sax.SAXNotRecognizedException; @@ -287,4 +288,18 @@ public static InputSource createInputSource(DOMDocument document) { inputSource.setSystemId(uri); return inputSource; } + + /** + * Returns false if the range is zero-length, and true otherwise. + * + * @param range the range to check + * @param adjust true if the leading and trailing quotes should be removed before checking if it's zero-length, false otherwise + * @return false if the range is zero-length, and true otherwise + */ + public static boolean isNonEmptyRange(DOMRange range, boolean adjust) { + if (range == null) { + return false; + } + return range.getEnd() - range.getStart() > (adjust ? 2 : 0); + } } diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java index 2d9164db7..39e315a04 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java @@ -1040,8 +1040,13 @@ public static Location createLocation(DOMRange target) { public static DocumentLink createDocumentLink(DOMRange target, String location, boolean adjust) throws BadLocationException { DOMDocument document = target.getOwnerDocument(); - Position start = document.positionAt(target.getStart() + (adjust ? 1 : 0)); - Position end = document.positionAt(target.getEnd() - (adjust ? 1 : 0)); + int startOffset = target.getStart() + (adjust ? 1 : 0); + int endOffset = target.getEnd() - (adjust ? 1 : 0); + if (startOffset == endOffset) { + throw new IllegalArgumentException("empty range"); + } + Position start = document.positionAt(startOffset); + Position end = document.positionAt(endOffset); return new DocumentLink(new Range(start, end), location); } diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/catalog/XMLCatalogDocumentLinkTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/catalog/XMLCatalogDocumentLinkTest.java index 114518f97..44dc0a9d9 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/catalog/XMLCatalogDocumentLinkTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/catalog/XMLCatalogDocumentLinkTest.java @@ -69,6 +69,20 @@ public void testCatalogWithCatalogDOCTYPE() { } + @Test + public void testCatalogWithCatalogDOCTYPEZeroLength() { + String xml = "\n" + + // + "\n" + // + " \n" + // + ""; + testDocumentLinkFor(xml, CATALOG_PATH, + dl(r(0, 64, 0, 75), "src/test/resources/catalog.dtd")); + + } + @Test public void testSystemSuffixEntryDocumentLink() { String xml = "\n" + // @@ -78,6 +92,14 @@ public void testSystemSuffixEntryDocumentLink() { dl(r(1, 45, 1, 57), "src/test/resources/mySchema.xsd")); } + @Test + public void testSystemSuffixEntryZeroLengthDocumentLink() { + String xml = "\n" + // + " \n" + // + ""; + testDocumentLinkFor(xml, CATALOG_PATH); + } + @Test public void testURISuffixEntryDocumentLink() { String xml = "\n" + // @@ -87,6 +109,14 @@ public void testURISuffixEntryDocumentLink() { dl(r(1, 42, 1, 54), "src/test/resources/mySchema.xsd")); } + @Test + public void testURISuffixEntryZeroLengthDocumentLink() { + String xml = "\n" + // + " \n" + // + ""; + testDocumentLinkFor(xml, CATALOG_PATH); + } + @Test public void testMustBeCatalog1() { String xml = ""; @@ -127,6 +157,14 @@ public void testDelegatePublicEntry() { dl(r(1, 27, 1, 54), "src/test/resources/catalogs/catalog-public.xml")); } + @Test + public void testDelegatePublicEntryZeroLength() { + String xml = "\n" + // + " \n" + // + ""; + testDocumentLinkFor(xml, CATALOG_PATH); + } + @Test public void testDelegateSystemEntry() { String xml = "\n" + // @@ -136,6 +174,14 @@ public void testDelegateSystemEntry() { dl(r(1, 27, 1, 54), "src/test/resources/catalogs/catalog-public.xml")); } + @Test + public void testDelegateSystemEntryZeroLength() { + String xml = "\n" + // + " \n" + // + ""; + testDocumentLinkFor(xml, CATALOG_PATH); + } + @Test public void testDelegateUriEntry() { String xml = "\n" + // diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java index 38318e38f..a861012d5 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java @@ -36,6 +36,16 @@ public void docTypeSYSTEM() throws BadLocationException { dl(r(1, 31, 1, 55), "src/test/resources/dtd/entities/base.dtd")); } + @Test + public void docTypeSYSTEMZeroLengh() throws BadLocationException { + String xml = "\r\n" + // + "\r\n" + // + ""; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.xml"); + } + @Test public void docTypePUBLIC() throws BadLocationException { String xml = "\r\n" + // @@ -47,6 +57,16 @@ public void docTypePUBLIC() throws BadLocationException { dl(r(1, 38, 1, 62), "src/test/resources/dtd/entities/base.dtd")); } + @Test + public void docTypePUBLICZeroLength() throws BadLocationException { + String xml = "\r\n" + // + "\r\n" + // + ""; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.xml"); + } + @Test public void noLinks() throws BadLocationException { String xml = "\r\n" + // diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLModelDocumentLinkTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLModelDocumentLinkTest.java index bb626858c..6bef42638 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLModelDocumentLinkTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLModelDocumentLinkTest.java @@ -35,4 +35,34 @@ public void xmlModelHref() throws BadLocationException { XMLAssert.testDocumentLinkFor(xml, "src/test/resources/Format.xml", dl(r(1, 18, 1, 32), "src/test/resources/xsd/Format.xsd")); } + + @Test + public void xmlModelHrefZeroLength() throws BadLocationException { + String xml = "\r\n" + // + "\r\n" + // + "\r\n" + // + " \r\n" + // + " "; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/Format.xml"); + } + + @Test + public void xmlModelHrefZeroLength2() throws BadLocationException { + String xml = "\r\n" + // + "\r\n" + // + "\r\n" + // + " \r\n" + // + " "; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/Format.xml"); + } + + @Test + public void xmlModelHrefZeroLength3() throws BadLocationException { + String xml = "\r\n" + // + "\r\n" + // + "\r\n" + // + " \r\n" + // + " "; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/Format.xml"); + } } diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/dtd/DTDDocumentLinkTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/dtd/DTDDocumentLinkTest.java index 135172094..18c09477a 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/dtd/DTDDocumentLinkTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/dtd/DTDDocumentLinkTest.java @@ -31,4 +31,10 @@ public void entity() throws BadLocationException { XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.dtd", dl(r(0, 28, 0, 43), "src/test/resources/document.ent")); } + + @Test + public void entityZeroLength() throws BadLocationException { + String xml = ""; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.dtd"); + } } diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/relaxng/grammar/rng/RNGDocumentLinkingExtensionsTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/relaxng/grammar/rng/RNGDocumentLinkingExtensionsTest.java index 5a1a41816..223f4a47a 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/relaxng/grammar/rng/RNGDocumentLinkingExtensionsTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/relaxng/grammar/rng/RNGDocumentLinkingExtensionsTest.java @@ -103,4 +103,55 @@ public void externalRef() throws BadLocationException { XMLAssert.testDocumentLinkFor(xml, "src/test/resources/relaxng/main.rng", dl(r(11, 24, 11, 34), "src/test/resources/relaxng/inline.rng")); } + + @Test + public void emptyIncludeHref() throws BadLocationException { + String xml = "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/relaxng/main.rng"); + } + + @Test + public void emptyIncludeHref2() throws BadLocationException { + String xml = "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/relaxng/main.rng"); + } + + @Test + public void emptyIncludeHref3() throws BadLocationException { + String xml = "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/relaxng/main.rng"); + } } diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/xinclude/XIncludeDocumentLinkTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/xinclude/XIncludeDocumentLinkTest.java index a6b175433..387eb9273 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/xinclude/XIncludeDocumentLinkTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/xinclude/XIncludeDocumentLinkTest.java @@ -45,4 +45,15 @@ public void includeHrefInAnyLevel() throws BadLocationException { XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xinclude/main.xml", dl(r(3, 22, 3, 35), "src/test/resources/xinclude/reference.xml")); } + + @Test + public void includeHrefZeroLength() throws BadLocationException { + String xml = "\n" + // + " \n" + // + " \n" + // + " \n" + // <-- documentLink + " \n" + // + "\n"; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xinclude/main.xml"); + } }