Fix ArithmeticError when starting game with zero players by immortal71 · Pull Request #2336 · OWASP/cornucopia

immortal71 · 2026-02-21T12:20:15Z

Description

This PR resolves a critical bug where attempting to start a game without sufficient players causes an ArithmeticError crash due to division by zero in the card distribution logic.

Problem

The start_game event handler would crash when:

A game is created
Fewer than required players join
Someone triggers "Start Game" event

The crash occurred at this line:

player_id: Enum.fetch!(players, rem(i, Enum.count(players))).id

When Enum.count(players) = 0, the rem(i, 0) operation throws an ArithmeticError.

Additional issues identified during Copilot AI review:

Server-side validation didn't match UI requirement (3+ players)
update_game errors were not handled
length(players) was computed in every loop iteration (inefficient)

Solution

Code Changes

Server-Side Validation (Aligned with UI)
- Require minimum 3 players to start (matches UI requirement)
- Display user-friendly error message: "Cannot start game: At least 3 players are required to start."
- Prevent game start with 0, 1, or 2 players
Error Handling
- Pattern match on update_game result
- Show error flash if update fails: "Failed to start game. Please try again."
- Reload game state on failure to maintain consistency
Performance Optimization
- Compute player_count = length(players) once before loop
- Reuse player_count in rem/2 to avoid repeated traversals
Code Safety
- Replaced Enum.fetch! with safer Enum.at
- Added proper error handling for all edge cases
- Consistent return values in all cond branches
Comprehensive Tests
- Test validation with 0/1/2 players (all blocked with same error)
- Test successful start with 3+ players (button visible, game starts)
- Test idempotency (cannot restart already-started game)
- All tests directly trigger events to verify server-side security

Testing

All tests pass:

test "prevents starting game with fewer than 3 players"
test "successfully starts game with 3+ players"  
test "does not restart an already started game"

Copilot

Pull request overview

Fixes a LiveView crash in GameLive.Show when starting a game with zero players by adding server-side player-count validation before dealing cards.

Changes:

Add guard branches in "start_game" handler for already-started games and empty player lists.
Make card dealing logic safer by avoiding Enum.fetch!/2 and modulo-by-zero.
Add LiveView tests for starting games under different player counts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
copi.owasp.org/lib/copi_web/live/game_live/show.ex	Adds validation branches to prevent division-by-zero and returns consistently from the event handler.
copi.owasp.org/test/copi_web/live/game_live/show_test.exs	Introduces tests intended to cover 0/1/2+ player start behavior scenarios.

Copilot · 2026-02-21T12:25:30Z

copi.owasp.org/lib/copi_web/live/game_live/show.ex

+      length(game.players) < 2 ->
+        # Less than 2 players - warn but allow
+        {:noreply,
+         socket
+         |> put_flash(:info, "Note: Game works best with at least 2 players")
+         |> assign(:game, game)}
+


The < 2 players branch says "warn but allow", but it currently returns without dealing cards or setting started_at. Either update the logic to actually proceed with game start after setting the info flash, or change the comment/message and tests to reflect that starting is blocked with 1 player.

Copilot · 2026-02-21T12:25:31Z

copi.owasp.org/lib/copi_web/live/game_live/show.ex

+      Enum.empty?(game.players) ->
+        # No players have joined yet
+        {:noreply,
+         socket
+         |> put_flash(:error, "Cannot start game: No players have joined yet. Please add at least one player before starting.")
+         |> assign(:game, game)}
+
+      length(game.players) < 2 ->
+        # Less than 2 players - warn but allow
+        {:noreply,
+         socket
+         |> put_flash(:info, "Note: Game works best with at least 2 players")
+         |> assign(:game, game)}
+
+      true ->
+        # Valid player count, proceed with game start
+        all_cards = Copi.Cornucopia.list_cards_shuffled(game.edition, game.suits, latest_version(game.edition))
+        players = game.players


start_game now only blocks when player count is 0 or 1, but the UI currently only renders/enables the Start Game button once Enum.count(@game.players) > 2 (i.e., 3+ players). This mismatch makes the server-side rules unclear and still allows a crafted phx-click event to start the game with 2 players (or 1 if you later "allow" it). Align the server-side validation with the intended minimum player requirement, and update the UI copy/tests accordingly.

Copilot · 2026-02-21T12:25:31Z

copi.owasp.org/lib/copi_web/live/game_live/show.ex

+        Copi.Cornucopia.update_game(game, %{started_at: DateTime.truncate(DateTime.utc_now(), :second)})

-      {:ok, updated_game} = Game.find(game.id)
-      CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
+        {:ok, updated_game} = Game.find(game.id)
+        CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)


Copi.Cornucopia.update_game/2 returns {:ok, game} / {:error, changeset}, but the result is ignored here. If the update fails, this code will still Game.find/1 + broadcast, potentially showing an inconsistent state to clients. Pattern match on the result (or handle the error with a flash/log) and only broadcast after a successful update.

Copilot · 2026-02-21T12:25:31Z

copi.owasp.org/lib/copi_web/live/game_live/show.ex

+        all_cards
+        |> Enum.with_index()
+        |> Enum.each(fn {card, i} ->
+          Copi.Repo.insert!(%DealtCard{
+            card_id: card.id,
+            player_id: Enum.at(players, rem(i, length(players))).id
+          })


Inside the dealing loop, length(players) is recomputed for every card. Since length/1 traverses the list, compute the player count once before the loop (e.g., player_count = length(players)) and reuse it in rem/2 to avoid repeated work.

Copilot · 2026-02-21T12:25:31Z

copi.owasp.org/test/copi_web/live/game_live/show_test.exs

+    test "prevents starting game with no players", %{conn: conn, game: game} do
+      {:ok, view, _html} = live(conn, "/games/#{game.id}")
+
+      # Attempt to start game with no players
+      view |> element("button", "Start Game") |> render_click()
+
+      # Should display error message
+      assert render(view) =~ "Cannot start game: No players have joined yet"
+
+      # Verify game was not started
+      {:ok, updated_game} = Cornucopia.Game.find(game.id)
+      assert updated_game.started_at == nil
+    end
+
+    test "warns when starting game with only 1 player", %{conn: conn, game: game} do
+      # Add one player
+      {:ok, _player} = Cornucopia.create_player(%{name: "Solo Player", game_id: game.id})
+
+      {:ok, view, _html} = live(conn, "/games/#{game.id}")
+
+      # Attempt to start game with 1 player
+      view |> element("button", "Start Game") |> render_click()
+
+      # Should display warning message
+      assert render(view) =~ "Game works best with at least 2 players"
+    end
+
+    test "successfully starts game with 2+ players", %{conn: conn, game: game} do
+      # Add two players
+      {:ok, _player1} = Cornucopia.create_player(%{name: "Player 1", game_id: game.id})
+      {:ok, _player2} = Cornucopia.create_player(%{name: "Player 2", game_id: game.id})
+
+      {:ok, view, _html} = live(conn, "/games/#{game.id}")
+
+      # Start game
+      view |> element("button", "Start Game") |> render_click()
+


These tests attempt to click a "Start Game" button via element("button", "Start Game"), but show.html.heex only renders that button when there are 3+ players (Enum.count(@game.players) > 2). As written, the 0/1/2-player tests will fail because the element does not exist. Either trigger the event directly in the test (bypassing the DOM) or change the UI to render the button (disabled) for lower player counts and then assert on the flash/behavior.

Copilot · 2026-02-21T12:25:31Z

copi.owasp.org/test/copi_web/live/game_live/show_test.exs

+    test "successfully starts game with 2+ players", %{conn: conn, game: game} do
+      # Add two players
+      {:ok, _player1} = Cornucopia.create_player(%{name: "Player 1", game_id: game.id})
+      {:ok, _player2} = Cornucopia.create_player(%{name: "Player 2", game_id: game.id})
+
+      {:ok, view, _html} = live(conn, "/games/#{game.id}")
+
+      # Start game
+      view |> element("button", "Start Game") |> render_click()
+
+      # Verify game was started
+      {:ok, updated_game} = Cornucopia.Game.find(game.id)
+      assert updated_game.started_at != nil
+    end


The "successfully starts game with 2+ players" test expects a start with only 2 players, but the current UI copy/logic and existing CopiWeb.GameLiveTest cover starting when there are 3 players. Decide on the actual minimum player requirement (2 vs 3) and update the handler validation + tests (including existing tests/UI text) to match.

sydseter · 2026-02-21T13:17:21Z

@immortal71 Thank you for your time, please ensure you have properly signed your commit.

prakhar0x01 · 2026-02-21T15:04:17Z

@immortal71 ,

I faced the same issue, To pass CI checks, update your current working branch fix/start-game-validation , then update your changes and push the code.

Fixes OWASP#2335 This PR resolves a critical bug where attempting to start a game without sufficient players causes an ArithmeticError crash due to division by zero in the card distribution logic. Problem: - The start_game event handler would crash when rem(i, 0) was called - No server-side validation for minimum player count - Player count computed in every loop iteration (inefficient) - No error handling for update_game failures Solution: - Require minimum 3 players (aligned with UI requirement) - Add server-side validation with user-friendly error messages - Optimize by computing player_count once before loop - Handle update_game errors with proper error flash messages - Replace Enum.fetch! with safer Enum.at Tests: - Test validation with 0/1/2 players (all blocked) - Test successful start with 3+ players - Test idempotency (cannot restart already-started game) All tests directly trigger events to verify server-side security. Signed-off-by: immortal71 <newaashish190@gmail.com>

immortal71 · 2026-02-22T03:02:07Z

@sydseter done

Copilot AI review requested due to automatic review settings February 21, 2026 12:20

immortal71 requested review from cw-owasp, rewtd and sydseter as code owners February 21, 2026 12:20

Copilot started reviewing on behalf of immortal71 February 21, 2026 12:20 View session

Copilot AI reviewed Feb 21, 2026

View reviewed changes

immortal71 force-pushed the fix/start-game-validation branch from a5bbaa6 to 36aca8b Compare February 21, 2026 17:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

Fix ArithmeticError when starting game with zero players#2336

Fix ArithmeticError when starting game with zero players#2336
immortal71 wants to merge 1 commit intoOWASP:masterfrom
immortal71:fix/start-game-validation

immortal71 commented Feb 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

sydseter commented Feb 21, 2026

Uh oh!

prakhar0x01 commented Feb 21, 2026

Uh oh!

immortal71 commented Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Comments

Conversation

immortal71 commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Solution

Code Changes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

sydseter commented Feb 21, 2026

Uh oh!

prakhar0x01 commented Feb 21, 2026

Uh oh!

immortal71 commented Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

immortal71 commented Feb 21, 2026 •

edited

Loading