Skip to content

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

@potiuk potiuk deleted the revert-41665-sanitizeExtraLinks branch October 2, 2024 21:44
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