Skip to content

XRENDERING-815: XWiki syntax renderer doesn't escape closing macro syntax in various attributes - #433

Merged
michitux merged 4 commits into
xwiki:masterfrom
michitux:XRENDERING-815
Sep 14, 2026
Merged

michitux merged 4 commits into
xwiki:masterfrom
michitux:XRENDERING-815

Conversation

@michitux

Copy link
Copy Markdown
Member

Jira URL

https://jira.xwiki.org/browse/XRENDERING-815

Changes

Description

  • Escape the "{" runs of every value that the renderer serializes as-is:
    • (%...%) parameter values
    • link and image references, their parameters, and the xwiki/2.1 queryString and anchor reference parameters
    • the id macro name, which had no escaping at all and could also be broken by a quote in the name
  • Introduce XWikiSyntaxEscapeHandler#escapeCurlyBrackets as escaping helper that correctly escapes every character of a run of "{" instead of only the pairs: the previous two-pass "{{{"-then-"{{" replacement corrupted runs of four "{" and left a literal "{{" behind for runs of five, so it could still break out of a macro.
  • For free-standing references which cannot be escaped at all, fall back to the full [[...]] syntax when printing the reference free-standing would put a "{{" into the output, as that could close the macro the reference is serialized in. This is only about the macro syntax: the image and attachment tokens of the parser accept a "{{" and parse such a reference back unchanged, and where they don't - the URI token accepts no "{" at all - the reference is truncated just like by every other character that token rejects (a "}" or a "," for instance). That pre-existing limitation of free-standing references is not addressed here.
  • Correctly escape the reference of links that were initially freestanding when forced into the full syntax.
  • Expect [[{{macro}}]] in links6.test: the escaped form parses back to the very same reference while the previous output could badly interfere with outer macro syntax.

Clarifications

  • This fixes more than the original issue description, the general aim of this is to improve the correctness of the XWiki syntax renderer to make parse-render roundtrips correct.
  • Both bug investigation and implementation were mostly done by Claude Opus 5 (1M context) noreply@anthropic.com with some guidance and review from my side.

Screenshots & Video

No UI changes.

Executed Tests

Whole xwiki-rendering with quality profile.

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • stable-18.4.x
    • stable-17.10.x
    • stable-16.10.x

…ntax in various attributes

* Escape the "{" runs of every value that the renderer serializes as-is:
    * (%...%) parameter values
    * link and image references, their parameters, and the xwiki/2.1
      queryString and anchor reference parameters
    * the id macro name, which had no escaping at all and could also be
      broken by a quote in the name
* Introduce XWikiSyntaxEscapeHandler#escapeCurlyBrackets as escaping
  helper that correctly escapes every character of a run of "{" instead
  of only the pairs: the previous two-pass "{{{"-then-"{{" replacement
  corrupted runs of four "{" and left a literal "{{" behind for runs of
  five, so it could still break out of a macro.
* For free-standing references which cannot be escaped at all, fall back
  to the full [[...]] syntax when printing the reference free-standing
  would put a "{{" into the output, as that could close the macro the
  reference is serialized in. This is only about the macro syntax: the
  image and attachment tokens of the parser accept a "{{" and parse such
  a reference back unchanged, and where they don't - the URI token
  accepts no "{" at all - the reference is truncated just like by every
  other character that token rejects (a "}" or a "," for instance). That
  pre-existing limitation of free-standing references is not addressed
  here.
* Correctly escape the reference of links that were initially
  freestanding when forced into the full syntax.
* Expect [[~{~{macro}}]] in links6.test: the escaped form parses back to
  the very same reference while the previous output could badly
  interfere with outer macro syntax.

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

XWikiSyntaxChainingRenderer can still emit an IdBlock incorrectly after a preceding {; it should use the inline-macro printing path.

Pull request overview

Fixes XWiki 2.0/2.1 renderer round-trip issues caused by unescaped {{ sequences in macro content.

Changes:

  • Adds robust curly-bracket escaping for parameters, references, IDs, and link metadata.
  • Uses full link syntax for unsafe free-standing references.
  • Adds regression and round-trip tests.
File summaries
File Description
xwiki-rendering-syntaxes/xwiki-rendering-syntax-xwiki21/src/test/java/org/xwiki/rendering/internal/renderer/xwiki21/XWikiSyntaxMacroContentRoundTripTest.java Adds XWiki 2.1 macro-content round-trip coverage.
xwiki-rendering-syntaxes/xwiki-rendering-syntax-xwiki21/src/main/java/org/xwiki/rendering/internal/renderer/xwiki21/reference/XWikiSyntaxResourceRenderer.java Escapes XWiki 2.1 references, query strings, anchors, and link parameters.
xwiki-rendering-syntaxes/xwiki-rendering-syntax-xwiki20/src/test/java/org/xwiki/rendering/internal/renderer/xwiki20/XWikiSyntaxMacroContentRoundTripTest.java Adds XWiki 2.0 macro-content round-trip coverage.
xwiki-rendering-syntaxes/xwiki-rendering-syntax-xwiki20/src/test/java/org/xwiki/rendering/internal/renderer/xwiki20/XWikiSyntaxFreeStandingReferenceTest.java Tests full-syntax fallback for unsafe references.
xwiki-rendering-syntaxes/xwiki-rendering-syntax-xwiki20/src/main/java/org/xwiki/rendering/internal/renderer/xwiki20/XWikiSyntaxEscapeHandler.java Escapes complete curly-bracket runs.
xwiki-rendering-syntaxes/xwiki-rendering-syntax-xwiki20/src/main/java/org/xwiki/rendering/internal/renderer/xwiki20/XWikiSyntaxChainingRenderer.java Escapes parameters and ID macro values.
xwiki-rendering-syntaxes/xwiki-rendering-syntax-xwiki20/src/main/java/org/xwiki/rendering/internal/renderer/xwiki20/reference/XWikiSyntaxResourceRenderer.java Escapes references and selects safe link syntax.
xwiki-rendering-integration-tests/src/test/resources/wiki/link/links6.test Updates escaped-reference expectations.
xwiki-rendering-integration-tests/src/test/resources/simple/macros/macro39.test Adds escaped link-reference coverage.
xwiki-rendering-integration-tests/src/test/resources/simple/macros/macro38.test Adds escaped macro-parameter coverage.
Review details

Suppressed comments (1)

xwiki-rendering-syntaxes/xwiki-rendering-syntax-xwiki20/src/main/java/org/xwiki/rendering/internal/renderer/xwiki20/XWikiSyntaxChainingRenderer.java:532

  • This emits an inline macro through print(...), so the escape printer does not apply its printInlineMacro() guard for a preceding delayed {. An IdBlock immediately following text { can therefore render as {{{id..., which the parser treats as verbatim syntax instead of a literal brace followed by the id macro. Use the inline-macro printing path here, as onMacro() does, so the preceding brace is escaped.
        print(getMacroPrinter().renderMacro("id", Map.of("name", name), null, true));
  • Files reviewed: 10/10 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

michitux and others added 2 commits September 11, 2026 10:47
…ntax in various attributes

* Print the id macro through the inline macro printing of the printer so
  that a "{" printed just before it is escaped: otherwise the output
  started with "{{{" and was parsed back as a verbatim block.
* Keep the bookkeeping of the regular printing - marking the first
  element as rendered and closing pending empty formatting parameters -
  by extracting it into printInlineMacro() instead of delegating to
  onMacro(), whose inline branch skips both and would thus glue a
  following paragraph to a standalone id macro.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ntax in various attributes

* Replace deprecated methods in the changed code by their non-deprecated
  equivalents - the escape character "~" is already passed in the
  constructor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@michitux
michitux requested a review from tmortagne September 11, 2026 11:07
…ntax in various attributes

* Clarify comment regarding macro content escaping.
@michitux
michitux merged commit 3352bcf into xwiki:master Sep 14, 2026
4 checks passed
@michitux
michitux deleted the XRENDERING-815 branch September 14, 2026 13:06
@github-actions

Copy link
Copy Markdown

💔 All backports failed

Status Branch Result
❌ stable-16.10.x Backport failed because of merge conflicts
❌ stable-17.10.x Backport failed because of merge conflicts
❌ stable-18.4.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

backport --pr 433

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

michitux added a commit that referenced this pull request Sep 15, 2026
…ntax in various attributes (#433)

* Escape the "{" runs of every value that the renderer serializes as-is:
    * (%...%) parameter values
    * link and image references, their parameters, and the xwiki/2.1
      queryString and anchor reference parameters
    * the id macro name, which had no escaping at all and could also be
      broken by a quote in the name
* Introduce XWikiSyntaxEscapeHandler#escapeCurlyBrackets as escaping
  helper that correctly escapes every character of a run of "{" instead
  of only the pairs: the previous two-pass "{{{"-then-"{{" replacement
  corrupted runs of four "{" and left a literal "{{" behind for runs of
  five, so it could still break out of a macro.
* For free-standing references which cannot be escaped at all, fall back
  to the full [[...]] syntax when printing the reference free-standing
  would put a "{{" into the output, as that could close the macro the
  reference is serialized in. This is only about the macro syntax: the
  image and attachment tokens of the parser accept a "{{" and parse such
  a reference back unchanged, and where they don't - the URI token
  accepts no "{" at all - the reference is truncated just like by every
  other character that token rejects (a "}" or a "," for instance). That
  pre-existing limitation of free-standing references is not addressed
  here.
* Correctly escape the reference of links that were initially
  freestanding when forced into the full syntax.
* Expect [[~{~{macro}}]] in links6.test: the escaped form parses back to
  the very same reference while the previous output could badly
  interfere with outer macro syntax.
* Print the id macro through the inline macro printing of the printer so
  that a "{" printed just before it is escaped: otherwise the output
  started with "{{{" and was parsed back as a verbatim block.
* Keep the bookkeeping of the regular printing - marking the first
  element as rendered and closing pending empty formatting parameters -
  by extracting it into printInlineMacro() instead of delegating to
  onMacro(), whose inline branch skips both and would thus glue a
  following paragraph to a standalone id macro.
* Replace deprecated methods in the changed code by their non-deprecated
  equivalents - the escape character "~" is already passed in the
  constructor.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 3352bcf)
michitux added a commit that referenced this pull request Sep 15, 2026
…ntax in various attributes (#433)

* Escape the "{" runs of every value that the renderer serializes as-is:
    * (%...%) parameter values
    * link and image references, their parameters, and the xwiki/2.1
      queryString and anchor reference parameters
    * the id macro name, which had no escaping at all and could also be
      broken by a quote in the name
* Introduce XWikiSyntaxEscapeHandler#escapeCurlyBrackets as escaping
  helper that correctly escapes every character of a run of "{" instead
  of only the pairs: the previous two-pass "{{{"-then-"{{" replacement
  corrupted runs of four "{" and left a literal "{{" behind for runs of
  five, so it could still break out of a macro.
* For free-standing references which cannot be escaped at all, fall back
  to the full [[...]] syntax when printing the reference free-standing
  would put a "{{" into the output, as that could close the macro the
  reference is serialized in. This is only about the macro syntax: the
  image and attachment tokens of the parser accept a "{{" and parse such
  a reference back unchanged, and where they don't - the URI token
  accepts no "{" at all - the reference is truncated just like by every
  other character that token rejects (a "}" or a "," for instance). That
  pre-existing limitation of free-standing references is not addressed
  here.
* Correctly escape the reference of links that were initially
  freestanding when forced into the full syntax.
* Expect [[~{~{macro}}]] in links6.test: the escaped form parses back to
  the very same reference while the previous output could badly
  interfere with outer macro syntax.
* Print the id macro through the inline macro printing of the printer so
  that a "{" printed just before it is escaped: otherwise the output
  started with "{{{" and was parsed back as a verbatim block.
* Keep the bookkeeping of the regular printing - marking the first
  element as rendered and closing pending empty formatting parameters -
  by extracting it into printInlineMacro() instead of delegating to
  onMacro(), whose inline branch skips both and would thus glue a
  following paragraph to a standalone id macro.
* Replace deprecated methods in the changed code by their non-deprecated
  equivalents - the escape character "~" is already passed in the
  constructor.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 3352bcf)
michitux added a commit that referenced this pull request Sep 15, 2026
…ntax in various attributes (#433)

* Escape the "{" runs of every value that the renderer serializes as-is:
    * (%...%) parameter values
    * link and image references, their parameters, and the xwiki/2.1
      queryString and anchor reference parameters
    * the id macro name, which had no escaping at all and could also be
      broken by a quote in the name
* Introduce XWikiSyntaxEscapeHandler#escapeCurlyBrackets as escaping
  helper that correctly escapes every character of a run of "{" instead
  of only the pairs: the previous two-pass "{{{"-then-"{{" replacement
  corrupted runs of four "{" and left a literal "{{" behind for runs of
  five, so it could still break out of a macro.
* For free-standing references which cannot be escaped at all, fall back
  to the full [[...]] syntax when printing the reference free-standing
  would put a "{{" into the output, as that could close the macro the
  reference is serialized in. This is only about the macro syntax: the
  image and attachment tokens of the parser accept a "{{" and parse such
  a reference back unchanged, and where they don't - the URI token
  accepts no "{" at all - the reference is truncated just like by every
  other character that token rejects (a "}" or a "," for instance). That
  pre-existing limitation of free-standing references is not addressed
  here.
* Correctly escape the reference of links that were initially
  freestanding when forced into the full syntax.
* Expect [[~{~{macro}}]] in links6.test: the escaped form parses back to
  the very same reference while the previous output could badly
  interfere with outer macro syntax.
* Print the id macro through the inline macro printing of the printer so
  that a "{" printed just before it is escaped: otherwise the output
  started with "{{{" and was parsed back as a verbatim block.
* Keep the bookkeeping of the regular printing - marking the first
  element as rendered and closing pending empty formatting parameters -
  by extracting it into printInlineMacro() instead of delegating to
  onMacro(), whose inline branch skips both and would thus glue a
  following paragraph to a standalone id macro.
* Replace deprecated methods in the changed code by their non-deprecated
  equivalents - the escape character "~" is already passed in the
  constructor.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 3352bcf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants