Bug #3121: NullPointerException in XQuery when defaultCollationName is not a known collation - Saxon - Saxonica Developer Community

Actions

Send by e-mail Copy link

Bug #3121

closed

NullPointerException in XQuery when defaultCollationName is not a known collation

Added by Michael Kay almost 8 years ago. Updated over 7 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Michael Kay

Category:

Internals

Sprint/Milestone:

Start date:

2017-01-30

Due date:

% Done:

100%

Estimated time:

Legacy ID:

Applies to branch:

9.7

Fix Committed on Branch:

9.7, trunk

Fixed in Maintenance Release:

9.7.0.15

Platforms:

Description

This test case throws an NPE:

public void testCollationQuery() throws XPathException {
        Configuration config = Configuration.newConfiguration();
        config.setConfigurationProperty(FeatureKeys.GENERATE_BYTE_CODE, false);
        StaticQueryContext sqc = config.newStaticQueryContext();
        sqc.declareDefaultCollation("");
        DynamicQueryContext dqc = new DynamicQueryContext(config);
        dqc.setApplyFunctionConversionRulesToExternalVariables(false);

        String query = "declare base-uri \"/..../xmark/\";\n" +
                "let $auction := fn:doc(\"xmark1.xml\") return\n" +
                "for $i in $auction/site//item\n" +
                "where contains(string(exactly-one($i/description)), \"gold\")\n" +
                "return $i/name/text()";

        XQueryExpression xqExp = sqc.compileQuery(query);
        SequenceIterator itr = xqExp.iterator(dqc);
        Item item = itr.next();
        assertNotNull(item);
        assertNotNull(item.getStringValue());
    }

The code in CollatingFunctionFixed.setRetainedStaticContext ignores any exception from allocateCollator(), claiming "ignore the failure, it will be reported later" - but this is not the case, the function fails with an NPE at run-time if no collator has been allocated.

It's possible that the code was written on the assumption that although the default collation name isn't known at compile time, it might be known at run-time. But I don't think that logic is followed through.

However, setRetainedStaticContext() isn't defined to throw exceptions, so the fix isn't obvious.

Actions

Copy link

Updated by Michael Kay almost 8 years ago

Comments on the method sqc.declareDefaultCollation(); say that it doesn't validate that the supplied collation is a known collection name because the base URI (for resolving a relative collation name) is not known at this stage. Which raises the question whether it really makes sense to allow a relative URI to be supplied here, and what should be the meaning? Do we really allow the same relative default collation URI to resolve differently depending on which query module it is used in? I very much doubt that will work.

The only place that sqc.declareDefaultCollation() is called from internally is XQueryCompiler.declareDefaultCollation(), and the Javadoc for this method says the collation URI must be an absolute URI; it also checks that the collation actually exists. So my inclination is to move the logic in XQueryCompiler that validates the supplied URI down into StaticQueryContext.

Actions

Copy link

Updated by Michael Kay almost 8 years ago

If we stick with the unpatched code for a moment, and change the test to use byte code generation, we get a crash in byte code generation:

java.lang.AssertionError: Generating call on non-existent method expandCollationURI in class net.sf.saxon.functions.CollatingFunctionFixed

at com.saxonica.ee.bytecode.util.Generator.invokeStaticMethod(Generator.java:271)

at com.saxonica.ee.bytecode.ContainsCompiler.useDynamicCollation(ContainsCompiler.java:146)

at com.saxonica.ee.bytecode.ContainsCompiler.compileToBoolean(ContainsCompiler.java:48)

What seems to be happening here is that ContainsCompiler has a path for generating byte code in the case where the collation URI isn't known until run-time. At some stage we stopped invoking bytecode generation for this case because it is rather rare; but the code is still there, and generates calls on static methods that no longer exist. We're getting into this path accidentally when the collation URI (a) is known statically, (b) doesn't exist, and (c) isn't reported as an error at compile time.

I'm getting rid of the dead code on the 9.8 branch but leaving it there (dormant) for 9.7.

Actions

Copy link