[Bugfix] Color component values outside 0-255 wrap silently without warning #213

New issue

Open

opened 2026-01-12 03:49:00 +00:00 by john · 0 comments

john commented

2026-01-12 03:49:00 +00:00

Owner

Problem

In ParseColorArg (GridLayers.cpp:113-121), color component values are accepted as arbitrary integers and passed directly to sf::Color, which uses sf::Uint8 (unsigned 8-bit). Values outside the valid 0-255 range wrap silently due to integer overflow.

int r = (int)PyLong_AsLong(PySequence_Fast_GET_ITEM(seq, 0));
int g = (int)PyLong_AsLong(PySequence_Fast_GET_ITEM(seq, 1));
int b = (int)PyLong_AsLong(PySequence_Fast_GET_ITEM(seq, 2));
int a = (len == 4) ? (int)PyLong_AsLong(PySequence_Fast_GET_ITEM(seq, 3)) : 255;
// ...
out_color = sf::Color(r, g, b, a);  // Wraps to sf::Uint8

Example

layer.apply_threshold(hmap, (0.0, 1.0), (0, 0, 300))
# User expects blue with value 300
# Actually gets (0, 0, 44) because 300 % 256 = 44

Impact

Confusing results without any error or warning
Users may not realize their color values are being silently modified
Particularly problematic with programmatically generated colors

Proposed Solution

Add validation with either:

Error: Raise ValueError if any component is outside 0-255
Warning + Clamp: Issue a UserWarning and clamp values to valid range

Option 1 is stricter but catches bugs. Option 2 is more forgiving for edge cases.

if (r < 0 || r > 255 || g < 0 || g > 255 || b < 0 || b > 255 || a < 0 || a > 255) {
    PyErr_Format(PyExc_ValueError, 
        "%s color components must be in range 0-255, got (%d, %d, %d, %d)",
        arg_name, r, g, b, a);
    return false;
}

Affected Code

GridLayers.cpp: ParseColorArg() helper function
Affects: ColorLayer.apply_threshold(), ColorLayer.apply_gradient(), ColorLayer.apply_ranges()
Also affects: ColorLayer.fill(), ColorLayer.set(), ColorLayer.fill_rect() (which have their own inline parsing)

Discovered during code review of #200 and #201 implementations.

## Problem In `ParseColorArg` (GridLayers.cpp:113-121), color component values are accepted as arbitrary integers and passed directly to `sf::Color`, which uses `sf::Uint8` (unsigned 8-bit). Values outside the valid 0-255 range wrap silently due to integer overflow. ```cpp int r = (int)PyLong_AsLong(PySequence_Fast_GET_ITEM(seq, 0)); int g = (int)PyLong_AsLong(PySequence_Fast_GET_ITEM(seq, 1)); int b = (int)PyLong_AsLong(PySequence_Fast_GET_ITEM(seq, 2)); int a = (len == 4) ? (int)PyLong_AsLong(PySequence_Fast_GET_ITEM(seq, 3)) : 255; // ... out_color = sf::Color(r, g, b, a); // Wraps to sf::Uint8 ``` ## Example ```python layer.apply_threshold(hmap, (0.0, 1.0), (0, 0, 300)) # User expects blue with value 300 # Actually gets (0, 0, 44) because 300 % 256 = 44 ``` ## Impact - Confusing results without any error or warning - Users may not realize their color values are being silently modified - Particularly problematic with programmatically generated colors ## Proposed Solution Add validation with either: 1. **Error**: Raise `ValueError` if any component is outside 0-255 2. **Warning + Clamp**: Issue a `UserWarning` and clamp values to valid range Option 1 is stricter but catches bugs. Option 2 is more forgiving for edge cases. ```cpp if (r < 0 || r > 255 || g < 0 || g > 255 || b < 0 || b > 255 || a < 0 || a > 255) { PyErr_Format(PyExc_ValueError, "%s color components must be in range 0-255, got (%d, %d, %d, %d)", arg_name, r, g, b, a); return false; } ``` ## Affected Code - `GridLayers.cpp`: `ParseColorArg()` helper function - Affects: `ColorLayer.apply_threshold()`, `ColorLayer.apply_gradient()`, `ColorLayer.apply_ranges()` - Also affects: `ColorLayer.fill()`, `ColorLayer.set()`, `ColorLayer.fill_rect()` (which have their own inline parsing) ## Related Discovered during code review of #200 and #201 implementations.

john added the

Tiny Feature

label

2026-01-12 03:49:16 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

john/McRogueFace#213

No description provided.

Rows
Columns