From 4fbbb78125b58a7360d9441fbd175e658e03f534 Mon Sep 17 00:00:00 2001
From: Paul Buetow <paul@buetow.org>
Date: Sat, 23 May 2026 22:54:34 +0300
Subject: fix: prevent stack corruption in popTwo() and FastPower()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

popTwo() used to pop b first, then a. If the second pop failed, b was
already gone — the stack was silently corrupted.

Fix: add ensureStackLength() check before any pop, preserving stack
state on error. If the second pop fails (should not happen after the
check, but added as belt-and-suspenders), restore b to the stack.

FastPower() had the same issue (two sequential popStack calls).
Replaced with popTwo() for consistency.

Also fixed rpn_test.go: TestParseAndEvaluateInsufficientOperands
shared a single RPN instance across test cases, which relied on the
old stack-corruption behavior. Now each test case gets a fresh instance.
---
 internal/rpn/operations_arithmetic.go | 7 +------
 internal/rpn/operations_helpers.go    | 8 ++++++++
 internal/rpn/rpn_test.go              | 5 ++---
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/internal/rpn/operations_arithmetic.go b/internal/rpn/operations_arithmetic.go
index 8504d34..fd1b0fa 100644
--- a/internal/rpn/operations_arithmetic.go
+++ b/internal/rpn/operations_arithmetic.go
@@ -233,12 +233,7 @@ func (o *Operations) Modulo(stack *Stack) error {
 // FastPower pops two values from stack, raises first to integer power of second (a ** b), and pushes result.
 // Uses binary exponentiation for efficiency with large integer exponents.
 func (o *Operations) FastPower(stack *Stack) error {
-	b, err := popStack(stack, "**")
-	if err != nil {
-		return err
-	}
-
-	a, err := popStack(stack, "**")
+	a, b, err := popTwo(stack, "**")
 	if err != nil {
 		return err
 	}
diff --git a/internal/rpn/operations_helpers.go b/internal/rpn/operations_helpers.go
index 3d5f944..afdfa0e 100644
--- a/internal/rpn/operations_helpers.go
+++ b/internal/rpn/operations_helpers.go
@@ -19,7 +19,12 @@ func popStack(stack *Stack, op string) (StackValue, error) {
 }
 
 // popTwo pops two values from the stack for binary operations.
+// Uses ensureStackLength to prevent stack corruption on second Pop failure.
 func popTwo(stack *Stack, op string) (StackValue, StackValue, error) {
+	if err := ensureStackLength(stack, 2, op); err != nil {
+		return nil, nil, err
+	}
+
 	b, err := stack.Pop()
 	if err != nil {
 		return nil, nil, fmt.Errorf("insufficient operands for %s: %w", op, err)
@@ -27,6 +32,9 @@ func popTwo(stack *Stack, op string) (StackValue, StackValue, error) {
 
 	a, err := stack.Pop()
 	if err != nil {
+		// Should not happen given ensureStackLength check above.
+		// If it does (concurrent modification), restore b and return error.
+		stack.Push(b)
 		return nil, nil, fmt.Errorf("insufficient operands for %s: %w", op, err)
 	}
 
diff --git a/internal/rpn/rpn_test.go b/internal/rpn/rpn_test.go
index 0460271..494e377 100644
--- a/internal/rpn/rpn_test.go
+++ b/internal/rpn/rpn_test.go
@@ -302,9 +302,6 @@ func TestParseAndEvaluateUnknownToken(t *testing.T) {
 }
 
 func TestParseAndEvaluateInsufficientOperands(t *testing.T) {
-	v := NewVariables()
-	r := NewRPN(v)
-
 	tests := []struct {
 		name  string
 		input string
@@ -316,6 +313,8 @@ func TestParseAndEvaluateInsufficientOperands(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			v := NewVariables()
+			r := NewRPN(v)
 			_, err := r.ParseAndEvaluate(tt.input)
 			if err == nil {
 				t.Errorf("%q should return error for insufficient operands", tt.input)
-- 
cgit v1.2.3