From 246d668951dbfee201aef945f46c699819ae75df Mon Sep 17 00:00:00 2001 From: catloversg <152669316+catloversg@users.noreply.github.com> Date: Tue, 12 Nov 2024 22:39:21 +0700 Subject: [PATCH] CORPORATION: Rewrite validation code for strings of price and quantity (#1753) --- src/Corporation/Actions.ts | 192 +++++++----------- src/Corporation/Division.ts | 151 +++++++++----- .../ui/modals/SellMaterialModal.tsx | 6 + .../ui/modals/SellProductModal.tsx | 20 +- test/jest/Corporation.test.ts | 48 ++++- 5 files changed, 240 insertions(+), 177 deletions(-) diff --git a/src/Corporation/Actions.ts b/src/Corporation/Actions.ts index 0dcbffedd..ac63e1217 100644 --- a/src/Corporation/Actions.ts +++ b/src/Corporation/Actions.ts @@ -198,125 +198,89 @@ export function acceptInvestmentOffer(corporation: Corporation): void { corporation.investorShares += investShares; } -export function sellMaterial(material: Material, amount: string, price: string): void { - if (price === "") price = "0"; - if (amount === "") amount = "0"; - let cost = price.replace(/\s+/g, ""); - cost = cost.replace(/[^-()\d/*+.MPe]/g, ""); //Sanitize cost - let temp = cost.replace(/MP/, "1.234e5"); - try { - if (temp.includes("MP")) throw "Only one reference to MP is allowed in sell price."; - temp = eval?.(temp); - } catch (error) { - throw new Error("Invalid value or expression for sell price field", { cause: error }); - } +export function convertPriceString(price: string): string { + /** + * Replace invalid characters. Only accepts: + * - Digit characters + * - 4 most basic algebraic operations (+ - * /) + * - Parentheses + * - Dot character + * - Any characters in this list: [e, E, M, P] + */ + const sanitizedPrice = price.replace(/[^\d+\-*/().eEMP]/g, ""); - if (temp == null || isNaN(parseFloat(temp))) { - throw new Error("Invalid value or expression for sell price field"); - } - - if (cost.includes("MP")) { - material.desiredSellPrice = cost; //Dynamically evaluated - } else { - material.desiredSellPrice = temp; - } - - //Parse quantity - amount = amount.toUpperCase(); - if (amount.includes("MAX") || amount.includes("PROD") || amount.includes("INV")) { - let q = amount.replace(/\s+/g, ""); - q = q.replace(/[^-()\d/*+.MAXPRODINV]/g, ""); - let tempQty = q.replace(/MAX/g, material.maxSellPerCycle.toString()); - tempQty = tempQty.replace(/PROD/g, material.productionAmount.toString()); - tempQty = tempQty.replace(/INV/g, material.productionAmount.toString()); + // Replace MP with test numbers. + for (const testNumber of [-1.2e123, -123456, 123456, 1.2e123]) { + const temp = sanitizedPrice.replace(/MP/g, testNumber.toString()); + let evaluatedTemp: unknown; try { - tempQty = eval?.(tempQty); + evaluatedTemp = eval?.(temp); + if (typeof evaluatedTemp !== "number" || !Number.isFinite(evaluatedTemp)) { + throw new Error( + `Evaluated value is not a valid number: ${evaluatedTemp}. Price: ${price}. sanitizedPrice: ${sanitizedPrice}. testNumber: ${testNumber}.`, + ); + } } catch (error) { - throw new Error("Invalid value or expression for sell quantity field", { cause: error }); + throw new Error(`Invalid value or expression for sell price field: ${error}`, { cause: error }); } - - if (tempQty == null || isNaN(parseFloat(tempQty))) { - throw new Error("Invalid value or expression for sell quantity field"); - } - material.desiredSellAmount = q; //Use sanitized input - } else if (isNaN(parseFloat(amount)) || parseFloat(amount) < 0) { - throw new Error("Invalid value for sell quantity field! Must be numeric or 'PROD' or 'MAX'"); - } else { - let q = parseFloat(amount); - if (isNaN(q)) { - q = 0; - } - material.desiredSellAmount = q; } + + // Use sanitized price. + return sanitizedPrice; +} + +export function convertAmountString(amount: string): string { + /** + * Replace invalid characters. Only accepts: + * - Digit characters + * - 4 most basic algebraic operations (+ - * /) + * - Parentheses + * - Dot character + * - Any characters in this list: [e, E, M, A, X, P, R, O, D, I, N, V] + */ + const sanitizedAmount = amount.replace(/[^\d+\-*/().eEMAXPRODINV]/g, ""); + + for (const testNumber of [-1.2e123, -123456, 123456, 1.2e123]) { + let temp = sanitizedAmount.replace(/MAX/g, testNumber.toString()); + temp = temp.replace(/PROD/g, testNumber.toString()); + temp = temp.replace(/INV/g, testNumber.toString()); + let evaluatedTemp: unknown; + try { + evaluatedTemp = eval?.(temp); + if (typeof evaluatedTemp !== "number" || !Number.isFinite(evaluatedTemp)) { + throw new Error( + `Evaluated value is not a valid number: ${evaluatedTemp}. Amount: ${amount}. sanitizedAmount: ${sanitizedAmount}. testNumber: ${testNumber}.`, + ); + } + } catch (error) { + throw new Error(`Invalid value or expression for sell quantity field: ${error}`, { cause: error }); + } + } + + // Use sanitized amount. + return sanitizedAmount; +} + +export function sellMaterial(material: Material, amount: string, price: string): void { + const convertedPrice = convertPriceString(price.toUpperCase()); + const convertedAmount = convertAmountString(amount.toUpperCase()); + + material.desiredSellPrice = convertedPrice; + material.desiredSellAmount = convertedAmount; } export function sellProduct(product: Product, city: CityName, amt: string, price: string, all: boolean): void { - //Parse price - // initliaze newPrice with oldPrice as default - let newPrice = product.cityData[city].desiredSellPrice; - if (price.includes("MP")) { - //Dynamically evaluated quantity. First test to make sure its valid - //Sanitize input, then replace dynamic variables with arbitrary numbers - price = price.replace(/\s+/g, ""); - price = price.replace(/[^-()\d/*+.MPe]/g, ""); - let temp = price.replace(/MP/, "1.234e5"); - try { - if (temp.includes("MP")) throw "Only one reference to MP is allowed in sell price."; - temp = eval?.(temp); - } catch (error) { - throw new Error("Invalid value or expression for sell price field.", { cause: error }); - } - if (temp == null || isNaN(parseFloat(temp))) { - throw new Error("Invalid value or expression for sell price field."); - } - newPrice = price; //Use sanitized price - } else { - const cost = parseFloat(price); - if (isNaN(cost)) { - throw new Error("Invalid value for sell price field"); - } - newPrice = cost; - } + const convertedPrice = convertPriceString(price.toUpperCase()); + const convertedAmount = convertAmountString(amt.toUpperCase()); - // Parse quantity - amt = amt.toUpperCase(); - //initialize newAmount with old as default - let newAmount = product.cityData[city].desiredSellAmount; - if (amt.includes("MAX") || amt.includes("PROD") || amt.includes("INV")) { - //Dynamically evaluated quantity. First test to make sure its valid - let qty = amt.replace(/\s+/g, ""); - qty = qty.replace(/[^-()\d/*+.MAXPRODINV]/g, ""); - let temp = qty.replace(/MAX/g, product.maxSellAmount.toString()); - temp = temp.replace(/PROD/g, product.cityData[city].productionAmount.toString()); - temp = temp.replace(/INV/g, product.cityData[city].stored.toString()); - try { - temp = eval?.(temp); - } catch (error) { - throw new Error("Invalid value or expression for sell quantity field", { cause: error }); - } - - if (temp == null || isNaN(parseFloat(temp))) { - throw new Error("Invalid value or expression for sell quantity field"); - } - newAmount = qty; //Use sanitized input - } else if (isNaN(parseFloat(amt)) || parseFloat(amt) < 0) { - throw new Error("Invalid value for sell quantity field! Must be numeric or 'PROD' or 'MAX'"); - } else { - let qty = parseFloat(amt); - if (isNaN(qty)) { - qty = 0; - } - newAmount = qty; - } - //apply new price and amount to all or just current if (all) { for (const cityName of Object.values(CityName)) { - product.cityData[cityName].desiredSellAmount = newAmount; - product.cityData[cityName].desiredSellPrice = newPrice; + product.cityData[cityName].desiredSellAmount = convertedAmount; + product.cityData[cityName].desiredSellPrice = convertedPrice; } } else { - product.cityData[city].desiredSellAmount = newAmount; - product.cityData[city].desiredSellPrice = newPrice; + product.cityData[city].desiredSellAmount = convertedAmount; + product.cityData[city].desiredSellPrice = convertedPrice; } } @@ -577,23 +541,21 @@ Attempted export amount: ${amount}`); sanitizedAmt = sanitizedAmt.replace(/[^-()\d/*+.MAXEPRODINV]/g, ""); for (const testReplacement of ["(1.23)", "(-1.23)"]) { const replaced = sanitizedAmt.replace(/(MAX|IPROD|EPROD|IINV|EINV)/g, testReplacement); - let evaluated, error; + let evaluated: unknown; try { evaluated = eval?.(replaced); - } catch (e) { - error = e; - } - if (!error && isNaN(evaluated)) error = "evaluated value is NaN"; - if (error) { + if (typeof evaluated !== "number" || !Number.isFinite(evaluated)) { + throw new Error(`Evaluated value is not a valid number: ${evaluated}`); + } + } catch (error) { throw new Error( `Error while trying to set the exported amount of ${material.name}. Error occurred while testing keyword replacement with ${testReplacement}. Your input: ${amount} Sanitized input: ${sanitizedAmt} Input after replacement: ${replaced} -Evaluated value: ${evaluated}` + - // eslint-disable-next-line @typescript-eslint/no-base-to-string - `Error encountered: ${error}`, +Evaluated value: ${evaluated} +Error encountered: ${error}`, ); } } diff --git a/src/Corporation/Division.ts b/src/Corporation/Division.ts index 5886b764e..d88d5d642 100644 --- a/src/Corporation/Division.ts +++ b/src/Corporation/Division.ts @@ -16,6 +16,7 @@ import { PartialRecord, getRecordEntries, getRecordKeys, getRecordValues } from import { Material } from "./Material"; import { getKeyList } from "../utils/helpers/getKeyList"; import { calculateMarkupMultiplier } from "./helpers"; +import { exceptionAlert } from "../utils/helpers/exceptionAlert"; import { throwIfReachable } from "../utils/helpers/throwIfReachable"; interface DivisionParams { @@ -538,25 +539,30 @@ export class Division { // The amount gets re-multiplied later, so this is the correct // amount to calculate with for "MAX". const adjustedQty = mat.stored / (corpConstants.secondsPerMarketCycle * marketCycles); - if (typeof mat.desiredSellAmount === "string") { - //Dynamically evaluated - let tmp = mat.desiredSellAmount.replace(/MAX/g, adjustedQty.toString()); - tmp = tmp.replace(/PROD/g, mat.productionAmount.toString()); - try { - sellAmt = eval?.(tmp); - } catch (e) { - dialogBoxCreate( - `Error evaluating your sell amount for material ${mat.name} in ${this.name}'s ${city} office. The sell amount is being set to zero`, - ); - sellAmt = 0; + /** + * desiredSellAmount is usually a string, but it also can be a number in old versions. eval requires a + * string, so we convert it to a string here, replace placeholders, and then pass it to eval. + */ + let temp = String(mat.desiredSellAmount); + temp = temp.replace(/MAX/g, adjustedQty.toString()); + temp = temp.replace(/PROD/g, mat.productionAmount.toString()); + temp = temp.replace(/INV/g, mat.stored.toString()); + try { + // Typecasting here is fine. We will validate the result immediately after this line. + sellAmt = eval?.(temp) as number; + if (typeof sellAmt !== "number" || !Number.isFinite(sellAmt)) { + throw new Error(`Evaluated value is not a valid number: ${sellAmt}`); } - } else { - sellAmt = mat.desiredSellAmount; + } catch (error) { + dialogBoxCreate( + `Error evaluating your sell amount for material ${mat.name} in ${this.name}'s ${city} office. Error: ${error}.`, + ); + continue; } // Determine the cost that the material will be sold at const markupLimit = mat.getMarkupLimit(); - let sCost; + let sCost: number; if (mat.marketTa2) { // Reverse engineer the 'maxSell' formula // 1. Set 'maxSell' = sellAmt @@ -590,11 +596,29 @@ export class Division { sCost = optimalPrice; } else if (mat.marketTa1) { sCost = mat.marketPrice + markupLimit; - } else if (typeof mat.desiredSellPrice === "string") { - sCost = mat.desiredSellPrice.replace(/MP/g, mat.marketPrice.toString()); - sCost = eval?.(sCost); } else { - sCost = mat.desiredSellPrice; + // If the player does not set the price, desiredSellPrice will be an empty string. + if (mat.desiredSellPrice === "") { + continue; + } + /** + * desiredSellPrice is usually a string, but it also can be a number in old versions. eval requires a + * string, so we convert it to a string here, replace the placeholder MP, and then pass it to eval later. + */ + const temp = String(mat.desiredSellPrice).replace(/MP/g, mat.marketPrice.toString()); + try { + // Typecasting here is fine. We will validate the result immediately after this line. + sCost = eval?.(temp) as number; + if (typeof sCost !== "number" || !Number.isFinite(sCost)) { + throw new Error(`Evaluated value is not a valid number: ${sCost}`); + } + } catch (error) { + dialogBoxCreate( + `Error evaluating your sell price for material ${mat.name} in ${this.name}'s ${city} office. ` + + `The sell amount is being set to zero. Error: ${error}`, + ); + continue; + } } mat.uiMarketPrice = sCost; @@ -658,19 +682,17 @@ export class Division { amtStr = amtStr.replace(/IINV/g, `(${tempMaterial.stored})`); let amt = 0; try { - amt = eval?.(amtStr); + // Typecasting here is fine. We will validate the result immediately after this line. + amt = eval?.(amtStr) as number; + if (typeof amt !== "number" || !Number.isFinite(amt)) { + throw new Error(`Evaluated value is not a valid number: ${amt}`); + } } catch (e) { dialogBoxCreate( `Calculating export for ${mat.name} in ${this.name}'s ${city} division failed with error: ${e}`, ); continue; } - if (isNaN(amt)) { - dialogBoxCreate( - `Error calculating export amount for ${mat.name} in ${this.name}'s ${city} division.`, - ); - continue; - } amt = amt * corpConstants.secondsPerMarketCycle * marketCycles; if (mat.stored < amt) { @@ -869,35 +891,43 @@ export class Division { const marketFactor = this.getMarketFactor(product); //Competition + demand // Parse player sell-amount input (needed for TA.II and selling) - let sellAmt: number | string; + let sellAmt: number; // The amount gets re-multiplied later, so this is the correct // amount to calculate with for "MAX". const adjustedQty = product.cityData[city].stored / (corpConstants.secondsPerMarketCycle * marketCycles); - const desiredSellAmount = product.cityData[city].desiredSellAmount; - if (typeof desiredSellAmount === "string") { - //Sell amount is dynamically evaluated - let tmp: number | string = desiredSellAmount.replace(/MAX/g, adjustedQty.toString()); - tmp = tmp.replace(/PROD/g, product.cityData[city].productionAmount.toString()); - try { - tmp = eval?.(tmp); - if (typeof tmp !== "number") throw ""; - } catch (e) { - dialogBoxCreate( - `Error evaluating your sell price expression for ${product.name} in ${this.name}'s ${city} office. Sell price is being set to MAX`, - ); - tmp = product.maxSellAmount; + /** + * desiredSellAmount is usually a string, but it also can be a number in old versions. eval requires a + * string, so we convert it to a string here, replace placeholders, and then pass it to eval. + */ + const desiredSellAmount = String(product.cityData[city].desiredSellAmount); + let temp = desiredSellAmount.replace(/MAX/g, adjustedQty.toString()); + temp = temp.replace(/PROD/g, product.cityData[city].productionAmount.toString()); + temp = temp.replace(/INV/g, product.cityData[city].stored.toString()); + try { + // Typecasting here is fine. We will validate the result immediately after this line. + sellAmt = eval?.(temp) as number; + if (typeof sellAmt !== "number" || !Number.isFinite(sellAmt)) { + throw new Error(`Evaluated value is not a valid number: ${sellAmt}`); } - sellAmt = tmp; - } else if (desiredSellAmount && desiredSellAmount > 0) { - sellAmt = desiredSellAmount; - } else sellAmt = adjustedQty; + } catch (error) { + dialogBoxCreate( + `Error evaluating your sell amount for ${product.name} in ${this.name}'s ${city} office. Error: ${error}.`, + ); + // break the case "SALE" + break; + } - if (sellAmt < 0) sellAmt = 0; + if (sellAmt < 0) { + sellAmt = 0; + } + if (product.markup === 0) { + exceptionAlert(new Error("product.markup is 0")); + product.markup = 1; + } // Calculate Sale Cost (sCost), which could be dynamically evaluated const markupLimit = Math.max(product.cityData[city].effectiveRating, 0.001) / product.markup; let sCost: number; - const sellPrice = product.cityData[city].desiredSellPrice; if (product.marketTa2) { // Reverse engineer the 'maxSell' formula // 1. Set 'maxSell' = sellAmt @@ -930,16 +960,29 @@ export class Division { sCost = optimalPrice; } else if (product.marketTa1) { sCost = product.cityData[city].productionCost + markupLimit; - } else if (typeof sellPrice === "string") { - let sCostString = sellPrice; - if (product.markup === 0) { - console.error(`mku is zero, reverting to 1 to avoid Infinity`); - product.markup = 1; - } - sCostString = sCostString.replace(/MP/g, product.cityData[city].productionCost.toString()); - sCost = eval?.(sCostString); } else { - sCost = sellPrice; + // If the player does not set the price, desiredSellPrice will be an empty string. + if (product.cityData[city].desiredSellPrice === "") { + // break the case "SALE" + break; + } + const temp = String(product.cityData[city].desiredSellPrice).replace( + /MP/g, + product.cityData[city].productionCost.toString(), + ); + try { + // Typecasting here is fine. We will validate the result immediately after this line. + sCost = eval?.(temp) as number; + if (typeof sCost !== "number" || !Number.isFinite(sCost)) { + throw new Error(`Evaluated value is not a valid number: ${sCost}`); + } + } catch (error) { + dialogBoxCreate( + `Error evaluating your sell price for product ${product.name} in ${this.name}'s ${city} office. Error: ${error}.`, + ); + // break the case "SALE" + break; + } } product.uiMarketPrice[city] = sCost; diff --git a/src/Corporation/ui/modals/SellMaterialModal.tsx b/src/Corporation/ui/modals/SellMaterialModal.tsx index 0c12b682e..c911d5c1a 100644 --- a/src/Corporation/ui/modals/SellMaterialModal.tsx +++ b/src/Corporation/ui/modals/SellMaterialModal.tsx @@ -31,10 +31,16 @@ export function SellMaterialModal(props: IProps): React.ReactElement { } function onAmtChange(event: React.ChangeEvent): void { + if (event.target.value === "") { + return; + } setAmt(event.target.value); } function onPriceChange(event: React.ChangeEvent): void { + if (event.target.value === "") { + return; + } setPrice(event.target.value); } diff --git a/src/Corporation/ui/modals/SellProductModal.tsx b/src/Corporation/ui/modals/SellProductModal.tsx index 76553ee6b..0e12ff658 100644 --- a/src/Corporation/ui/modals/SellProductModal.tsx +++ b/src/Corporation/ui/modals/SellProductModal.tsx @@ -21,8 +21,8 @@ interface IProps { // Create a popup that let the player manage sales of a material export function SellProductModal(props: IProps): React.ReactElement { const [checked, setChecked] = useState(true); - const [iQty, setQty] = useState(String(props.product.cityData[props.city].desiredSellAmount)); - const [px, setPx] = useState(String(props.product.cityData[props.city].desiredSellPrice)); + const [amt, setAmt] = useState(String(props.product.cityData[props.city].desiredSellAmount)); + const [price, setPrice] = useState(String(props.product.cityData[props.city].desiredSellPrice)); function onCheckedChange(event: React.ChangeEvent): void { setChecked(event.target.checked); @@ -30,7 +30,7 @@ export function SellProductModal(props: IProps): React.ReactElement { function sellProduct(): void { try { - actions.sellProduct(props.product, props.city, iQty, px, checked); + actions.sellProduct(props.product, props.city, amt, price, checked); } catch (error) { dialogBoxCreate(String(error)); } @@ -39,11 +39,17 @@ export function SellProductModal(props: IProps): React.ReactElement { } function onAmtChange(event: React.ChangeEvent): void { - setQty(event.target.value); + if (event.target.value === "") { + return; + } + setAmt(event.target.value); } function onPriceChange(event: React.ChangeEvent): void { - setPx(event.target.value); + if (event.target.value === "") { + return; + } + setPrice(event.target.value); } function onKeyDown(event: React.KeyboardEvent): void { @@ -75,14 +81,14 @@ export function SellProductModal(props: IProps): React.ReactElement {
- + diff --git a/test/jest/Corporation.test.ts b/test/jest/Corporation.test.ts index 3aa3c031a..50eebd7e3 100644 --- a/test/jest/Corporation.test.ts +++ b/test/jest/Corporation.test.ts @@ -11,6 +11,8 @@ import { PlayerObject } from "../../src/PersonObjects/Player/PlayerObject"; import { acceptInvestmentOffer, buyBackShares, + convertAmountString, + convertPriceString, goPublic, issueNewShares, sellShares, @@ -84,7 +86,10 @@ describe("Corporation", () => { it("should be preserved by seed funding", () => { const seedFunded = true; Player.startCorporation("TestCorp", seedFunded); - expectSharesToAddUp(Player.corporation!); + if (!Player.corporation) { + throw new Error("Player.startCorporation failed to create a corporation."); + } + expectSharesToAddUp(Player.corporation); }); it("should be preserved by acceptInvestmentOffer", () => { acceptInvestmentOffer(corporation); @@ -137,4 +142,45 @@ describe("Corporation", () => { }, ); }); + + describe("convertPriceString", () => { + it("should pass normally", () => { + expect(convertPriceString("MP")).toStrictEqual("MP"); + expect(convertPriceString("MP+1")).toStrictEqual("MP+1"); + expect(convertPriceString("MP+MP")).toStrictEqual("MP+MP"); + expect(convertPriceString("123")).toStrictEqual("123"); + expect(convertPriceString("123+456")).toStrictEqual("123+456"); + expect(convertPriceString("1e10")).toStrictEqual("1e10"); + expect(convertPriceString("1E10")).toStrictEqual("1E10"); + }); + it("should throw errors", () => { + expect(() => convertPriceString("")).toThrow(); + expect(() => convertPriceString("null")).toThrow(); + expect(() => convertPriceString("undefined")).toThrow(); + expect(() => convertPriceString("Infinity")).toThrow(); + expect(() => convertPriceString("abc")).toThrow(); + }); + }); + + describe("convertAmountString", () => { + it("should pass normally", () => { + expect(convertAmountString("MAX")).toStrictEqual("MAX"); + expect(convertAmountString("PROD")).toStrictEqual("PROD"); + expect(convertAmountString("INV")).toStrictEqual("INV"); + expect(convertAmountString("MAX+1")).toStrictEqual("MAX+1"); + expect(convertAmountString("MAX+MAX")).toStrictEqual("MAX+MAX"); + expect(convertAmountString("MAX+PROD+INV")).toStrictEqual("MAX+PROD+INV"); + expect(convertAmountString("123")).toStrictEqual("123"); + expect(convertAmountString("123+456")).toStrictEqual("123+456"); + expect(convertAmountString("1e10")).toStrictEqual("1e10"); + expect(convertAmountString("1E10")).toStrictEqual("1E10"); + }); + it("should throw errors", () => { + expect(() => convertAmountString("")).toThrow(); + expect(() => convertAmountString("null")).toThrow(); + expect(() => convertAmountString("undefined")).toThrow(); + expect(() => convertAmountString("Infinity")).toThrow(); + expect(() => convertAmountString("abc")).toThrow(); + }); + }); });