Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(richText): do not handle relative links without leading slash as router links #5703

Merged

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jun 13, 2024

☑️ Resolves

  • Fix Mailto in avatar overlay is wrong nextcloud/spreed#12516
  • autolink supports relative URLs, for example, /call/ab34cd is resolved as a Talk route.
  • It considers a link as relative if it doesn't start with https?://.
  • However, call/abc or mailto:email@nextcloud.ltd also doesn't start with HTTP.
  • The best is to only allow relative URLs with leading slashes /.
  • Even the check for leading / is enough for explicitly adding a check for non-http schemes.

VueRouter handles such URLs according to the URL RFC, for example, being on /call/abc page, a link def is resolved as /call/def relative to /call/. We don't want to support such links anyway, so relative URL MUST start with a slash.

Alternative solution: add a leading slash / to relative links.

🚧 Tasks

  • Check for non-HTTP schemes.
  • Support relative links only if they start with /.

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

@ShGKme ShGKme added bug Something isn't working 3. to review Waiting for reviews feature: avatar Related to the avatar component feature: richtext Related to the richtext component labels Jun 13, 2024
@ShGKme ShGKme added this to the 8.13.0 milestone Jun 13, 2024
@ShGKme ShGKme self-assigned this Jun 13, 2024
@ShGKme ShGKme force-pushed the fix/autolink--support-non-http-scheme-and-no-trailing-slash branch 2 times, most recently from 3c3e641 to d43acf1 Compare June 13, 2024 14:56
@ShGKme ShGKme changed the title fix(richText): do not handle relative links without trailing slash as router links fix(richText): do not handle relative links without leading slash as router links Jun 13, 2024
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the fix/autolink--support-non-http-scheme-and-no-trailing-slash branch from d43acf1 to 752fa36 Compare June 13, 2024 15:28
Copy link
Contributor

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Looks like it should fix it, but didn't test

@susnux susnux merged commit 5fa538a into master Jun 13, 2024
19 checks passed
@susnux susnux deleted the fix/autolink--support-non-http-scheme-and-no-trailing-slash branch June 13, 2024 15:33
@susnux
Copy link
Contributor

susnux commented Jun 13, 2024

/backport to next

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: avatar Related to the avatar component feature: richtext Related to the richtext component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mailto in avatar overlay is wrong
3 participants