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 sanitizeURL #599

Merged
merged 1 commit into from
Jun 10, 2020
Merged

Fix sanitizeURL #599

merged 1 commit into from
Jun 10, 2020

Conversation

peolic
Copy link
Contributor

@peolic peolic commented Jun 6, 2020

Fixes this bug:
Sanitized URLs can become malformed - missing // after the protocol

Details

image

Code I tested this with:

Details
const twitterURL = new URL("https://www.twitter.com");
const instagramURL = new URL("https://www.instagram.com");

const sanitiseURL = (url, siteURL) => {
  if (!url) {
    return url;
  }

  if (url.startsWith("http://") || url.startsWith("https://")) {
    // just return the entire URL
    return url;
  }

  if (siteURL) {
    // if url starts with the site host, then prepend the protocol
    if (url.startsWith(siteURL.host)) {
      return `${siteURL.protocol}//${url}`;
    }

    // otherwise, construct the url from the protocol, host and passed url
    return `${siteURL.protocol}//${siteURL.host}/${url}`;
  }

  // just prepend the protocol - assume https
  return `https://${url}`;
};

const test = (url1, url2) => console.log(sanitiseURL(url1, url2));

test('mytest', twitterURL);
test('www.twitter.com/mytest', twitterURL);
test('http://www.twitter.com/mytest?u=e123', twitterURL);
test('https://twitter.com/mytest?u=e123', twitterURL);
test('twitter.com/mytest?');

test('mytest', instagramURL);
test('www.instagram.com/mytest', instagramURL);
test('http://www.instagram.com/mytest?u=e123', instagramURL);
test('https://instagram.com/mytest?u=e123', instagramURL);
test('instagram.com/mytest?');

@WithoutPants WithoutPants added the bug Something isn't working label Jun 7, 2020
@WithoutPants WithoutPants added this to the Version 0.2.1 milestone Jun 9, 2020
@WithoutPants WithoutPants changed the base branch from develop to master June 9, 2020 07:09
Copy link
Collaborator

@WithoutPants WithoutPants left a comment

Choose a reason for hiding this comment

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

I've changed the base to master so that I can patch it into 0.2 directly.

Thanks for the fix. I was scratching my head for a bit trying to work out what the original problem was. Turns out that Chrome must fix up the malformed URL automatically. I had to jump to a different browser to see the bug.

@WithoutPants WithoutPants merged commit ef8cba8 into stashapp:master Jun 10, 2020
@peolic peolic deleted the fix-sanitize-url branch June 10, 2020 12:52
Tweeticoats pushed a commit to Tweeticoats/stash that referenced this pull request Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants