itchio / itch

🎮 The best way to play your itch.io games
https://itch.io/app
MIT License
2.36k stars 210 forks source link

:bug: Remove slash duplication in user profile URLs #3009

Closed botamochi0x12 closed 3 months ago

botamochi0x12 commented 4 months ago

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 itchio/itch/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.

botamochi0x12 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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 commented 3 months ago

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