Skip to content

fix(NcRichText): do not render invalid relative markdown links#8397

Merged
Antreesy merged 2 commits intomainfrom
fix/noid/external-links
Apr 23, 2026
Merged

fix(NcRichText): do not render invalid relative markdown links#8397
Antreesy merged 2 commits intomainfrom
fix/noid/external-links

Conversation

@Antreesy
Copy link
Copy Markdown
Contributor

@Antreesy Antreesy commented Apr 7, 2026 •
edited
Loading

☑️ Resolves

fix(NcRichText)!: render all non-resolved links as external

  • this covers:
    • external links (as before)
    • internal absolute links, but from another app (as before)
    • and internal relative links, that do not have a router match for current app (new)
      • possibly a breaking change?
      • this is likely an invalid user input, it shouldn't unload the page and destroy an app

What would be rendered for component <NcRichText :autolink="true" :useMarkdown="true" />:

  • https://autolink.me - https://autolink.me
  • [hello](/world) - hello with href: '/world'
  • [hello](https://nextcloud.com) - hello with href: 'https://nextcloud.com'
  • [hello](tel:+49123456789) - hello with href: 'tel:+49123456789'
  • [hello](mailto:+49123456789) - hello with href: 'mailto:+49123456789'
  • [hello](other:proto) - hello as plain text
  • [hello](world) - hello as plain text
  • [hello](?parameters=1) - hello as plain text
  • [hello](#anchor) - hello as plain text

🚧 Tasks

  • Consult other teams for using of relative links, whether it's breaking for them

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 2️⃣ Backport to stable8 for maintained Vue 2 version or not applicable

@Antreesy Antreesy added this to the 9.6.0 milestone Apr 7, 2026
@Antreesy Antreesy requested a review from ShGKme April 7, 2026 11:46
@Antreesy Antreesy self-assigned this Apr 7, 2026
@Antreesy Antreesy added 3. to review Waiting for reviews feature: richtext Related to the richtext component labels Apr 7, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 7, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.47%. Comparing base (2e3ba39) to head (a5ae090).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/components/NcRichText/NcRichText.vue 50.00% 4 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8397      +/-   ##
==========================================
- Coverage   54.50%   54.47%   -0.04%     
==========================================
  Files         104      105       +1     
  Lines        3405     3411       +6     
  Branches      995      996       +1     
==========================================
+ Hits         1856     1858       +2     
- Misses       1309     1313       +4     
  Partials      240      240

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Antreesy Antreesy modified the milestones: 9.6.0, 9.7.0 Apr 15, 2026
susnux
susnux approved these changes Apr 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

otherwise fine :)

Comment thread src/components/NcRichText/NcRichTextExternalLink.vue Outdated
ShGKme
ShGKme reviewed Apr 20, 2026
View reviewed changes
Comment thread src/components/NcRichText/autolink.ts
ShGKme
ShGKme reviewed Apr 20, 2026
View reviewed changes
Comment thread src/components/NcRichText/NcRichTextExternalLink.vue
ShGKme
ShGKme reviewed Apr 20, 2026
View reviewed changes
Comment thread src/components/NcRichText/NcRichTextExternalLink.vue Outdated
@ShGKme
Copy link
Copy Markdown
Contributor

ShGKme commented Apr 20, 2026

fix(NcRichText)!: render all non-resolved links as external

  • this covers:

    • external links (as before)

    • internal absolute links, but from another app (as before)

    • and internal relative links, that do not have a router match for current app (new)

      • possibly a breaking change?
      • this is likely an invalid user input, it shouldn't unload the page and destroy an app

I'd even consider forbidding such links.

F relative link can even be a link to a completely different service if Nextcloud is hosted on a non-empty base URL.

@ShGKme ShGKme added the bug Something isn't working label Apr 20, 2026
@Antreesy Antreesy mentioned this pull request Apr 21, 2026
Merged
3 tasks
@Antreesy Antreesy force-pushed the fix/noid/external-links branch from 6b21c2b to ad1d51c Compare April 21, 2026 12:24
@Antreesy Antreesy changed the base branch from main to fix/8397/nested-links April 21, 2026 12:26
@Antreesy Antreesy requested review from ShGKme and susnux April 21, 2026 12:26
@Antreesy
Copy link
Copy Markdown
Contributor Author

Antreesy commented Apr 21, 2026 •
edited
Loading

I'd even consider forbidding such links.
F relative link can even be a link to a completely different service if Nextcloud is hosted on a non-empty base URL.

See last commit for the change 👇🏽

@Antreesy Antreesy force-pushed the fix/noid/external-links branch from ad1d51c to 03e8416 Compare April 22, 2026 15:14
DorraJaouad
DorraJaouad reviewed Apr 23, 2026
View reviewed changes
Comment thread src/components/NcRichText/NcRichTextExternalLink.vue
Comment thread src/components/NcRichText/NcRichText.vue
Base automatically changed from fix/8397/nested-links to main April 23, 2026 12:12
DorraJaouad
DorraJaouad approved these changes Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

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

All good then

Antreesy added 2 commits April 23, 2026 14:53
@Antreesy
fix: extract NcRichTextExternalLink component
97636a4 
- add noExtDecoration prop for reusing the component

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy
fix(NcRichText)!: do not render non-resolved relative links
a5ae090 
- still works: external links, internal absolute links, and internal relative links, that do have a router match for current app
- internal relative links without match (also ?query and #anchor) only render an inner text
- allow tel: and mail: to reduce breaking changes

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy force-pushed the fix/noid/external-links branch from 2b4160d to a5ae090 Compare April 23, 2026 13:02
@Antreesy Antreesy changed the title fix(NcRichText): render invalid relative markdown links as external fix(NcRichText): do not render invalid relative markdown links Apr 23, 2026
@Antreesy Antreesy merged commit 0aadabf into main Apr 23, 2026
27 checks passed
@Antreesy Antreesy deleted the fix/noid/external-links branch April 23, 2026 13:08
@Antreesy
Copy link
Copy Markdown
Contributor Author

Antreesy commented Apr 23, 2026

/backport to stable8

@backportbot backportbot Bot mentioned this pull request Apr 23, 2026
Draft
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ShGKme ShGKme ShGKme approved these changes

@DorraJaouad DorraJaouad DorraJaouad approved these changes

@mejo- mejo- Awaiting requested review from mejo-

@susnux susnux Awaiting requested review from susnux

Assignees

@Antreesy Antreesy

Labels

3. to review Waiting for reviews bug Something isn't working feature: richtext Related to the richtext component

Projects

None yet

Milestone

9.7.0

Development

Successfully merging this pull request may close these issues.

4 participants

@Antreesy @ShGKme @susnux @DorraJaouad