Bug #2536: Failure to check that node returned by extension function has wrong NamePool - Saxon - Saxonica Developer Community

Actions

Send by e-mail Copy link

Bug #2536

closed

Failure to check that node returned by extension function has wrong NamePool

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

Status:

Closed

Priority:

Normal

Assignee:

Michael Kay

Category:

Saxon extensions

Sprint/Milestone:

9.7.0.2

Start date:

2015-12-10

Due date:

% Done:

100%

Estimated time:

Legacy ID:

Applies to branch:

9.5, 9.6, 9.7

Fix Committed on Branch:

9.6, 9.7

Fixed in Maintenance Release:

9.7.0.2, 9.6.0.9

Platforms:

Description

Saxon is intended to check that when a node is returned by a Java reflexive extension function, the node uses the same NamePool as was used to compile the calling query or stylesheet. If it uses a different namepool, the mapping of integer codes to QNames will be different so names may not match when they should, or vice versa.

This check is not always happening. Saxon decides to perform the check if the static result type permits nodes to be returned. In the case in question, the declared return type of the Java method is "Object"; from this Saxon determines a static type for the function call of JavaExternalObjectType, and this is considered to exclude nodes.

In fact, it is the type inference that is wrong. The declared type of the Java method is Object, but the actual return value is a SequenceIterator. Saxon assumes that when the declared result type is Object, the converted result will be a wrapped External Object, but this is not the case if the actual value is a SequenceIterator. (Surprisingly, it seems that if the dynamic result is something like NodeInfo, then it will still be wrapped as an external object: but I haven't tested that.)

The problem was reported against 9.5 but is still present in 9.7 (though I expect the detailed code paths may have changed).

Actions

Copy link

Updated by O'Neil Delpratt almost 9 years ago

Applies to branch 9.5, 9.6, 9.7 added

Actions

Copy link

Updated by Michael Kay almost 9 years ago

Fix Committed on Branch 9.7 added

For 9.7 I have fixed the problem as follows:

To JPConverter we add a new converter for use when the static type of the method result is "Object". This does the conversion by allocating a new converter at run-time based on the type of the actual instance returned. The static inferred type for a method with return type Object becomes item()*. The effect is that when such a method returns a recognized object such as Sequence, SequenceIterator, NodeInfo, StringValue, etc, then it behaves in the same way as if that were the declared type.

As a result of this change, the static type becomes "item()" and therefore the check for a compatible NamePool in any returned nodes is performed.

This change isn't 100% backwards compatible. For example, if a method declared to return Object returns a NodeInfo, then in 9.6 the value returned by the function call would be a NodeInfo wrapped as an external Java object; it will now be a node.

Actions

Copy link

Updated by Michael Kay almost 9 years ago

Fix Committed on Branch 9.6 added

For 9.6 I have commited a more cautious fix: for a method with declared return type "Object", we still allocate a WrapExternalObject converter to handle the result, but we now infer the static type item() rather than javaExternalObject for the result of this converter. This is slightly devious, because the converter in fact always returns a wrapped external object, but it recognizes the fact that when the method actually returns a SequenceIterator, we don't in fact invoke the converter but return the result unconverted, and in this case the result can be of any item type. Changing the static type inference is enough to ensure that the configuration check is carried out on any returned nodes.

Actions

Copy link

Updated by Michael Kay almost 9 years ago

Status changed from New to Resolved

Actions

Copy link

Updated by John Lumley almost 9 years ago

In 9.7 under certain circumstances JPConverter.FromObject.convert() can loop, as the allocate() logic chooses Object over more specific classes. This is random as it depends upon the entry order in a HashMap as to whether Object is found first.

The FromObject reference is removed from the conversion map and a specific test made for Object within allocate() after the conversion map has returned null.

Patch applied to the 9.7 branch. The 9.6 logic is different (Object doesn't appear in the conversion map) and shouldn't show this problem.

Actions

Copy link