From bc6a0632de88a3ec90be465dce164f6e21c816bc Mon Sep 17 00:00:00 2001 From: Strencher <46447572+Strencher@users.noreply.github.com> Date: Mon, 1 May 2023 21:36:56 +0200 Subject: [PATCH] Code cleanup, deprecation notice, add maxRedirects --- preload/src/api/fetch.js | 56 +++++++++++++++++++++++++++---- renderer/src/modules/api/fetch.js | 36 ++++++++++++++++---- renderer/src/modules/api/index.js | 7 ++-- renderer/src/polyfill/index.js | 12 ++++++- 4 files changed, 93 insertions(+), 18 deletions(-) diff --git a/preload/src/api/fetch.js b/preload/src/api/fetch.js index f423b1df..b9f105f6 100644 --- a/preload/src/api/fetch.js +++ b/preload/src/api/fetch.js @@ -1,8 +1,23 @@ import * as https from "https"; import * as http from "http"; +const MAX_DEFAULT_REDIRECTS = 20; const redirectCodes = new Set([301, 302, 307, 308]); +/** + * @typedef {Object} FetchOptions + * @property {"GET" | "PUT" | "POST" | "DELETE"} [method] - Request method. + * @property {Record} [headers] - Request headers. + * @property {"manual" | "follow"} [redirect] - Whether to follow redirects. + * @property {number} [maxRedirects] - Maximum amount of redirects to be followed. + * @property {AbortSignal} [signal] - Signal to abruptly cancel the request + * @property {Uint8Array | string} [body] - Defines a request body. Data must be serializable. + */ + +/** + * @param {string} url + * @param {FetchOptions} options + */ export function nativeFetch(url, options) { let state = "PENDING"; const data = {content: [], headers: null, statusCode: null, url: url, statusText: "", redirected: false}; @@ -10,7 +25,7 @@ export function nativeFetch(url, options) { const errors = new Set(); /** * @param {URL} url */ - const execute = (url, options, redirect = false) => { + const execute = (url, options, redirectCount = 0) => { const Module = url.protocol === "http" ? http : https; const req = Module.request(url.href, { @@ -18,13 +33,31 @@ export function nativeFetch(url, options) { method: options.method ?? "GET" }, res => { if (redirectCodes.has(res.statusCode) && res.headers.location && options.redirect !== "manual") { - const final = new URL(res.headers.location); + redirectCount++; + + if (redirectCount >= (options.maxRedirects ?? MAX_DEFAULT_REDIRECTS)) { + state = "ABORTED"; + const error = new Error(`Maximum amount of redirects reached (${options.maxRedirects ?? MAX_DEFAULT_REDIRECTS})`); + errors.forEach(e => e(error)); + + return; + } + + let final; + try { + final = new URL(res.headers.location); + } + catch (error) { + state = "ABORTED"; + errors.forEach(e => e(error)); + return; + } for (const [key, value] of new URL(url).searchParams.entries()) { final.searchParams.set(key, value); } - return execute(final, options, true); + return execute(final, options, redirectCount); } res.on("data", chunk => data.content.push(chunk)); @@ -34,7 +67,7 @@ export function nativeFetch(url, options) { data.statusCode = res.statusCode; data.url = url.toString(); data.statusText = res.statusMessage; - data.redirected = redirect; + data.redirected = redirectCount > 0; state = "DONE"; listeners.forEach(listener => listener()); @@ -50,10 +83,12 @@ export function nativeFetch(url, options) { catch (error) { state = "ABORTED"; errors.forEach(e => e(error)); - } finally { + } + finally { req.end(); } - } else { + } + else { req.end(); } @@ -65,7 +100,14 @@ export function nativeFetch(url, options) { } }; - execute(new URL(url), options); + try { + const parsed = new URL(url); + execute(parsed, options); + } + catch (error) { + state = "ABORTED"; + errors.forEach(e => e(error)); + } return { onComplete(listener) { diff --git a/renderer/src/modules/api/fetch.js b/renderer/src/modules/api/fetch.js index e9a3efb5..1d541395 100644 --- a/renderer/src/modules/api/fetch.js +++ b/renderer/src/modules/api/fetch.js @@ -1,5 +1,7 @@ import Remote from "../../polyfill/remote"; +const methods = new Set(["GET" | "PUT" | "POST" | "DELETE"]); + class FetchResponse extends Response { constructor(options) { super(options.content, { @@ -30,14 +32,34 @@ const convertSignal = signal => { }; }; -export function fetch(url, options = {}) { +/** + * @typedef {Object} FetchOptions + * @property {"GET" | "PUT" | "POST" | "DELETE"} [method] - Request method. + * @property {Record} [headers] - Request headers. + * @property {"manual" | "follow"} [redirect] - Whether to follow redirects. + * @property {number} [maxRedirects] - Maximum amount of redirects to be followed. + * @property {AbortSignal} [signal] - Signal to abruptly cancel the request + * @property {Uint8Array | string} [body] - Defines a request body. Data must be serializable. + */ + +/** + * @param {string} url + * @param {FetchOptions} options + * @returns {Promise} + */ +export default function fetch(url, options = {}) { return new Promise((resolve, reject) => { - const ctx = Remote.nativeFetch(url, { - ...(options.headers && {headers: options.headers instanceof Headers ? Object.fromEntries(options.headers.entries()) : options.headers}), - ...(options.body && {body: options.body}), - ...(options.method && {method: options.method}), - ...(options.signal && {signal: convertSignal(options.signal)}) - }); + const data = {}; + + if (typeof options.headers === "object") { + data.headers = options.headers instanceof Headers ? Object.fromEntries(options.headers.entries()) : options.headers; + } + + if (typeof options.body === "string" || options.body instanceof Uint8Array) data.body = options.body; + if (typeof options.method === "string" && methods.has(options.method)) data.method = options.method; + if (options.signal instanceof AbortSignal) data.signal = convertSignal(options.signal); + + const ctx = Remote.nativeFetch(url, data); ctx.onError(error => { reject(error); diff --git a/renderer/src/modules/api/index.js b/renderer/src/modules/api/index.js index cab1f18f..a30dba41 100644 --- a/renderer/src/modules/api/index.js +++ b/renderer/src/modules/api/index.js @@ -12,7 +12,7 @@ import Utils from "./utils"; import Webpack from "./webpack"; import * as Legacy from "./legacy"; import ContextMenu from "./contextmenu"; -import {fetch} from "./fetch"; +import fetch from "./fetch"; import {DiscordModules} from "modules"; const bounded = new Map(); @@ -58,7 +58,7 @@ export default class BdApi { Components = { get Tooltip() {return DiscordModules.Tooltip;} } - fetch = fetch; + Net = {fetch}; } // Add legacy functions @@ -128,8 +128,9 @@ BdApi.Components = { get Tooltip() {return DiscordModules.Tooltip;} }; -BdApi.fetch = fetch; +BdApi.Net = {fetch}; Object.freeze(BdApi); +Object.freeze(BdApi.Net); Object.freeze(BdApi.prototype); Object.freeze(BdApi.Components); diff --git a/renderer/src/polyfill/index.js b/renderer/src/polyfill/index.js index 72434088..0c375dd2 100644 --- a/renderer/src/polyfill/index.js +++ b/renderer/src/polyfill/index.js @@ -7,6 +7,12 @@ import * as https from "./https"; import Buffer from "./buffer"; import crypto from "./crypto"; import Remote from "./remote"; +import Logger from "common/logger"; + +const deprecated = new Map([ + ["request", "Use BdApi.Net.fetch instead."], + ["https", "Use BdApi.Net.fetch instead."], +]); const originalFs = Object.assign({}, fs); originalFs.writeFileSync = (path, data, options) => fs.writeFileSync(path, data, Object.assign({}, options, {originalFs: true})); @@ -14,6 +20,10 @@ originalFs.writeFile = (path, data, options) => fs.writeFile(path, data, Object. export const createRequire = function (path) { return mod => { + if (deprecated.has(mod)) { + Logger.warn("Remote~Require", `The "${mod}" module is marked as deprecated. ${deprecated.get(mod)}`); + } + switch (mod) { case "request": return request; case "https": return https; @@ -42,4 +52,4 @@ require.resolve = (path) => { } }; -export default require; \ No newline at end of file +export default require;