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

ASF-ui abuses /Api/WWW/Send where it could use more appropriate API instead #1319

Closed
JustArchi opened this issue Mar 8, 2021 · 3 comments · Fixed by #1323
Closed

ASF-ui abuses /Api/WWW/Send where it could use more appropriate API instead #1319

JustArchi opened this issue Mar 8, 2021 · 3 comments · Fixed by #1323
Assignees
Labels
🐛 Bug Issues marked with this label indicate unintended program behaviour that needs correction. ✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 🔴 High priority Issues marked with this label indicate the most serious problems, especially security-related.

Comments

@JustArchi
Copy link
Member

JustArchi commented Mar 8, 2021

Description

In ui.js we have such function:

export async function isReleaseAvailable() {
  const lastChecked = get('last-checked-for-update');
  if (lastChecked && (lastChecked > (Date.now() - 60 * 60 * 1000))) {
    const latestCachedVersion = get('latest-release');
    return (latestCachedVersion > asf.version);
  }

  const updateChannel = (asf.updateChannel === UPDATECHANNEL.EXPERIMENTAL) ? 'releases' : 'releases/latest';
  const response = await http.post('www/send', { url: `https://api.github.com/repos/JustArchiNET/ArchiSteamFarm/${updateChannel}` });
  set('last-checked-for-update', Date.now());

  const latestRelease = JSON.parse(response);
  if (!latestRelease) return false;

  const latestVersion = (asf.updateChannel === UPDATECHANNEL.EXPERIMENTAL) ? latestRelease[0].tag_name : latestRelease.tag_name;
  set('latest-release', latestVersion);

  return (latestVersion > asf.version);
}

This abuses the internal API, as Send was intended to be used only for fetching resources that are not possible to reach through more appropriate endpoints.

Expected behavior

ASF exposes two nice endpoints to fetch ASF release:

image

The above function should be rewritten and call for /Api/WWW/GitHub/Release when asf.updateChannel === UPDATECHANNEL.EXPERIMENTAL and /Api/WWW/GitHub/Release/latest otherwise.

/Api/WWW/GitHub/Release:

{
  "Result": {
    "ChangelogHTML": "<p>Changes since <strong><a href=\"https://github.com/JustArchi/ArchiSteamFarm/releases/tag/5.0.4.3\">V5.0.4.3</a></strong>:</p>\n<ul>\n<li>Our <code>generic</code> variant of ASF no longer requires the very latest runtime to run, if it's not mandatory for operation (#2219).</li>\n<li>Rewritten <code>SteamTarget</code> NLog logging target for full <code>async</code> support. This is crucial e.g. for logging crashes through <code>SteamTarget</code>.</li>\n<li>Latest ASF-ui with new features, improvements and bugfixes.</li>\n<li>Updated <strong><a href=\"https://github.com/JustArchiNET/ArchiSteamFarm/wiki/Localization\">localization</a></strong> provided by our community.</li>\n<li>Usual amount of other core improvements, optimizations and bugfixes.</li>\n</ul>\n",
    "ReleasedAt": "2021-03-04T16:53:23Z",
    "Stable": false,
    "Version": "5.0.5.0"
  },
  "Message": "OK",
  "Success": true
}

/Api/WWW/GitHub/Release/latest:

{
  "Result": {
    "ChangelogHTML": "<p>Changes since <strong><a href=\"https://github.com/JustArchi/ArchiSteamFarm/releases/tag/5.0.3.2\">V5.0.3.2</a></strong>:</p>\n<ul>\n<li>Added <code>mab</code>, <code>mabadd</code> and <code>mabrm</code> commands (#2183).</li>\n<li>Further decreased chance of Steam PICS restarts to cause idling temporarily unavailable.</li>\n<li>Misc corrections in regards to error-handling of our STD plugin.</li>\n<li>Latest ASF-ui with new features, improvements and bugfixes.</li>\n<li>Updated <strong><a href=\"https://github.com/JustArchiNET/ArchiSteamFarm/wiki/Localization\">localization</a></strong> provided by our community.</li>\n<li>Usual amount of other core improvements, optimizations and bugfixes.</li>\n</ul>\n",
    "ReleasedAt": "2021-02-27T13:22:56Z",
    "Stable": true,
    "Version": "5.0.4.3"
  },
  "Message": "OK",
  "Success": true
}

Current behavior

ASF-ui calls the GitHub itself, which apart from looking awful and being much slower, is prone to breaking changes, not just the ones caused by GitHub, but also the fact that ASF repo could change URL in the future, making this whole call broken.

Additional information

Im evaluating whether I can implement the wiki requirements of ASF-ui in ASF itself and therefore remove the Send endpoint entirely, as it has a potential for broad abuse that I didn't consider before. The change mentioned here is something that should be done regardless of Send removal decision - I'm pointing it out to give this issue further pressure than low-priority wishlist.

@JustArchi JustArchi added 🐛 Bug Issues marked with this label indicate unintended program behaviour that needs correction. ✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 🟡 Medium priority Issues marked with this label have a priority, unless there is something even more important. labels Mar 8, 2021
@JustArchi
Copy link
Member Author

JustArchi commented Mar 8, 2021

Same goes for fetchWiki.js which makes it even worse, as it fetches last 20 releases and "hopes" to find the appropriate one, instead of fetching the version that the user has.

For working around the problem of user running version newer than last released, it's enough to check it before.

Pseudocode:

var currentVersion = GetCurrentASFVersion();
var targetRelease = GetURL("/Api/WWW/GitHub/Release");

if (currentVersion < targetRelease.Result.Version) {
    targetRelease = GetURL("/Api/WWW/GitHub/Release/" + currentVersion);
}

return targetRelease.Result.ReleasedAt;

Will work with any version that user might have.

@JustArchi
Copy link
Member Author

Raising the priority due to deprecation of /Api/WWW/Send in ASF:

  • ASF release with deprecated usage of Send will arrive in approx 1 month.
  • ASF release with removed usage of Send will arrive in approx 2 months.

This means this issue should be handled within around 60 days to avoid broken usage.

@JustArchi JustArchi added 🔴 High priority Issues marked with this label indicate the most serious problems, especially security-related. and removed 🟡 Medium priority Issues marked with this label have a priority, unless there is something even more important. labels Mar 8, 2021
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐛 Bug Issues marked with this label indicate unintended program behaviour that needs correction. ✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 🔴 High priority Issues marked with this label indicate the most serious problems, especially security-related.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants