From a962f3dfecee3a129879ccdb52c91330d9c70c7b Mon Sep 17 00:00:00 2001 From: zkldi Date: Fri, 23 Jul 2021 19:10:32 +0100 Subject: [PATCH] Fix critical accident in RequestLoggerMiddleware --- server/package.json | 2 +- server/pnpm-lock.yaml | 8 ++--- .../scripts/init-folder-cache/folder-cache.ts | 2 +- server/src/lib/logger/logger.test.ts | 21 +++++++++++- server/src/lib/logger/logger.ts | 15 +++++++- server/src/lib/ugpt-stat/get-related.ts | 34 +++++++++++++++++++ server/src/lib/ugpt-stat/get-stats.test.ts | 17 ++++++++++ server/src/lib/ugpt-stat/get-stats.ts | 16 ++++++--- .../src/server/middleware/request-logger.ts | 25 ++++++++++---- .../_game/_playtype/charts/_chartID/router.ts | 3 +- .../games/_game/_playtype/stats/router.ts | 6 +++- server/src/server/server.ts | 4 +-- server/src/utils/misc.ts | 22 ------------ server/tsconfig.build.json | 12 +++---- server/tsconfig.json | 2 +- 15 files changed, 138 insertions(+), 51 deletions(-) create mode 100644 server/src/lib/ugpt-stat/get-related.ts diff --git a/server/package.json b/server/package.json index aeb0b77e3..83cbd2f9f 100644 --- a/server/package.json +++ b/server/package.json @@ -8,7 +8,7 @@ "watchtest": "tap --watch", "build": "tsc --project tsconfig.build.json", "lint": "eslint ./src --ext .ts --fix", - "start": "tsc --project tsconfig.build.json && node js/main.js" + "start": "tsc --project tsconfig.build.json && export NODE_PATH=js/ && node js/main.js" }, "author": "zkldi", "license": "AGPL3", diff --git a/server/pnpm-lock.yaml b/server/pnpm-lock.yaml index 81d7374f5..2a55a78f5 100644 --- a/server/pnpm-lock.yaml +++ b/server/pnpm-lock.yaml @@ -81,7 +81,7 @@ dependencies: rate-limit-redis: 2.1.0 redis: 3.1.2 rimraf: 3.0.2 - tachi-common: github.com/zkldi/tachi-common/362c3375c5f11b998aaedc34e4f14b62873b2ac7_ts-node@10.0.0+typescript@4.3.4 + tachi-common: github.com/zkldi/tachi-common/62ea387173a0ee7019adf3371b90f6328081ae92_ts-node@10.0.0+typescript@4.3.4 typescript: 4.3.4 winston: 3.3.3 @@ -4214,9 +4214,9 @@ packages: engines: {node: '>=6'} dev: true - github.com/zkldi/tachi-common/362c3375c5f11b998aaedc34e4f14b62873b2ac7_ts-node@10.0.0+typescript@4.3.4: - resolution: {tarball: https://codeload.github.com/zkldi/tachi-common/tar.gz/362c3375c5f11b998aaedc34e4f14b62873b2ac7} - id: github.com/zkldi/tachi-common/362c3375c5f11b998aaedc34e4f14b62873b2ac7 + github.com/zkldi/tachi-common/62ea387173a0ee7019adf3371b90f6328081ae92_ts-node@10.0.0+typescript@4.3.4: + resolution: {tarball: https://codeload.github.com/zkldi/tachi-common/tar.gz/62ea387173a0ee7019adf3371b90f6328081ae92} + id: github.com/zkldi/tachi-common/62ea387173a0ee7019adf3371b90f6328081ae92 name: tachi-common version: 0.1.0 dependencies: diff --git a/server/scripts/init-folder-cache/folder-cache.ts b/server/scripts/init-folder-cache/folder-cache.ts index b3da18a9e..8f4a3348a 100644 --- a/server/scripts/init-folder-cache/folder-cache.ts +++ b/server/scripts/init-folder-cache/folder-cache.ts @@ -1,3 +1,3 @@ -import { InitaliseFolderChartLookup } from "../../src/common/folder"; +import { InitaliseFolderChartLookup } from "../../src/utils/folder"; InitaliseFolderChartLookup(); diff --git a/server/src/lib/logger/logger.test.ts b/server/src/lib/logger/logger.test.ts index 5903430e5..fb00941e2 100644 --- a/server/src/lib/logger/logger.test.ts +++ b/server/src/lib/logger/logger.test.ts @@ -1,6 +1,6 @@ import t from "tap"; import { CloseAllConnections } from "../../test-utils/close-connections"; -import CreateLogCtx, { Transports } from "./logger"; +import CreateLogCtx, { ChangeRootLogLevel, rootLogger, Transports } from "./logger"; t.test("Logger Tests", (t) => { const logger = CreateLogCtx(__filename); @@ -22,4 +22,23 @@ t.test("Logger Tests", (t) => { t.end(); }); +t.test("#ChangeRootLogLevel", (t) => { + t.test("Should work with the root logger.", (t) => { + rootLogger.info("The below message should appear."); + + ChangeRootLogLevel("verbose"); + rootLogger.verbose("== SHOULD APPEAR =="); + + ChangeRootLogLevel("info"); + + rootLogger.info("The below message SHOULD NOT appear."); + + rootLogger.verbose("== SHOULD NOT APPEAR =="); + + t.end(); + }); + + t.end(); +}); + t.teardown(CloseAllConnections); diff --git a/server/src/lib/logger/logger.ts b/server/src/lib/logger/logger.ts index 16a2d6936..89b69ae78 100644 --- a/server/src/lib/logger/logger.ts +++ b/server/src/lib/logger/logger.ts @@ -97,7 +97,10 @@ const consoleFormatRoute = format.combine( }) ); -let tports = []; +let tports: ( + | winston.transports.ConsoleTransportInstance + | winston.transports.FileTransportInstance +)[] = []; /* istanbul ignore next */ if (IN_TESTING) { @@ -155,6 +158,16 @@ export function AppendLogCtx(context: string, lg: KtLogger): KtLogger { return lg.child({ context: newContext }) as KtLogger; } +export function ChangeRootLogLevel( + level: "crit" | "severe" | "error" | "warn" | "info" | "verbose" | "debug" +) { + rootLogger.info(`Changing log level to ${level}.`); + + for (const tp of tports) { + tp.level = level; + } +} + export const Transports = tports; export default CreateLogCtx; diff --git a/server/src/lib/ugpt-stat/get-related.ts b/server/src/lib/ugpt-stat/get-related.ts new file mode 100644 index 000000000..c27cae4b4 --- /dev/null +++ b/server/src/lib/ugpt-stat/get-related.ts @@ -0,0 +1,34 @@ +import db from "external/mongo/db"; +import CreateLogCtx from "lib/logger/logger"; +import { Game, UGPTStatDetails } from "tachi-common"; + +const logger = CreateLogCtx(__filename); + +export async function GetRelatedStatDocuments(stat: UGPTStatDetails, game: Game) { + if (stat.mode === "chart") { + const chart = await db.charts[game].findOne({ chartID: stat.chartID }); + + if (!chart) { + logger.error(`This stat refers to a chart that does not exist?`, { stat }); + throw new Error(`Stat refers to a chart that does not exist? ${stat.chartID}.`); + } + + const song = await db.songs[game].findOne({ id: chart?.songID }); + + if (!song) { + logger.severe(`Song-Chart Mismatch - ${chart.songID}.`, { chart }); + throw new Error(`Song-Chart Mismatch on ${chart.songID}.`); + } + + return { song, chart }; + } else if (stat.mode === "folder") { + const folders = await db.folders.find({ + folderID: { $in: Array.isArray(stat.folderID) ? stat.folderID : [stat.folderID] }, + }); + + return { folders }; + } + + logger.error(`Invalid stat - has nonsense stat.mode.`, { stat }); + throw new Error(`Invalid stat.mode in stat?`); +} diff --git a/server/src/lib/ugpt-stat/get-stats.test.ts b/server/src/lib/ugpt-stat/get-stats.test.ts index 2b2f2b97f..605e08241 100644 --- a/server/src/lib/ugpt-stat/get-stats.test.ts +++ b/server/src/lib/ugpt-stat/get-stats.test.ts @@ -15,6 +15,7 @@ import { CreateFolderChartLookup } from "../../utils/folder"; t.beforeEach(ResetDBState); t.beforeEach(async () => { + await db.folders.insert(TestingIIDXFolderSP10); await CreateFolderChartLookup(TestingIIDXFolderSP10); await db["game-settings"].remove({}); @@ -57,12 +58,28 @@ t.test("#EvalulateUsersGPTStats", (t) => { value: 1, outOf: 1, }, + related: { + folders: [ + { + folderID: TestingIIDXFolderSP10.folderID, + }, + ], + }, }, { stat: { chartID: Testing511SPA.chartID }, value: { value: 1479, }, + related: { + song: { + title: "5.1.1.", + }, + chart: { + difficulty: "ANOTHER", + playtype: "SP", + }, + }, }, ]); diff --git a/server/src/lib/ugpt-stat/get-stats.ts b/server/src/lib/ugpt-stat/get-stats.ts index e7629e9a8..c7598a6f1 100644 --- a/server/src/lib/ugpt-stat/get-stats.ts +++ b/server/src/lib/ugpt-stat/get-stats.ts @@ -1,7 +1,8 @@ import db from "../../external/mongo/db"; -import { integer, Game, Playtypes } from "tachi-common"; +import { integer, Game, Playtypes, UGPTStatDetails } from "tachi-common"; import CreateLogCtx from "../logger/logger"; import { EvaluateUGPTStat } from "./evaluator"; +import { GetRelatedStatDocuments } from "./get-related"; const logger = CreateLogCtx(__filename); @@ -34,10 +35,17 @@ export async function EvaluateUsersGPTStats( } const results = await Promise.all( - settings.preferences.stats.map((details) => - EvaluateUGPTStat(details, userID).then((v) => ({ stat: details, value: v })) - ) + settings.preferences.stats.map((details) => EvaluateStats(details, userID, game)) ); return results; } + +async function EvaluateStats(details: UGPTStatDetails, userID: integer, game: Game) { + const [result, related] = await Promise.all([ + EvaluateUGPTStat(details, userID), + GetRelatedStatDocuments(details, game), + ]); + + return { stat: details, value: result, related }; +} diff --git a/server/src/server/middleware/request-logger.ts b/server/src/server/middleware/request-logger.ts index a98c17893..25419f53f 100644 --- a/server/src/server/middleware/request-logger.ts +++ b/server/src/server/middleware/request-logger.ts @@ -7,27 +7,40 @@ const logger = CreateLogCtx(__filename); // Taken from Jonathan Turnock - This is an *incredibly* nice // solution for post-request express logging! -const ResSendInteceptor = (res: Response, send: Response["send"]) => (content: unknown) => { +const ResJsonInteceptor = (res: Response, json: Response["json"]) => (content: unknown) => { // @ts-expect-error general monkeypatching error res.contentBody = content; - res.send = send; - res.send(content); + res.json = json; + res.json(content); }; export const RequestLoggerMiddleware: RequestHandler = (req, res, next) => { + // I'm not really a fan of this style of omission - but it works. + + // we **KNOW** for certain that there are only two endpoints where a password is + // sent to us - /register and /auth. + // and both of those use `password` as a key. + // still, i don't like it. + + const safeBody = Object.prototype.hasOwnProperty.call(req.body, "password") + ? Object.assign({ password: "[OMITTED]" }, req.body) + : req.body; + logger.debug(`Received request ${req.method} ${req.url}.`, { query: req.query, - body: req.body, + body: safeBody, }); // @ts-expect-error we're doing some wacky monkey patching - res.send = ResSendInteceptor(res, res.send); + res.json = ResJsonInteceptor(res, res.json); res.on("finish", () => { const contents = { - // @ts-expect-error we're doing some monkey patching + // @ts-expect-error we're doing some monkey patching - contentBody is what we're returning. body: res.contentBody, statusCode: res.statusCode, + requestQuery: req.query, + requestBody: safeBody, }; if (res.statusCode < 400) { diff --git a/server/src/server/router/api/v1/games/_game/_playtype/charts/_chartID/router.ts b/server/src/server/router/api/v1/games/_game/_playtype/charts/_chartID/router.ts index 8986f9de0..08569ac55 100644 --- a/server/src/server/router/api/v1/games/_game/_playtype/charts/_chartID/router.ts +++ b/server/src/server/router/api/v1/games/_game/_playtype/charts/_chartID/router.ts @@ -3,11 +3,12 @@ import db from "../../../../../../../../../external/mongo/db"; import { SYMBOL_TachiData } from "../../../../../../../../../lib/constants/tachi"; import CreateLogCtx from "../../../../../../../../../lib/logger/logger"; import { SearchUsersRegExp } from "../../../../../../../../../lib/search/search"; -import { FormatChart, IsString } from "../../../../../../../../../utils/misc"; +import { IsString } from "../../../../../../../../../utils/misc"; import { ParseStrPositiveNonZeroInt } from "../../../../../../../../../utils/string-checks"; import { GetUsersWithIDs } from "../../../../../../../../../utils/user"; import { HandleTierlistIDParam } from "../../folders/middleware"; import { ValidateAndGetChart } from "./middleware"; +import { FormatChart } from "tachi-common"; const logger = CreateLogCtx(__filename); diff --git a/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/stats/router.ts b/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/stats/router.ts index 6e1e3f932..68334ae2f 100644 --- a/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/stats/router.ts +++ b/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/stats/router.ts @@ -9,6 +9,7 @@ import { EvaluateUGPTStat } from "../../../../../../../../../../lib/ugpt-stat/ev import db from "../../../../../../../../../../external/mongo/db"; import { RequirePermissions } from "../../../../../../../../../middleware/auth"; import { RequireAuthedAsUser } from "../../../../middleware"; +import { GetRelatedStatDocuments } from "lib/ugpt-stat/get-related"; const router: Router = Router({ mergeParams: true }); /** @@ -62,6 +63,7 @@ router.get("/", async (req, res) => { */ router.get("/custom", async (req, res) => { const user = req[SYMBOL_TachiData]!.requestedUser!; + const game = req[SYMBOL_TachiData]!.game!; let stat: UGPTStatDetails; @@ -127,10 +129,12 @@ router.get("/custom", async (req, res) => { const result = await EvaluateUGPTStat(stat, user.id); + const related = await GetRelatedStatDocuments(stat, game); + return res.status(200).json({ success: true, description: `Evaluated Stat for ${user.username}`, - body: result, + body: { result, related }, }); }); diff --git a/server/src/server/server.ts b/server/src/server/server.ts index b508e511d..c00c1c5cd 100644 --- a/server/src/server/server.ts +++ b/server/src/server/server.ts @@ -103,6 +103,8 @@ import mainRouter from "./router/router"; import { SYMBOL_TachiAPIAuth } from "../lib/constants/tachi"; import { RequestLoggerMiddleware } from "./middleware/request-logger"; +app.use(RequestLoggerMiddleware); + app.use("/", mainRouter); // The RUN_OWN_CDN option means that our /cdn path has to be hosted by us. In production, @@ -169,6 +171,4 @@ const MAIN_ERR_HANDLER: express.ErrorRequestHandler = (err, req, res, next) => { app.use(MAIN_ERR_HANDLER); -app.use(RequestLoggerMiddleware); - export default app; diff --git a/server/src/utils/misc.ts b/server/src/utils/misc.ts index c35114802..cdc544861 100644 --- a/server/src/utils/misc.ts +++ b/server/src/utils/misc.ts @@ -73,28 +73,6 @@ export function IsString(val: unknown): val is string { return typeof val === "string"; } -export function FormatChart(game: Game, song: AnySongDocument, chart: AnyChartDocument) { - if (game === "bms") { - return song.title; - } - - const gameConfig = GetGameConfig(game); - - let playtypeStr = `${chart.playtype} `; - - if (gameConfig.validPlaytypes.length === 1) { - playtypeStr = ""; - } - - // return the most recent version this chart appeared in if it - // is not primary. - if (!chart.isPrimary) { - return `${song.title} (${playtypeStr}${chart.difficulty} ${chart.versions[0]})`; - } - - return `${song.title} (${playtypeStr}${chart.difficulty})`; -} - export function DedupeArr(arr: T[]): T[] { return [...new Set(arr)]; } diff --git a/server/tsconfig.build.json b/server/tsconfig.build.json index 4cbcac9a5..e62e479af 100644 --- a/server/tsconfig.build.json +++ b/server/tsconfig.build.json @@ -1,8 +1,8 @@ { - "extends": "./tsconfig.json", - "exclude": [ - "node_modules", - "src/test-utils", - "src/**/*.test.ts" - ] + "extends": "./tsconfig.json", + "exclude": [ + "node_modules", + "src/test-utils", + "src/**/*.test.ts" + ] } \ No newline at end of file diff --git a/server/tsconfig.json b/server/tsconfig.json index d18985e1b..3f9cbd541 100644 --- a/server/tsconfig.json +++ b/server/tsconfig.json @@ -19,7 +19,7 @@ "typeRoots": [ "@types", "node_modules/@types" - ] + ], }, "ts-node": { "require": [