Skip to content

XMLSyntaxParser: tolerate a UTF-8 BOM at the start of XML documents #9540

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

Open
NicolaIsotta wants to merge 1 commit into
base: master
Choose a base branch
from
+31 −18
Open

XMLSyntaxParser: tolerate a UTF-8 BOM at the start of XML documents #9540

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
22 changes: 5 additions & 17 deletions ide/xml.xdm/src/org/netbeans/modules/xml/xdm/nodes/XMLSyntaxParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
import org.netbeans.api.xml.lexer.XMLTokenId;

public class XMLSyntaxParser {


private static final String BOM = "\uFEFF";

@mbien mbien Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion: rename to BYTE_ORDER_MARK or add comment to disambiguate from the common XML TLA for bill of materials.


public Document parse(BaseDocument basedoc)
throws IOException, BadLocationException {
try {
Expand All @@ -44,22 +46,8 @@ public Document parse(BaseDocument basedoc)
List<Token> currentTokens = new ArrayList<Token>();
TokenHierarchy th = TokenHierarchy.get(basedoc);
TokenSequence<XMLTokenId> tokenSequence = th.tokenSequence();
org.netbeans.api.lexer.Token<XMLTokenId> token = tokenSequence.token();
// Add the text token, if any, before xml decalration to document node
if(token != null && token.id() == XMLTokenId.TEXT) {
currentTokens.add(Token.create(token.text().toString(),TokenType.TOKEN_CHARACTER_DATA));
if(tokenSequence.moveNext()) {
token = tokenSequence.token();
}
// if the xml decalration is not there assign this token to document
if(token.id() != XMLTokenId.PI_START) {
currentNode.setTokens(new ArrayList<Token>(currentTokens));
currentTokens.clear();
}
}
Comment on lines 48 to -59

@mbien mbien Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure why this was there, but the tests, including your new test case, seem to pass with it.

What was the reason you removed it?


while (tokenSequence.moveNext()) {
token = tokenSequence.token();
org.netbeans.api.lexer.Token<XMLTokenId> token = tokenSequence.token();
XMLTokenId tokenId = token.id();
String image = token.text().toString();
TokenType tokenType = TokenType.TOKEN_WHITESPACE;
Expand Down Expand Up @@ -245,7 +233,7 @@ public Document parse(BaseDocument basedoc)
((Element)parent).appendChild(currentNode, false);
} else {//parent is Document
if(token.id() != XMLTokenId.BLOCK_COMMENT &&
token.text().toString().trim().length() > 0) {
!token.text().toString().isBlank() && !BOM.equals(token.text().toString().trim())) {
Comment on lines -248 to +236

@mbien mbien Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. the field image holds the value for token.text().toString() and could be used here i think
  2. the difference between the whitespace semantics of isBlank() and trim() is significant here, right? I would recommend to add a comment that trim() must be kept, otherwise someone might change it to strip() or wonder why this code looks almost identical to the one of the top level BLOCK_COMMENT case and adjust both.

throw new IOException("Invalid token '" + token.text() +
"' found in document: " +
"Please use the text editor to resolve the issues...");
Expand Down
18 changes: 17 additions & 1 deletion ide/xml.xdm/test/unit/src/org/netbeans/modules/xml/xdm/nodes/XMLSyntaxParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public static Test suite() {
suite.addTest(new XMLSyntaxParserTest("testParseWSDL"));
// Disabled as referenced files were partly not donated by oracle to apache
// suite.addTest(new XMLSyntaxParserTest("testParsePerformace"));
suite.addTest(new XMLSyntaxParserTest("testParseBOM"));
return suite;
}

Expand Down Expand Up @@ -254,5 +255,20 @@ public void testParsePerformace() throws Exception {
//FlushVisitor fv = new FlushVisitor();
//String docBuf = fv.flushModel(doc);
//assertEquals("The document should be unaltered",basedoc.getText(0,basedoc.getLength()),docBuf);
}
}

/**
* Test of parse method, of class org.netbeans.modules.xmltools.xmlmodel.nodes.XMLSyntaxParser.
* XMLSyntaxParser should handle xml files with BOM
*/
public void testParseBOM() throws Exception {
BaseDocument basedoc = getDocument("nodes/testBOM.xml");
XMLSyntaxParser parser = new XMLSyntaxParser();
Document doc = parser.parse(basedoc);
assertNotNull("Document can not be null", doc);
FlushVisitor fv = new FlushVisitor();
String docBuf = fv.flushModel(doc);
assertEquals("The document should be unaltered",basedoc.getText(0,basedoc.getLength()),docBuf);
}

}
9 changes: 9 additions & 0 deletions ide/xml.xdm/test/unit/src/org/netbeans/modules/xml/xdm/nodes/testBOM.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0"?>
<company>
<employee ssn="xx-xx-xxxx" id="123"
address="16 Network Circle"
phone="123-456-7890" >Vidhya Narayanan
</employee>
<!-- -->
<employee/>
</company>
Loading