Skip to content

fix(rgbgradient): raise a clear error for an empty color gradient - #289

Open
eeshsaxena wants to merge 1 commit into
flozz:masterfrom
eeshsaxena:fix/rgbgradient-empty-colors
Open

fix(rgbgradient): raise a clear error for an empty color gradient#289
eeshsaxena wants to merge 1 commit into
flozz:masterfrom
eeshsaxena:fix/rgbgradient-empty-colors

Conversation

@eeshsaxena

Copy link
Copy Markdown
Contributor

Bug

_handle_rgbgradient_dict() (rivalcfg/handlers/rgbgradient.py) accesses gradient[-1] in its smooth-gradient step without checking that any color was parsed:

# Smooth gradient (if possible) by adding a final color
if len(gradient) < 14 and gradient[-1]["pos"] != 100:
    gradient.append({"pos": 100, "color": gradient[0]["color"]})

A dict with no "colors" key, or "colors": [], is reachable through the documented Python dict API, and it makes gradient empty. gradient[-1] then raises IndexError: list index out of range before process_value() can reach its own if len(gradient) == 0: raise ValueError("no color: ...") check just below.

rgbgradientv2 is affected too, since its process_value() reuses this same function.

from rivalcfg.handlers import rgbgradient
setting_info = {"rgbgradient_header": {...}, "led_id": 1}

rgbgradient.process_value(setting_info, {"duration": 1000})
# IndexError: list index out of range   (expected: ValueError "no color")

rgbgradient.process_value(setting_info, {"colors": []})
# IndexError: list index out of range

Fix

Guard the smooth-gradient step with if gradient and .... The empty case now falls through to the existing "no color" ValueError in both rgbgradient and rgbgradientv2, which is the intended behavior. Valid single colors, gradients, and tuples are unaffected.

Tests

Adds regression tests to test/handlers/test_rgbgradient.py and test/handlers/test_rgbgradientv2.py (dict without a colors key, and empty colors). They fail on master (IndexError) and pass with this change; the full gradient handler suite stays green (86 passed). black and flake8 are clean.

`_handle_rgbgradient_dict()` accessed `gradient[-1]` in the smooth-gradient
step without first checking that any color was parsed. A dict with no
"colors" key (or `"colors": []`), which is reachable through the documented
Python API, therefore raised `IndexError: list index out of range` instead
of the intended `ValueError("no color: ...")` that `process_value()` raises
right after. rgbgradientv2 hits it too, since it reuses this function.

Guard the smooth-gradient step with `if gradient and ...`; the empty case
now falls through to the existing "no color" check in both handlers.

Adds regression tests to test_rgbgradient.py and test_rgbgradientv2.py.
@eeshsaxena

Copy link
Copy Markdown
Contributor Author

Hi! Gentle nudge on this one whenever you have some bandwidth. It's a small, self-contained fix (fix(rgbgradient): raise a clear error for an empty color gradient), and it's currently mergeable with no conflicts. No urgency at all, and I'm happy to make any changes you'd like. Thanks for maintaining rivalcfg!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant