github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-07-22 22:17:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Reject invalid formulas on numeric inputs (#10659)

Browse Source 
Relative operators were accepted in operand position, so "10+*3" silently evaluated to 310 instead of being rejected. Make negation a first-class operand so legitimate negative operands ("10 + -3") keep working.

Fixes #9581

Signed-off-by: Akshit Nassa <akshitnassa412@gmail.com>
Co-authored-by: Akshit Nassa <akshitnassa412@gmail.com>
Co-authored-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
AK 2026-07-20 10:15:29 -04:00 committed by GitHub
parent f06339fb87
commit f2a9dd1a08
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 108 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
frontend/src/app/util/simple_math.cljs
View File

@ -9,17 +9,19 @@
[app.common.data :as d]
[app.common.exceptions :as ex]
[cljs.spec.alpha :as s]
[clojure.string :refer [index-of]]
[cuerdas.core :as str]
[instaparse.core :as insta]))
(def parser
(insta/parser
"opt-expr = '' | expr
expr = term (<spaces> ('+'|'-') <spaces> expr)* |
('+'|'-'|'*'|'/') <spaces> factor
term = factor (<spaces> ('*'|'/') <spaces> term)*
factor = number | ('(' <spaces> expr <spaces> ')')
;; Note that there is ambiguity, so we don't allow relative substraction,
;; a leading '-' is always a negation.
"opt-expr = '' | rel-expr | expr
rel-expr = ('+'|'*'|'/') <spaces> factor
expr = term (<spaces> ('+'|'-') <spaces> term)*
term = factor (<spaces> ('*'|'/') <spaces> factor)*
factor = number | neg | ('(' <spaces> expr <spaces> ')')
neg = <'-'> <spaces> factor
number = #'[0-9]*[.,]?[0-9]+%?'
spaces = ' '*"))
@ -31,26 +33,29 @@
:opt-expr
(if (empty? args) nil (interpret (first args) init-value))
:rel-expr
(let [operator (first args)
second-value (interpret (second args) init-value)]
(case operator
"+" (+ init-value second-value)
"*" (* init-value second-value)
"/" (/ init-value second-value)))
:neg
(- (interpret (first args) init-value))
:expr
(if (index-of "+-*/" (first args))
(let [operator (first args)
second-value (interpret (second args) init-value)]
(case operator
"+" (+ init-value second-value)
"-" (- 0 second-value) ;; Note that there is ambiguity, so we don't allow
"*" (* init-value second-value) ;; relative substraction, it's only a negative number
"/" (/ init-value second-value)))
(let [value (interpret (first args) init-value)]
(loop [value value
rest-expr (rest args)]
(if (empty? rest-expr)
value
(let [operator (first rest-expr)
second-value (interpret (second rest-expr) init-value)
rest-expr (-> rest-expr rest rest)]
(case operator
"+" (recur (+ value second-value) rest-expr)
"-" (recur (- value second-value) rest-expr)))))))
(let [value (interpret (first args) init-value)]
(loop [value value
rest-expr (rest args)]
(if (empty? rest-expr)
value
(let [operator (first rest-expr)
second-value (interpret (second rest-expr) init-value)
rest-expr (-> rest-expr rest rest)]
(case operator
"+" (recur (+ value second-value) rest-expr)
"-" (recur (- value second-value) rest-expr))))))
:term
(let [value (interpret (first args) init-value)]

78
frontend/test/frontend_tests/util_simple_math_test.cljs
View File

@ -107,3 +107,81 @@
(t/testing "Partial invalid expression should return nil"
(let [result (sm/expr-eval "10 + abc" 100)]
(t/is (= result nil)))))
(t/deftest test-relative-operators-only-at-top-level
(t/testing "Two consecutive operators should return nil"
(t/is (= nil (sm/expr-eval "10+*3" 100)))
(t/is (= nil (sm/expr-eval "10 + *3" 100)))
(t/is (= nil (sm/expr-eval "10+/3" 100)))
(t/is (= nil (sm/expr-eval "10 + +5" 100)))
(t/is (= nil (sm/expr-eval "10 * *5" 100)))
(t/is (= nil (sm/expr-eval "10 * /5" 100))))
(t/testing "Relative operators inside parentheses should return nil"
(t/is (= nil (sm/expr-eval "(*3)" 100)))
(t/is (= nil (sm/expr-eval "10 + (*3)" 100)))
(t/is (= nil (sm/expr-eval "+(10 + *3)" 100))))
(t/testing "A dangling operator should return nil"
(t/is (= nil (sm/expr-eval "10 +" 100)))
(t/is (= nil (sm/expr-eval "+" 100)))
(t/is (= nil (sm/expr-eval "*" 100))))
(t/testing "Valid relative expressions keep working"
(t/is (= 130 (sm/expr-eval "+30" 100)))
(t/is (= 300 (sm/expr-eval "*3" 100)))
(t/is (= -30 (sm/expr-eval "-30" 100)))
(t/is (= 50 (sm/expr-eval "/2" 100)))))
(t/deftest test-negative-operands
(t/testing "A negative number is a valid operand"
(t/is (= 7 (sm/expr-eval "10 + -3" 100)))
(t/is (= 7 (sm/expr-eval "10+-3" 100)))
(t/is (= 15 (sm/expr-eval "10 - -5" 100)))
(t/is (= -30 (sm/expr-eval "10 * -3" 100)))
(t/is (= 4 (sm/expr-eval "10 + -3 * 2" 100)))
(t/is (= -20 (sm/expr-eval "-(10 + 10)" 100)))
(t/is (= 5 (sm/expr-eval "10 - -5 * 1 - 10" 100))))
(t/testing "A negative number is a valid relative operand"
(t/is (= 70 (sm/expr-eval "+ -30" 100)))
(t/is (= -300 (sm/expr-eval "* -3" 100))))
(t/testing "A negative percentage is a valid operand"
(t/is (= 75 (sm/expr-eval "100 + -25%" 100)))
(t/is (= -25 (sm/expr-eval "-25%" 100)))))
(t/deftest test-operator-associativity
(t/testing "Chained subtraction is evaluated left to right"
(t/is (= 55 (sm/expr-eval "100 - 35 - 10" 999)))
(t/is (= 65 (sm/expr-eval "100 - 35 + 10 - 10" 999)))
(t/is (= 15 (sm/expr-eval "1 + 2 + 3 + 4 + 5" 999))))
(t/testing "Chained division is evaluated left to right"
(t/is (= 5 (sm/expr-eval "100 / 10 / 2" 999)))
(t/is (= 20 (sm/expr-eval "100 / 10 * 2" 999)))
(t/is (= 5 (sm/expr-eval "(100 / 10) / 2" 999))))
(t/testing "Precedence and parentheses are preserved"
(t/is (= 7 (sm/expr-eval "1 + 2 * 3" 999)))
(t/is (= 9 (sm/expr-eval "(1 + 2) * 3" 999)))))
(t/deftest test-edge-cases
(t/testing "Relative division by zero returns nil"
(t/is (= nil (sm/expr-eval "/0" 100)))
(t/is (= nil (sm/expr-eval "10 / 5 / 0" 100))))
(t/testing "Relative operators fall back to a zero init-value"
(t/is (= 10 (sm/expr-eval "+10" nil)))
(t/is (= 0 (sm/expr-eval "*10" nil))))
(t/testing "Percentages of the init-value"
(t/is (= 25 (sm/expr-eval "25%" 100)))
(t/is (= 125 (sm/expr-eval "+25%" 100))))
(t/testing "Non numeric input returns nil"
(t/is (= nil (sm/expr-eval " " 100)))
(t/is (= nil (sm/expr-eval "10 20" 100)))
(t/is (= nil (sm/expr-eval "(10" 100)))
(t/is (= nil (sm/expr-eval "10€" 100)))
(t/is (= nil (sm/expr-eval "🙂" 100)))))