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

Don't allow relative links in comments #1578

Merged
merged 1 commit into from
Apr 10, 2023
Merged

Conversation

paskal
Copy link
Sponsor Collaborator

@paskal paskal commented Jan 9, 2023

(url) is a text inserted by default and never an intended URL.

That additional validation will ensure that users won't post relative links because they are rarely intended.

This is how it looks without frontend changes:

image

Backend part for #809.

@paskal paskal added the backend label Jan 9, 2023
@paskal paskal requested a review from akellbl4 January 9, 2023 20:20
@paskal paskal requested a review from umputun as a code owner January 9, 2023 20:20
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3877434550

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 84.113%

Totals Coverage Status
Change from base Build 3872384385: 0.007%
Covered Lines: 5787
Relevant Lines: 6880

💛 - Coveralls

@akellbl4
Copy link
Collaborator

akellbl4 commented Jan 9, 2023

Are we gonna throw an error in case link provided without protocol? Like [google](google.com). Because it results in a link that refers to ${currentUrl}/google.com

@paskal paskal changed the title Don't allow links with URL unchanged, e.g. [text](url) Don't allow relative links in comments Jan 9, 2023
@paskal paskal marked this pull request as draft January 9, 2023 22:41
@paskal
Copy link
Sponsor Collaborator Author

paskal commented Jan 9, 2023

This change prohibits posting relative links. I have to think about how to parse markdown correctly, but the change is already working and can be tested. @umputun, what do you think about the whole idea of blocking the relative URLs?

@umputun
Copy link
Owner

umputun commented Jan 13, 2023

Using relative links intentionally is an extremely rare use case. One would need to have a file server on the same subdomain as Remark42 to make any sense of this. I believe disabling such links is a good idea and will work well in 99.9% of cases.

@paskal paskal force-pushed the paskal/comment_validation branch 4 times, most recently from 01c4723 to 270ff85 Compare April 1, 2023 18:58
@paskal paskal marked this pull request as ready for review April 1, 2023 19:28
@paskal paskal force-pushed the paskal/comment_validation branch from 270ff85 to f9e1e18 Compare April 1, 2023 19:51
(url) is a text inserted by default and never an intended URL.

That additional validation will ensure that users won't post relative
links because they are rarely intended.
@paskal paskal force-pushed the paskal/comment_validation branch from f9e1e18 to 6500548 Compare April 2, 2023 19:20
Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

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

LGTM

@umputun umputun merged commit 26f82ad into master Apr 10, 2023
@umputun umputun deleted the paskal/comment_validation branch April 10, 2023 04:30
@paskal paskal added this to the v1.12.0 milestone Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants