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

NewEgg #83

Closed
TPS opened this issue Oct 16, 2022 · 2 comments · Fixed by #127
Closed

NewEgg #83

TPS opened this issue Oct 16, 2022 · 2 comments · Fixed by #127
Labels
help wanted Extra attention is needed

Comments

@TPS
Copy link

TPS commented Oct 16, 2022

Similar to #18's Amazon example, when 1 has

  • https://www.newegg.com/black-acer-nitro-5-an515-57-59f7-gaming/p/N82E16834360174?Item=N82E16834360174

all 1 needs is

  • https://www.newegg.com/p/N82E16834360174
@svenjacobs svenjacobs added the help wanted Extra attention is needed label Oct 16, 2022
@aricooperdavis
Copy link
Contributor

I was in the process of addressing this, but on further inspection I'm not sure it's quite right for Léon.

The first URL isn't a tracking URL, it is actually the correct address for the product, whilst the second shorter URL just redirects to the first. I'm generally anti URL shortening services as they obscure the true address being shared, which can be concerning from a security perspective. It also requires a second network request to actually reach the resource that you're trying to share. As such, by automatically shortening these URLs, Léon would be reducing user security and increasing user wait-times without increasing user privacy.

@svenjacobs
Copy link
Owner

Thanks for your thoughts @aricooperdavis.

We're already doing this for Amazon links, for example. Most of the information in an Amazon or NewEgg product link is probably for SEO only and the URL can be shortened significantly. However, as you have pointed out, NewEgg performs an HTTP redirect to the long URL. Amazon doesn't do this.

The purpose of Leon is to remove tracking as well as redundant information from URLs. We also want to keep URLs short for Tweets or Toots for example without resorting to (alternative) short URLs (for instance youtu.be instead of youtube.com).

An additional HTTP redirect is unfortunate but we don't know whether NewEgg (or Amazon) will ever change this so this shouldn't be an argument against this kind of sanitizer?

Also please don't forget that if users don't want this kind of cleanup for a newegg.com URL, they can just disable this sanitizer.

svenjacobs pushed a commit that referenced this issue Nov 8, 2022
feat: Add support for NewEgg links, fix #83

Fix linting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants