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

Revert "Adding url sanitisation for extra links" #41993

Closed
wants to merge 1 commit into from

Conversation

utkarsharma2
Copy link
Contributor

@utkarsharma2 utkarsharma2 commented Sep 4, 2024

Reverts #41665

In the original PR, we considered absolute paths, but we also need to account for relative paths, as this is leading to issue. I tried to explore the possible option to extend the check to relative paths but couldn't find any good options as they can vary a lot(especially strings like page.html)

Possible relative paths

Relative to Current Directory:

page.html – Points to a file page.html in the current directory.
images/logo.png – Points to an images folder and the logo.png file within the current directory.

Relative to Parent Directory:

../about.html – Points to about.html in the parent directory.
../assets/styles.css – Points to styles.css in the assets folder in the parent directory.

Relative to the Root Directory:

/index.html – Points to index.html in the root directory of the site.
/css/styles.css – Points to the styles.css file in the css folder within the root directory.

Relative to the Current Path:

./file.html – Points to file.html in the current directory (same as file.html).
./folder/page.html – Points to page.html in the folder within the current directory.

Relative URL with Anchor (Fragment Identifier):

#section1 – Points to the element with the id="section1" within the current page.
page.html#header – Points to the #header within the page.html.

Relative URL with Query String:

search.html?q=term – Points to search.html with a query parameter q=term.
./view.php?id=123 – Points to view.php in the current directory with a query parameter id=123.

cc: @amoghrajesh @potiuk

@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Sep 4, 2024
@potiuk
Copy link
Member

potiuk commented Sep 4, 2024

I think we should fix it, not revert

@potiuk
Copy link
Member

potiuk commented Sep 4, 2024

The right sanitization should be:

  • https:// - like now

or

no schema

@ephraimbuddy
Copy link
Contributor

I think we should fix it, not revert

Also, I think we shouldn't have rc2 because of this. WDYT?

@potiuk
Copy link
Member

potiuk commented Sep 4, 2024

I think we should fix it, not revert

Also, I think we shouldn't have rc2 because of this. WDYT?

Agreed,

@utkarsharma2
Copy link
Contributor Author

Ya, I too think this is not a blocker for RC1.

Closing it in favor of PR - #41995

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants