Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vectorio doesn't consistently validate location #5325

Closed
lesamouraipourpre opened this issue Sep 9, 2021 · 3 comments · Fixed by #5327
Closed

Vectorio doesn't consistently validate location #5325

lesamouraipourpre opened this issue Sep 9, 2021 · 3 comments · Fixed by #5327

Comments

@lesamouraipourpre
Copy link

lesamouraipourpre commented Sep 9, 2021

CircuitPython version

Adafruit CircuitPython 7.0.0-beta.0 on 2021-08-24; Adafruit PyPortal with samd51j20
Adafruit CircuitPython 7.0.0-rc.1-46-g0bde894b5 on 2021-09-09; Adafruit PyPortal with samd51j20

Code/REPL

import displayio
import vectorio

line_palette = displayio.Palette(1)
line_palette[0] = 0xFFFFFF

line_one = vectorio.Rectangle(width=64, height=1, x=0, y=0, pixel_shader=line_palette)

# Test Three - Doesn't error when location is outside of signed int 16 values
line_one = vectorio.Rectangle(width=64, height=1, x=50000, y=50000, pixel_shader=line_palette)
print("Test One: Location set to (50000, 50000) but stored as", line_one.location, "It should have errored")

for i in range(0,100000,10000):
    # Test Two - Doesn't error when location is outside of signed int 16 values
    line_one.x = i
    line_one.y = i
    print("Test Two", i, line_one.location)

for i in range(0,100000,10000):
    print("Test Three", i)
    # Test Three - Correctly errors when location is outside of signed int 16 values
    line_one.location = (i, i)
    print("  T3", line_one.location)

Behavior

Test One: Location set to (50000, 50000) but stored as (-15536, -15536) It should have errored
Test Two 0 (0, 0)
Test Two 10000 (10000, 10000)
Test Two 20000 (20000, 20000)
Test Two 30000 (30000, 30000)
Test Two 40000 (-25536, -25536)
Test Two 50000 (-15536, -15536)
Test Two 60000 (-5536, -5536)
Test Two 70000 (4464, 4464)
Test Two 80000 (14464, 14464)
Test Two 90000 (24464, 24464)
Test Three 0
  T3 (0, 0)
Test Three 10000
  T3 (10000, 10000)
Test Three 20000
  T3 (20000, 20000)
Test Three 30000
  T3 (30000, 30000)
Test Three 40000
Traceback (most recent call last):
  File "<stdin>", line 23, in <module>
ValueError: unsupported point type

Description

When you set the location of a vectorio shape via the constructor, the x or y properties, no bounds checking is performed. If it is set via the location property, bounds checking is performed and raises a ValueError

Additional information

No response

@lesamouraipourpre
Copy link
Author

Looking at this further the constructor expects the x,y location to be in uint16 range but the validation is done against sint16 range.

void common_hal_vectorio_vector_shape_construct(vectorio_vector_shape_t *self,
vectorio_ishape_t ishape,
mp_obj_t pixel_shader, uint16_t x, uint16_t y) {

@tannewt
Copy link
Member

tannewt commented Sep 10, 2021

Let's not block 7.0.0 on this. It can be fixed afterwards.

@kvc0
Copy link

kvc0 commented Sep 11, 2021

@tannewt agree this shouldn't slow down the 7.0.0 release train. Good candidate for 7.0.1 inclusion.

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

Successfully merging a pull request may close this issue.

4 participants