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

🐛 Remove slash duplication in user profile URLs #3009

Conversation

botamochi0x12
Copy link
Contributor

Summary

Fixes slash duplication in user profile URLs.

https://itch.io/profile/botamochi0x12

Cause & Impact

The bug arises from a double slash in the path, resulting in URLs like https://itch.io//profile/the-user-name.
The constant originalItchio includes the trailing slash.

image -- from /issues/2975

Considerations

As mentioned in the issue #2975, other paths may also be affected. Introducing a package to construct URLs from bare strings could address this issue more comprehensively.

Fixed slash duplication in the path component to every user profile 
by removing trailing slash inside the constant `originalItchio`.
The bug is caused by a double slash in the path, for example, `https://itch.io//profile/the-user-name`.
@botamochi0x12
Copy link
Contributor Author

Hello @leafo,

I’ve submitted the pull request about "slash duplication in user profile URLs" on itchio/itch. Your review would be greatly appreciated whenever you have the time.

Thank you!

@3ter
Copy link

3ter commented May 15, 2024

Thanks for the contribution @botamochi0x12 ! I don't see any negative side effects of the changes. If I could review, I would provide a proper one.

The constant in question is only used in a handful of places:

src\main\butlerd\make-butler-instance.ts:
  42      "--address",
  43:     urls.itchio,
  44      "--user-agent",

https://itch.io is a valid URL for butler to recognize (I hope... it would be quite surprising if it needed the /).

src\main\reactors\login.ts:
   66                    widgetParams: {
   67:                     url: recaptchaUrl || urls.itchio + "/captcha",
   68                    },

  176      const epoch = Date.now() * 0.001;
  177:     const parsed = urlParser.parse(urls.itchio);
  178      const opts = {

Added slash for subdirectory and parsed URL doesn't need slashes at the end.

src\main\reactors\url.ts:
  66          wind: "root",
  67:         url: `${urls.itchio}/profile/${slug}`,
  68        })

Added slash for subdirectory.

src\main\util\paths.ts:
  13    let usersPath = join(app.getPath("userData"), "users");
  14:   if (urls.itchio !== urls.originalItchio) {
  15:     usersPath = join(usersPath, fsFriendlyHost(urls.itchio));
  16    }

  31    if (process.env.WHEN_IN_ROME) {
  32:     const parsed = urlParser.parse(urls.itchio);
  33      dbName = `butler-${parsed.host.replace(/^[A_Za-z\._\-]/g, "_")}.db`;

Comparison with itself and parsed URL doesn't need slashes at the end.

src\renderer\App\Layout\NonLocalIndicator.tsx:
  22    render() {
  23:     if (urls.itchio === urls.originalItchio) {
  24        return null;

  28        <IndicatorDiv title="itch is running off a private itch.io instance">
  29:         {urls.itchio}
  30        </IndicatorDiv>

Comparison with itself and display only.

src\renderer\pages\FeaturedPage.tsx:
  12        replace: true,
  13:       url: urls.itchio,
  14      });

Fetch from URL doesn't need slash.

src\renderer\pages\BrowserPage\newTabItems.ts:
  29      icon: "shuffle",
  30:     url: urls.itchio + "/randomizer",
  31    },

  34      icon: "shopping_cart",
  35:     url: urls.itchio + "/games/on-sale",
  36    },

  39      icon: "star",
  40:     url: urls.itchio + "/games/top-sellers",
  41    },

  44      icon: "fire",
  45:     url: urls.itchio + "/devlogs",
  46    },

Added slash for subdirectory.

src\renderer\pages\CollectionPage\index.tsx:
  125      // we don't know the slug, the website will redirect to the proper one
  126:     let url = `${urls.itchio}/c/${collectionId}/hello`;
  127      dispatch(actions.openInExternalBrowser({ url }));

Added slash for subdirectory.

src\renderer\scenes\HubScene\Sidebar\SearchResultsBar\index.tsx:
  80          wind: ambientWind(),
  81:         url: `${urls.itchio}?${encodeURIComponent(this.props.query)}`,
  82        })

Query parameters starting after ? shouldn't have a slash in front of them.

@botamochi0x12
Copy link
Contributor Author

Woa thanks for your fast reply @3ter !

If I could review, I would provide a proper one.

I'm glad to hear that you would review my PR.
May I ask you to provide a review?

@3ter
Copy link

3ter commented May 16, 2024

@botamochi0x12 But I can't 😉 . I don't have the rights to approve a PR as I'm no member of this repository.

I tried to get invested but for me the hurdle was too high (a mix of various factors). I just saw this small change and thought this is small enough to help a little with.

Pinging the maintainer was the right thing to do. @alts is also very helpful and I suppose his review could actually be of weight 😃.

@botamochi0x12
Copy link
Contributor Author

Thank you for your advice, @3ter, and I'm sorry for the delayed reply.
Your first comment has been very helpful to me.
I agree that pinging the maintainer was the right course of action, especially (considering the relatively low activity in the repository). I'll patiently await the maintainer's response 😉

@alts
Copy link
Collaborator

alts commented Jun 14, 2024

Thanks for the PR, and sorry for the long wait! This looks great

@alts alts merged commit f91e12a into itchio:master Jun 14, 2024
@botamochi0x12 botamochi0x12 deleted the bugfix/removing-duplicated-slash-in-path-to-user-profile branch July 3, 2024 14:55
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.

3 participants