Skip to content

Fix include directives for non-AsciiDoc files#777

Open
rolfedh wants to merge 1 commit intoquarkiverse:mainfrom
rolfedh:fix-asciidocj-include-non-adoc
Open

Fix include directives for non-AsciiDoc files#777
rolfedh wants to merge 1 commit intoquarkiverse:mainfrom
rolfedh:fix-asciidocj-include-non-adoc

Conversation

@rolfedh
Copy link

@rolfedh rolfedh commented Mar 10, 2026

Summary

  • Add configurable quarkus.asciidoc.include-extensions property to control which non-AsciiDoc file types the custom include processor handles
  • Default: .java,.kt,.js,.ts,.groovy,.scala (languages using //-style tag markers)
  • .adoc/.asciidoc files are always handled regardless of config
  • Fix URL prefix check (matches()find()) so URL include targets are correctly delegated to the default handler

Problem

The custom AsciidocJInclude processor's handles() method only accepted .adoc and .asciidoc targets. Include directives for other file types fell through to AsciidoctorJ's default include handler.

For co-located files (same directory), the default handler works fine. But for cross-directory includes using document attributes, the default handler fails because Roq feeds content to AsciidoctorJ via asciidoctor.load() (as a string, not from a file), so there is no source file context to resolve relative paths against.

For example, this pattern fails with the default handler:

[source,java]
----
include::{generated-dir}/examples/MyApp.java[tags=example]
----

Fix

Add a configurable allowlist of file extensions (quarkus.asciidoc.include-extensions) that the custom processor handles. The default list covers languages where //-style tag markers work correctly. Users can extend or restrict the list via application.properties.

The existing path resolution, security boundary enforcement (ROOTDIR check), and tag/line extraction logic already work correctly for all file types.

Test plan

  • All 15 tests pass (9 existing + 6 new AsciidocJIncludeHandlesTest for handles() logic)
  • Blog example builds successfully (mvn verify -pl blog)
  • Verified with quarkusio.github.io guides that use include:: for .java files with tag extraction

🤖 Generated with Claude Code

@rolfedh rolfedh requested a review from a team as a code owner March 10, 2026 02:13
Copy link
Collaborator

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

I would feel safer if we had a list of authrized extensions for include in the config

@jtama
Copy link
Member

jtama commented Mar 10, 2026

Also I just tried something like include::my-text-file.txt and that works perfectly fine. Am I missing something ?

@ia3andy
Copy link
Collaborator

ia3andy commented Mar 10, 2026

@jtama are you using jruby?

@jtama
Copy link
Member

jtama commented Mar 10, 2026

yes, here's what I tried. In one of my post folder I have two files :

  • index.adoc
  • Samples.java

In index.adoc :

Une fois qu'on a dit ça, on n'est pas bien avancé !

Si les `optionals` existent c'est avant tout pour donner une vision à un tiers, pour lui indiquer une intention, certainement pas pour éviter des `NullPointerException`.

== Quand les utiliser ?


On peut par exemple les utiliser quand on représente le monde extérieur. Il ne nous est en effet pas possible de le contrôler, en tout cas, nous n'en avons pas encore trouver le moyen.

Ici par exemple pour représenter une configuration externe :


[source,java]
----
include::Samples.java[tag=LeMondeExterieurConfig]
----

In Sample.java :

# tag::LeMondeExterieurConfig[]
public class LeMondeExterieurConfig {
    private Optional<String> login;
    private Optional<String> password;
    private Optional<Boolean> skiplogin;
}
# end::LeMondeExterieurConfig[]

# tag::LeMondeExterieurAcces[]
public interface LeMondeExterieurAcces {
    Optional<Person> findById(UUID id);
    List<Person> findByName(String name);
    Person findByNir(String nir) throws NotFoundException;
}
# end::LeMondeExterieurAcces[]

And the rendering :

image

@rolfedh rolfedh force-pushed the fix-asciidocj-include-non-adoc branch from a846f2a to 042a434 Compare March 10, 2026 13:07
@rolfedh
Copy link
Author

rolfedh commented Mar 10, 2026

@jtama Your example works because the included file (Samples.java) is co-located in the same post folder — in that case AsciidoctorJ's default include handler resolves the relative path fine.

The issue shows up with cross-directory includes that use document attributes to reference files elsewhere in the project, e.g.:

include::{generated-dir}/examples/MyApp.java[tags=example]

Since Roq feeds content to AsciidoctorJ via asciidoctor.load() (as a string, not from a file), the default handler has no source file context to resolve those paths against. The custom processor uses the BASEDIR/ROOTDIR options to resolve them.

Per @ia3andy's feedback, the allowed extensions are now configurable via quarkus.asciidoc.include-extensions (defaults to .java,.kt,.js,.ts,.groovy,.scala — languages that use //-style tag markers). Extensions like .yaml, .xml, .properties are excluded from the default because the custom tag extraction only supports // comment markers; users can add them if they don't use tag filtering.

@rolfedh
Copy link
Author

rolfedh commented Mar 10, 2026

Thanks @ia3andy and @jtama for the feedback! Here's what changed in the latest force-push:

  • Configurable allowlist: Added quarkus.asciidoc.include-extensions config property (defaults to .java,.kt,.js,.ts,.groovy,.scala)
  • .adoc/.asciidoc always handled regardless of config
  • Narrowed defaults to languages using //-style tag markers — excluded .xml, .json, .yaml, .properties, .txt since the custom tag extraction doesn't support # or <!-- --> comment styles
  • Fixed URL prefix check: matches()find() so URL include targets are correctly delegated to the default handler (pre-existing bug)
  • Added 6 unit tests for the handles() filtering logic
  • Updated PR description to reflect the new approach

@rolfedh rolfedh requested a review from ia3andy March 10, 2026 13:19
@ia3andy
Copy link
Collaborator

ia3andy commented Mar 10, 2026

I need to run a few check locally to remember how that behaves

@jtama
Copy link
Member

jtama commented Mar 10, 2026

if it's left as is, we could include resources outside of the project ? And in anycase, what is the base path if the include is a relative one ?

Add configurable extension allowlist (quarkus.asciidoc.include-extensions)
for Roq's custom include processor. The default list covers languages that
use //-style tag markers (.java, .kt, .js, .ts, .groovy, .scala).
The .adoc/.asciidoc extensions are always handled regardless of config.

This enables cross-directory and classpath-based include:: directives for
source files in technical documentation. Co-located includes already work
via the default AsciidoctorJ handler.

Also fix URL prefix check to use find() instead of matches(), which
previously failed to reject URL targets due to matches() requiring the
entire string to match.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rolfedh rolfedh force-pushed the fix-asciidocj-include-non-adoc branch from 042a434 to 5fe51ee Compare March 11, 2026 14:30
@rolfedh
Copy link
Author

rolfedh commented Mar 11, 2026

@jtama Great questions, thank you!

Security boundary: The process() method checks that the resolved include path stays within rootDir (lines 60-64) — if not, a SecurityException is thrown. This applies at SAFE mode and above, so includes cannot escape the project directory.

Your question actually helped me spot a pre-existing issue: absolute paths were not being normalized before the boundary check, so a path like /project/../etc/passwd could theoretically pass the startsWith(rootDir) check while resolving outside the root. I've fixed this in the latest force-push by moving .normalize() to apply to both absolute and relative paths:

// Before (only relative paths normalized):
Path targetPath = p.isAbsolute() ? p : baseDir.resolve(dir).resolve(target).normalize();

// After (both branches normalized):
Path targetPath = (p.isAbsolute() ? p : baseDir.resolve(dir).resolve(target)).normalize();

Base path for relative includes: Relative paths resolve against baseDir + reader.getDir(), where baseDir comes from AsciidoctorJ's BASEDIR option and reader.getDir() is the current document's directory context. So for co-located files (like your example), it resolves to the post folder. For cross-directory includes using attributes like {generated-dir}, the attribute expands first, making the path relative to baseDir.

Note: normalize() is purely lexical — it doesn't resolve symlinks. That matches AsciidoctorJ's own behavior and is a reasonable trade-off since toRealPath() would break the classloader resource flow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants