Skip to content

Conversation

nverwer
Copy link

@nverwer nverwer commented Mar 18, 2025

Description

The fn:transform function sometimes produced NullPointerExceptions. This happened for documents containing nodes of type org.exist.dom.memtree.ElementImpl and other subtypes of org.exist.dom.memtree.NodeImpl.

The function org.exist.xquery.functions.fn.transform.Convert.ToSaxon.ofNode(Node) uses org.exist.xquery.functions.fn.transform.TreeUtils.treeIndex(Node) to get a 'path' (as a list of indexes) of the Node in its containing document. This 'path' is used to find the node in a newly built document that is suitable for use by Saxon.
Using the 'path' that the treeIndex function produces assumes that it starts at the document node.

However, some nodes do not have a containing document. Specifically, in org.exist.dom.memtree.NodeImpl.getParentNode():

if (parent.getNodeType() == DOCUMENT_NODE && !((DocumentImpl) parent).isExplicitlyCreated()) {
            /*
                All nodes in the MemTree will return an Owner document due to how the MemTree is implemented,
                however the explicitlyCreated flag tells us whether there "really" was a Document Node or not.
                See https://github.com/eXist-db/exist/issues/1463
             */
            return null;

In this case, the 'path' returned by treeIndex does not contain the index (0) for the root node, and org.exist.xquery.functions.fn.transform.TreeUtils.xdmNodeAtIndex(XdmNode, List<Integer>) will return null when a node is expected.

The proposed fix tests for this case. Going up the ancestor chain, when a node that is not a document node, and that does not have a parent is found, the index 0 for this root element is inserted in the 'path'.

Testing

The NullPointerException happened in a rather complicated XQuery with documents composed from several sources.
I have not yet been able to make a simple test case, but I will keep trying.

Nico Verwer and others added 30 commits September 15, 2023 16:36
The Java code of DirectoryList throws an IllegalStateException when a non-existent directory is supplied as its first argument, which is an unchecked exception and therefore not caught in Java.
set :release to develop-6.x.x branch
[6.x.x] Repair JNLP interface, use correct BC library
- load the module from existdb instead of an external source
- test with and without xmldb: scheme
- fix template under test to actually do something
- ensure that the output is checked for correctness
When importing stylesheets the URIs are resolved in order

- registered import-uris in package repo
- XMLDB-locations (relative, absolute and with or without scheme)
- any other location is treated as an exteranal source

The test class and its cases were renamed to reflect their purpose.
Some inline comments were added to help describing the intent.
fixes eXist-db#5525
fixes eXist-db#5530

- add functx to autodeploy for xquery tests
- add tests for one-off queries with module imports
  - of a registered module without location hint
  - of a module with location hint
- change XQueryContext to allow imports again
- change SourceFactory to work with contextPath set to "."
keep :latest and :release tags aligned
add dev:6  tag for snapshots from develop-6.x.x

see eXist-db#3953
[6.x.x] allow module imports in one-off xqueries
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 25, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 26, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 26, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request May 30, 2025
@dizzzz dizzzz requested review from a team and DrRataplan July 24, 2025 09:25
@duncdrum duncdrum self-assigned this Aug 26, 2025
@duncdrum duncdrum added this to the eXist-6.4.1 milestone Aug 26, 2025
@duncdrum
Copy link
Contributor

@nverwer thank you for the time you spend on this. We would like to include this PR in a patch release of exist. Could you rebase the PR please so, we can start with a cleaner commit history? I will try to help you with adding a test case. Did you succeed in the meantime in wiggling things down? Could you share the xsl that triggered the NPE?

@nverwer
Copy link
Author

nverwer commented Aug 28, 2025

@nverwer Could you rebase the PR please so, we can start with a cleaner commit history?

What should I rebase this on? EDIT: I see, it is develop-6.x.x.

About a test case: We had the problem several months ago in a very large application. I could not make a concise test case back then, and it would be even more difficult now. Designing a test case requires a thorough understanding of the internal representations of XML in eXist-db, which I do not have.

I apologize for not coming up with a useful test. I am aware that eXist can only advance by the efforts of the community around it.
My main concern is to keep our application running, and this PR has helped us to use fn:transform instead of transform:transform.

@nverwer
Copy link
Author

nverwer commented Aug 28, 2025

After spending some time trying to rebase, it all seems to be terribly messed up, even resulting in automatically closing this PR. There are close to a 100 files with merge conflicts, most of which are totally unrelated to this PR. I have no idea why this went horribly wrong. I will give it one more try before giving up.

@nverwer
Copy link
Author

nverwer commented Aug 28, 2025

I tried to apply the changes to develop-6.x.x and will reopen this PR.

@nverwer nverwer reopened this Aug 28, 2025
@duncdrum
Copy link
Contributor

@nverwer thank you, gave me quite a scare there for a minute

@nverwer
Copy link
Author

nverwer commented Aug 28, 2025

@nverwer thank you, gave me quite a scare there for a minute
I should not have pushed prematurely. 😨
I could not reconstruct exactly what was in the original pull request, but it does include the fixes for the NPE. I added the code that allows maps and arrays to be passed as parameters to an XSL transformation. I probably should have made that into a separate pull request. 😬

@nverwer nverwer force-pushed the develop-6.x.x branch 2 times, most recently from ab0c76d to b196087 Compare September 23, 2025 09:31
@nverwer
Copy link
Author

nverwer commented Sep 23, 2025

In the meantime, my version of develop-6.x.x evolved, and I pushed it. I seems that this pull request was botched as a result.
Since nothing has happened with this pull request, I propose that it is closed without merging.

@nverwer
Copy link
Author

nverwer commented Sep 23, 2025

This PR became a mess after I changed my develop-6.x.x branch a few times, and these changes became visible here.
I got fed up with this PR hanging around without any progress, and closed it.
Then I decided to once more find the actual changes, and made a new PR: #5882

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs develop port needs Junit test Java test required to reproduce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants