Skip to content

[bugfix] Forward port PR #6544: fix XSLT imports and document builder isolation - #6546

Merged
dizzzz merged 7 commits into
eXist-db:developfrom
line-o:port/develop-6544
Jul 16, 2026
Merged

[bugfix] Forward port PR #6544: fix XSLT imports and document builder isolation#6546
dizzzz merged 7 commits into
eXist-db:developfrom
line-o:port/develop-6544

Conversation

@line-o

@line-o line-o commented Jul 5, 2026

Copy link
Copy Markdown
Member

Summary

Forward port of PR #6544 from develop-6.x.x to develop, fixing critical issues with XSLT stylesheet resolution and document builder isolation in fn:transform. Adapted for Saxon 12.5 compatibility.

Fixes:

What Changed

1. Document Builder Isolation (Delivery.java)

Problem: When multiple fn:transform calls execute in the same XQuery context, they share the same MemTreeBuilder. This causes output corruption when transforms execute concurrently.

Solution: Always use a fresh MemTreeBuilder for each transform result instead of reusing the context's builder.

2. Persistent Node Handling (Convert.java)

Problem: Persistent nodes stored in the database (NodeProxy instances) weren't properly dereferenced when passed as stylesheet parameters.

Solution:

  • Check Document interface instead of specific DocumentImpl class
  • Dereference NodeProxy to underlying DOM node in of(Sequence) method
  • Enables passing database-stored nodes as stylesheet parameters

3. XSLT Stylesheet Resolution for Relative URIs (Issue #5052 - Root Cause Fix)

Root Causes (discovered during porting):

  1. URIResolution.resolveURI(): Database paths like "/db/collection/stylesheet.xsl" were incorrectly treated as relative URIs because URI.isAbsolute() returns false (they lack a scheme component).

    • Fix: Treat database paths (starting with "/" or "xmldb:") as absolute for resolution purposes
  2. URIResolution.resolveDocument(): Did not set SystemId on returned DOMSource objects. When Saxon resolved and loaded included stylesheets, it had no base URI for resolving further nested includes.

    • Fix: Set SystemId = location on all returned DOMSource objects
  3. Options.resolvePossibleStylesheetLocation(): Simplified to use the resolved location directly as SystemId, ensuring consistent base URI setup for Saxon's compile-time resolver.

4. Other Changes

Transform.java: Updated createDestination() call sites to match new Delivery.java signature

Test Coverage

Two comprehensive XQSuite test modules verify all fixes:

  • fnTransform5052.xqm (13 tests): Relative stylesheet resolution with xsl:include/xsl:import
  • fnTransform6065.xqm (2 tests): Persistent database nodes as stylesheet parameters

Result: All 7110 tests pass with zero failures and zero errors.

@line-o
line-o requested a review from a team as a code owner July 5, 2026 12:49
@line-o
line-o marked this pull request as draft July 5, 2026 13:31
@line-o

line-o commented Jul 5, 2026

Copy link
Copy Markdown
Member Author

I propose to squash all four commits

[bugfix] Forward port PR #6544: fix XSLT imports and document builder isolation

Fixes #5051 (XSLT import resolution) and #6065 (document builder isolation) by
porting and refining key changes from develop-6.x.x, adapted for Saxon 12.5.

## Document Builder Isolation (Issue #6065)
- Always use fresh MemTreeBuilder for fn:transform results instead of sharing
  the query context's builder
- Prevents corruption when multiple fn:transform calls occur in parallel within
  the same execution context
- Fixes concurrent transform output corruption

## Persistent Node Handling
- Handle persistent NodeProxy nodes by dereferencing to underlying DOM nodes
- Enables passing database-stored nodes as stylesheet parameters to fn:transform
- Uses Document interface checks instead of specific implementation classes

## Stylesheet Resolution for Relative xsl:include/xsl:import (Issue #5052)

Root cause analysis revealed three interconnected issues:

1. URIResolution.resolveURI() incorrectly treated database paths (starting with
   "/" or "xmldb:") as relative URIs because they lacked a scheme component.
   A path like "/db/collection/stylesheet.xsl" is an absolute path but not an
   absolute URI in the RFC 3986 sense. Fixed by checking for database path
   prefixes in addition to URI scheme.

2. URIResolution.resolveDocument() did not set SystemId on returned DOMSource
   objects. When Saxon resolved xsl:include/xsl:import hrefs and loaded
   included stylesheets, it had no base URI for resolving further nested
   includes. Fixed by setting SystemId = location on all returned sources.

3. Options.resolvePossibleStylesheetLocation() simplified to use the resolved
   location parameter directly as SystemId, ensuring consistent base URI setup
   for Saxon's compile-time URI resolver. This supports the chain:
   - Stylesheet at /db/coll/main.xsl
   - Includes sub/inc.xsl → resolved to /db/coll/sub/inc.xsl (base URI via SystemId)
   - Includes nested.xsl → resolved to /db/coll/sub/nested.xsl (base URI via SystemId)

## Convert.java
- Check Document interface instead of specific DocumentImpl implementation
- Add NodeProxy dereference in of(Sequence) method to handle persistent nodes

## Transform.java
- Updated createDestination() call sites for new Delivery.java signature

## Test Coverage
- fnTransform5052.xqm: 13 tests for relative xsl:include/xsl:import and
  stylesheet-location resolution in database-stored stylesheets
- fnTransform6065.xqm: 2 tests for persistent nodes as stylesheet parameters

All tests pass with exit code 0.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

@line-o
line-o marked this pull request as ready for review July 5, 2026 18:39
line-o and others added 4 commits July 5, 2026 20:54
… builder isolation

Fixes eXist-db#5052 and eXist-db#6065 by porting key changes from develop-6.x.x:

1. Document builder isolation (Delivery.java):
   - Always use fresh MemTreeBuilder for transform results
   - Prevents corruption when multiple fn:transform calls occur in same context
   - Fixes issue eXist-db#6065 where concurrent transforms corrupted each other's output

2. Persistent node handling (Convert.java):
   - Handle persistent NodeProxy nodes by dereferencing to underlying DOM
   - Enables passing database-stored nodes as stylesheet parameters
   - Checks Document interface instead of specific implementation

3. Enhanced stylesheet location resolution (Options.java):
   - Return Tuple2<String, Source> to track actual resolved location
   - Sets SystemId on DOMSource for proper relative URI resolution
   - Fallback resolution for relative URIs using RFC 3986 + database resolution

4. Runtime URI resolution (Transform.java):
   - Install runtime URI resolver on XSLT controller
   - Enables fn:document() calls within stylesheets to resolve against database
   - Integrates with existing URIResolution.CompileTimeURIResolver
   - Adds helper methods: newFnTransformURIResolver(), databaseBaseURI(), isDatabaseURI()
   - Supports xsl:include/xsl:import resolution for stylesheets stored in database

5. Build configuration:
   - Downgrade IzPack to 5.2.3 (5.2.4+ requires Java 9+)

Test coverage:
- fnTransform5052.xqm: Tests relative xsl:include/import and stylesheet-location resolution
- fnTransform6065.xqm: Tests persistent nodes as stylesheet parameters

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Database URIs (paths starting with /, xmldb:, or exist://) should not be
resolved against the query context base URI, as they are already absolute
database paths. This was preventing the compile-time resolver from
correctly using the base URI to resolve relative xsl:include/xsl:import
hrefs in stylesheets stored in the database.

Fixes test failures in fnTransform5052.xqm where relative stylesheet
locations were not being resolved correctly.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
When the owner document's URI is null, use the location parameter as
the fallback for the actual location. This ensures the SystemId is
always set correctly for the compile-time resolver to use when
resolving relative xsl:include/xsl:import hrefs.

Fixes remaining test failures where relative stylesheet paths were
not being resolved.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…import paths

Fixes issues eXist-db#5052 and eXist-db#6065 by ensuring correct base URI handling for
stylesheet resolution:

1. URIResolution.resolveURI: Treat database paths (starting with "/" or
   "xmldb:") as absolute URIs for relative URI resolution. Previously,
   paths without a scheme were incorrectly treated as relative, preventing
   proper resolution of xsl:include/xsl:import hrefs.

2. URIResolution.resolveDocument: Set SystemId on all returned DOMSource
   objects so that nested includes/imports are resolved with the correct
   base URI. This ensures xsl:include statements in included stylesheets
   can correctly resolve their own relative paths.

3. Options.resolvePossibleStylesheetLocation: Simplified to use the
   resolved location parameter directly as the SystemId, ensuring
   consistent base URI setup for Saxon's compile-time URI resolver.

All fnTransform5052 and fnTransform6065 tests now pass.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@line-o
line-o force-pushed the port/develop-6544 branch from cb359a8 to b791e72 Compare July 5, 2026 18:55
@line-o line-o mentioned this pull request Jul 6, 2026
@line-o

line-o commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

@duncdrum pointed out that #5051 needs to be looked at in this context

@dizzzz
dizzzz requested review from a team, duncdrum, joewiz and reinhapa July 6, 2026 20:11
Comment thread exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Options.java Outdated
Comment thread exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Options.java Outdated
@line-o

line-o commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Good catches will change code - wonder if this is covered by a codacy rule

@line-o line-o added xquery issue is related to xquery implementation XSLT Issues arising from handing over to or retrieving values from Saxon get this label. labels Jul 9, 2026
@line-o line-o added this to v7.0.0 Jul 9, 2026
line-o and others added 3 commits July 12, 2026 10:25
…olution

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ldb: base

XmldbURI#getURI strips the xmldb: prefix for xmldb:exist:// URIs but keeps
it for the short xmldb:/db/... form, so unconditionally prepending it yielded
xmldb:xmldb:/db/... A resolved stylesheet is now its own system id, so this
was reached by any relative xsl:import within an imported stylesheet.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… stylesheet

RFC 3986 discards the last segment of the base, which is correct for a document
but not for a collection: resolving style.xsl against the collection /db/apps/app
yielded /db/apps/style.xsl. A collection and a document are not distinguishable by
path alone, so the absence of an extension in the last segment is taken to mean a
collection. Outside the database RFC 3986 still applies unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@line-o
line-o requested review from dizzzz and reinhapa July 12, 2026 10:39

@dizzzz dizzzz left a comment

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.

what about #5051 ?

@line-o

line-o commented Jul 12, 2026

Copy link
Copy Markdown
Member Author

What about it? It's mentioned in description.

@line-o

line-o commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

Is there something that blocks this PR I am not aware of?

@dizzzz
dizzzz merged commit 089bd02 into eXist-db:develop Jul 16, 2026
9 checks passed
@github-project-automation github-project-automation Bot moved this to Done in v7.0.0 Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

xquery issue is related to xquery implementation XSLT Issues arising from handing over to or retrieving values from Saxon get this label.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants