🐛 (import) disallow importing transactions with too large or small am…

…ounts Closes #1811
actualbudget · Mar 23, 2024 · 5b60154 · 5b60154
1 parent afaee6b
commit 5b60154
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 25 deletions.
diff --git a/packages/desktop-client/src/components/modals/ImportTransactions.jsx b/packages/desktop-client/src/components/modals/ImportTransactions.jsx
@@ -6,6 +6,7 @@ import { format as formatDate_ } from 'loot-core/src/shared/months';
 import {
   amountToCurrency,
   amountToInteger,
+  isSafeNumber,
   looselyParseAmount,
 } from 'loot-core/src/shared/util';
 
@@ -248,14 +249,19 @@ function applyFieldMappings(transaction, mappings) {
   return result;
 }
 
-function parseAmount(amount, mapper) {
+function parseAmount(amount, mapper, multiplier) {
   if (amount == null) {
     return null;
   }
+
   const parsed =
     typeof amount === 'string' ? looselyParseAmount(amount) : amount;
-  const value = mapper(parsed);
-  return value;
+
+  if (parsed === null) {
+    return null;
+  }
+
+  return mapper(parsed) * multiplier;
 }
 
 function parseAmountFields(
@@ -272,10 +278,10 @@ function parseAmountFields(
     // Split mode is a little weird; first we look for an outflow and
     // if that has a value, we never want to show a number in the
     // inflow. Same for `amount`; we choose outflow first and then inflow
-    const outflow = parseAmount(trans.outflow, n => -Math.abs(n)) * multiplier;
+    const outflow = parseAmount(trans.outflow, n => -Math.abs(n), multiplier);
     const inflow = outflow
       ? 0
-      : parseAmount(trans.inflow, n => Math.abs(n)) * multiplier;
+      : parseAmount(trans.inflow, n => Math.abs(n), multiplier);
 
     return {
       amount: outflow || inflow,
@@ -285,17 +291,21 @@ function parseAmountFields(
   }
   if (inOutMode) {
     return {
-      amount:
-        parseAmount(trans.amount, n =>
-          trans.inOut === outValue ? Math.abs(n) * -1 : Math.abs(n),
-        ) * multiplier,
+      amount: parseAmount(
+        trans.amount,
+        n => (trans.inOut === outValue ? Math.abs(n) * -1 : Math.abs(n)),
+        multiplier,
+      ),
       outflow: null,
       inflow: null,
     };
   }
   return {
-    amount:
-      parseAmount(trans.amount, n => (flipAmount ? n * -1 : n)) * multiplier,
+    amount: parseAmount(
+      trans.amount,
+      n => (flipAmount ? n * -1 : n),
+      multiplier,
+    ),
     outflow: null,
     inflow: null,
   };
@@ -336,17 +346,14 @@ function Transaction({
     [rawTransaction, fieldMappings],
   );
 
-  let { amount, outflow, inflow } = parseAmountFields(
+  const { amount, outflow, inflow } = parseAmountFields(
     transaction,
     splitMode,
     inOutMode,
     outValue,
     flipAmount,
     multiplierAmount,
   );
-  amount = amountToCurrency(amount);
-  outflow = amountToCurrency(outflow);
-  inflow = amountToCurrency(inflow);
 
   return (
     <Row
@@ -388,17 +395,33 @@ function Transaction({
         <>
           <Field
             width={90}
-            contentStyle={{ textAlign: 'right', ...styles.tnum }}
-            title={outflow}
+            contentStyle={{
+              textAlign: 'right',
+              ...styles.tnum,
+              ...(outflow === null ? { color: theme.errorText } : {}),
+            }}
+            title={
+              outflow === null
+                ? 'Invalid: unable to parse the value'
+                : amountToCurrency(outflow)
+            }
           >
-            {outflow}
+            {amountToCurrency(outflow)}
           </Field>
           <Field
             width={90}
-            contentStyle={{ textAlign: 'right', ...styles.tnum }}
-            title={inflow}
+            contentStyle={{
+              textAlign: 'right',
+              ...styles.tnum,
+              ...(inflow === null ? { color: theme.errorText } : {}),
+            }}
+            title={
+              inflow === null
+                ? 'Invalid: unable to parse the value'
+                : amountToCurrency(inflow)
+            }
           >
-            {inflow}
+            {amountToCurrency(inflow)}
           </Field>
         </>
       ) : (
@@ -414,10 +437,18 @@ function Transaction({
           )}
           <Field
             width={90}
-            contentStyle={{ textAlign: 'right', ...styles.tnum }}
-            title={amount}
+            contentStyle={{
+              textAlign: 'right',
+              ...styles.tnum,
+              ...(amount === null ? { color: theme.errorText } : {}),
+            }}
+            title={
+              amount === null
+                ? `Invalid: unable to parse the value (${transaction.amount})`
+                : amountToCurrency(amount)
+            }
           >
-            {amount}
+            {amountToCurrency(amount)}
           </Field>
         </>
       )}

diff --git a/packages/loot-core/src/shared/util.ts b/packages/loot-core/src/shared/util.ts
@@ -290,6 +290,14 @@ export function getNumberFormat({
 const MAX_SAFE_NUMBER = 2 ** 51 - 1;
 const MIN_SAFE_NUMBER = -MAX_SAFE_NUMBER;
 
+export function isSafeNumber(value: number): boolean {
+  try {
+    safeNumber(value);
+    return true;
+  } catch (e) {
+    return false;
+  }
+}
 export function safeNumber(value: number) {
   if (!Number.isInteger(value)) {
     throw new Error(
@@ -359,7 +367,8 @@ export function integerToAmount(n) {
 // currencies. We extract out the numbers and just ignore separators.
 export function looselyParseAmount(amount: string) {
   function safeNumber(v: number): null | number {
-    return isNaN(v) ? null : v;
+    if (isSafeNumber(v)) return v;
+    return null;
   }
 
   function extractNumbers(v: string): string {