Skip to content

OAK-12364: XPath queries fail to parse if they contain invalid XML characters #3084

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
ChlineSaurus wants to merge 2 commits into
base: trunk
Choose a base branch
from
+90 −1
Open

OAK-12364: XPath queries fail to parse if they contain invalid XML characters #3084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
976189a
OAK-12364: XPath queries fail to parse when path names contain invali…
Aug 17, 2026
281c35f
OAK-12364: use \u notation for the middle dot in tests for readability
Aug 20, 2026
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
8 changes: 8 additions & 0 deletions oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,10 @@ public Oak with(@NotNull Whiteboard whiteboard) {
LOG.info("Registered ignore limit in index selection feature: " + QueryEngineSettings.FT_IGNORE_LIMIT_IN_INDEX_SELECTION);
closer.register(ignoreLimitInIndexSelection);
queryEngineSettings.setIgnoreLimitInIndexSelectionFeature(ignoreLimitInIndexSelection);
Feature xmlNameCharsInPath = newFeature(QueryEngineSettings.FT_XML_NAME_CHARS_IN_PATH, whiteboard);
LOG.info("Registered XML name characters in path feature: " + QueryEngineSettings.FT_XML_NAME_CHARS_IN_PATH);
closer.register(xmlNameCharsInPath);
queryEngineSettings.setXmlNameCharsInPathFeature(xmlNameCharsInPath);
}

return this;
Expand Down Expand Up @@ -1009,6 +1013,10 @@ public void setIgnoreLimitInIndexSelectionFeature(@Nullable Feature feature) {
settings.setIgnoreLimitInIndexSelectionFeature(feature);
}

public void setXmlNameCharsInPathFeature(@Nullable Feature feature) {
settings.setXmlNameCharsInPathFeature(feature);
}

@Override
public void setQueryValidatorPattern(String key, String pattern, String comment, boolean failQuery) {
settings.getQueryValidator().setPattern(key, pattern, comment, failQuery);
Expand Down
12 changes: 12 additions & 0 deletions oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ public class QueryEngineSettings implements QueryEngineSettingsMBean, QueryLimit

public static final String FT_IGNORE_LIMIT_IN_INDEX_SELECTION = "FT_OAK-12057";

public static final String FT_XML_NAME_CHARS_IN_PATH = "FT_OAK-12364";

public static final int DEFAULT_PREFETCH_COUNT = Integer.getInteger(OAK_QUERY_PREFETCH_COUNT, -1);

public static final String OAK_QUERY_FAIL_TRAVERSAL = "oak.queryFailTraversal";
Expand Down Expand Up @@ -125,6 +127,7 @@ public class QueryEngineSettings implements QueryEngineSettingsMBean, QueryLimit
private Feature sortUnionQueryLegacyModeFeature;
private Feature optimizeXPathUnion;
private Feature ignoreLimitInIndexSelectionFeature;
private Feature xmlNameCharsInPathFeature;

private String autoOptionsMappingJson = "{}";
private QueryOptions.AutomaticQueryOptionsMapping autoOptionsMapping = new QueryOptions.AutomaticQueryOptionsMapping(autoOptionsMappingJson);
Expand Down Expand Up @@ -257,6 +260,15 @@ public boolean isIgnoreLimitInIndexSelection() {
return ignoreLimitInIndexSelectionFeature == null || ignoreLimitInIndexSelectionFeature.isEnabled();
}

public void setXmlNameCharsInPathFeature(@Nullable Feature feature) {
this.xmlNameCharsInPathFeature = feature;
}

public boolean isXmlNameCharsInPathEnabled() {
// enabled if the feature toggle is not used
return xmlNameCharsInPathFeature == null || xmlNameCharsInPathFeature.isEnabled();
}

public String getStrictPathRestriction() {
return strictPathRestriction.name();
}
Expand Down
6 changes: 5 additions & 1 deletion oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/XPathToSQL2Converter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@
import org.apache.jackrabbit.oak.query.QueryOptions.Traversal;
import org.apache.jackrabbit.oak.query.xpath.Statement.UnionStatement;
import org.apache.jackrabbit.util.ISO9075;
import org.apache.jackrabbit.util.XMLChar;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* This class can can convert a XPATH query to a SQL2 query.
* This class can convert a XPATH query to a SQL2 query.
*/
public class XPathToSQL2Converter {

Expand Down Expand Up @@ -1006,6 +1007,9 @@ private void initialize(String query) throws ParseException {
} else {
if (Character.isJavaIdentifierPart(c)) {
type = CHAR_NAME;
} else if ((settings == null || settings.isXmlNameCharsInPathEnabled()) && XMLChar.isName(c)) {

@bhabegger bhabegger Aug 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't we not enable this if settings is null (to keep the existing behavior in that case) ?

@ChlineSaurus ChlineSaurus Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was not sure about this. I also had in mind that we decided that we want to keep the old behavior as the default behavior for Oak changes, but in the Agents.md we write that in case something is a bug fix, default should be on, and only new features should be default off.
Further, it's not a very risky change, therefore I went with default enabled, but if we want to be more defensive I can change it. 😌

@reschke reschke Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a case where the new behavior would break something that previously "worked" (that is, didn't throw an exception)?

@ChlineSaurus ChlineSaurus Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is one case I found, but I'm not sure how realistic it is. The query so far interpreted XML characters as whitespaces. If this is in the middle, it failed, as described in the Issue. However, if the XML character is directly next to a separator, it is ignored, meaning the new behavior can change queries behavior for such queries instead of failing (See test below for an example.) Not sure the feature toggle is too defensive, but better safe than sorry.

@Test
    public void testXmlNameCharsInPathFeatureChangesBehavior() throws ParseException {
        // a middle dot (\u00b7) attached to a name is dropped when the feature is disabled,
        // but kept as part of the name when enabled, resulting in a different query
        String xpath = "//*[@a\u00b7 = 1]";

        QueryEngineSettings disabled = new QueryEngineSettings();
        disabled.setXmlNameCharsInPathFeature(createFeature(false));
        String withoutFeature = new XPathToSQL2Converter(disabled).convert(xpath);

        QueryEngineSettings enabled = new QueryEngineSettings();
        enabled.setXmlNameCharsInPathFeature(createFeature(true));
        String withFeature = new XPathToSQL2Converter(enabled).convert(xpath);

        assertTrue(withoutFeature.contains("[a] = 1"));
        assertTrue(withFeature.contains("[a\u00b7] = 1"));
        assertNotEquals(withoutFeature, withFeature);
    }

// accept XML name characters that ISO9075 leaves unencoded, so they are not split off the name
type = CHAR_NAME;
}
}
}
Expand Down
14 changes: 14 additions & 0 deletions oak-core/src/test/java/org/apache/jackrabbit/oak/query/XPathTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,20 @@ public void union() throws ParseException {
"/* xpath: /jcr:root/content// element(*, nt:folder) order by @jcr:score descending */");
}

@Test
public void xmlNameCharsInPathAreConverted() throws ParseException {
verify("/jcr:root/content/m\u00b7d/element(*, nt:base)",
"select [jcr:path], [jcr:score], * " +
"from [nt:base] as a " +
"where ischildnode(a, '/content/m\u00b7d') " +
"/* xpath: /jcr:root/content/m\u00b7d/element(*, nt:base) */");
verify("/jcr:root/content/m\u00b7d",
"select [jcr:path], [jcr:score], * " +
"from [nt:base] as a " +
"where issamenode(a, '/content/m\u00b7d') " +
"/* xpath: /jcr:root/content/m\u00b7d */");
}

private void verify(String xpath, String expectedSql2) throws ParseException {
String sql2 = new XPathToSQL2Converter().convert(xpath);
sql2 = formatSQL(sql2);
Expand Down
9 changes: 9 additions & 0 deletions oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/TraversingIndexQueryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,15 @@ public void testMultipleRelativeProperties() throws Exception {
assertQuery("//*[*/@d < 3]", "xpath", Arrays.asList("/content"));
}

@Test
public void testXmlNameCharsInPathAreQueryable() throws Exception {
root.getTree("/").addChild("content").addChild("m\u00b7d").addChild("child");
root.commit();

assertQuery("/jcr:root/content/m\u00b7d/element(*, nt:base)", "xpath",
List.of("/content/m\u00b7d/child"));
}

@Test
public void testLowercaseOnArrays() throws Exception {
// OAK-1829
Expand Down
42 changes: 42 additions & 0 deletions oak-core/src/test/java/org/apache/jackrabbit/oak/query/xpath/XPathToSQL2Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package org.apache.jackrabbit.oak.query.xpath;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.text.ParseException;

Expand Down Expand Up @@ -142,6 +144,46 @@ public void testOrWithFunctionCallFeatureEnabled() throws ParseException {
"select ... where contains(*, 'test') or [type] = 'page' ");
}

@Test
public void testXmlNameCharsInPathFeatureNotSet() throws ParseException {
String sql2 = new XPathToSQL2Converter(new QueryEngineSettings())
.convert("/jcr:root/a/m\u00b7d/element(*, nt:base)");
assertTrue(sql2.contains("'/a/m\u00b7d'"));
}

@Test
public void testXmlNameCharsInPathFeatureEnabled() throws ParseException {
QueryEngineSettings settings = new QueryEngineSettings();
settings.setXmlNameCharsInPathFeature(createFeature(true));
String sql2 = new XPathToSQL2Converter(settings)
.convert("/jcr:root/a/m\u00b7d/element(*, nt:base)");
assertTrue(sql2.contains("'/a/m\u00b7d'"));
}

@Test
public void testXmlNameCharsInPathFeatureDisabled() {
QueryEngineSettings settings = new QueryEngineSettings();
settings.setXmlNameCharsInPathFeature(createFeature(false));
try {
new XPathToSQL2Converter(settings)
.convert("/jcr:root/a/m\u00b7d/element(*, nt:base)");
fail("expected ParseException");
} catch (ParseException expected) {
// expected
}
}

@Test
public void testGenuineSpaceInPathStillFails() {
try {
new XPathToSQL2Converter(new QueryEngineSettings())
.convert("/jcr:root/a/m d/element(*, nt:base)");
fail("expected ParseException");
} catch (ParseException expected) {
// expected
}
}

/**
* Helper method to create a Feature mock with the specified enabled state.
*/
Expand Down