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 validating the URL of user home sites #355

Closed

Conversation

nud
Copy link
Contributor

@nud nud commented Jan 6, 2023

Home site validation used the URL regexp, but didn't anchor it. Because of this it was possible to add text before or after the URL and have it considered valid. For instance, this was considered a valid homesite value: "lorem ipsum https://example.org/#foo#bar dolor sit amet"

It is possible to fix it by anchoring the regexp, but IMO it is wiser to use the same URL validator used at other places in the site. Hence this commit replaces this validation with the http_url validator used for news links and bookmarks.

https://linuxfr.org/suivi/impossible-de-mettre-un-lien-vers-un-salon-matrix-dans-les-liens-d-une-depeche#comment-1911550

NB: it doesn't fix the problem in the suivi entry but another problem explained in the comment.

Home site validation used the URL regexp, but didn't anchor it. Because
of this it was possible to add text before or after the URL and have it
considered valid. For instance, this was considered a valid homesite
value: "lorem ipsum https://example.org/#foo#bar dolor sit amet"

It is possible to fix it by anchoring the regexp, but IMO it is wiser to
use the same URL validator used at other places in the site. Hence this
commit replaces this validation with the `http_url` validator used for
news links and bookmarks.

https://linuxfr.org/suivi/impossible-de-mettre-un-lien-vers-un-salon-matrix-dans-les-liens-d-une-depeche#comment-1911550
@nud nud force-pushed the suivi-2047-url-validation-inconsistency branch from ea3d8e4 to 25aa7d1 Compare January 6, 2023 13:47
@Trim
Copy link
Member

Trim commented Jan 6, 2023

Thanks it's a good idea to use the HTTP validator here too 👍

@Trim
Copy link
Member

Trim commented Jan 28, 2023

I'm closing this PR in favor of #356 which contains the commit posted here plus the fix for Matrix links. Thanks !

@Trim Trim closed this Jan 28, 2023
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.

2 participants