[Bug]: Some values of the `color` parameter in search API trigger a 500

[diox]

What happened?

When using the color parameter in the add-ons search API, we don't handle the following cases well:

• Empty color parameter https://addons-dev.allizom.org/api/v5/addons/search/?type=statictheme&color=
• Some invalid color parameter like 8-characters hex colors https://addons-dev.allizom.org/api/v5/addons/search/?type=statictheme&color=336699aa

https://mozilla.sentry.io/issues/5802087597/?project=6310819

What did you expect to happen?

We should:

• For empty color, ignore the parameter completely
• For 8 characters hex strings, cut the last 2 chars (that represent transparency, which is not relevant here) and try to parse it
• Anything else that doesn't look like a valid color we should return a 400 error with an error message explaining that we expect a 6 chars hex string color code.

Is there an existing issue for this?

• [X] I have searched the existing issues

┆Issue is synchronized with this Jira Task

[diox]

@chrstinalin looks like this can still fail if sending " ": https://mozilla.sentry.io/issues/5899353034/?project=6224627&referrer=github-pr-bot

I can reproduce with curl http://olympia.test/api/v5/addons/search/\?type\=statictheme\&color\=%20

Maybe to be extra safe we need something like:

diff --git a/src/olympia/search/filters.py b/src/olympia/search/filters.py
index f4505d5a97..0d9421db1d 100644
--- a/src/olympia/search/filters.py
+++ b/src/olympia/search/filters.py
@@ -441,6 +441,8 @@ class AddonColorQueryParam(AddonQueryParam):
             hexvalue = ''.join(2 * c for c in hexvalue)
         try:
             rgb = tuple(bytearray.fromhex(hexvalue))
+            if len(rgb) != 3:
+                raise ValueError()
         except ValueError as err:
             raise ValueError(
                 gettext('Invalid "%s" parameter.' % self.query_param)

[ioanarusiczki]

The only time I could reproduce a 500 error instead of a 400 is when I used white space between = and & in the url below

https://addons-dev.allizom.org/en-US/firefox/search/?color=%20&type=statictheme

I was just editing ...

[chrstinalin]

@diox It looks like its unique to whitespace due to bytearray.fromhex() taking whitespace, as inputs < 3 that aren't whitespace correctly throw the error.

Create a bytearray object from a string of hexadecimal numbers. Spaces between two numbers are accepted. Example: bytearray.fromhex('B9 01EF') -> bytearray(b'\xb9\x01\xef')

Now that you point it out, there's a few more edge cases I'd be worried about with whitespace, like aa%20aa%20aaor a%20a%20a.

[diox]

Maybe instead of lstrip('#') we could remove anything that is not a valid hexadecimal character before doing the [:6] part if we really want to be nice. Or just throw an error. In any case this is the user clearly playing with the data so I'm not worried about it, but we should find a way to not raise a 500.

[ioanarusiczki]

I could not reproduce a 500 error anymore. If I use under 6 chars it sometimes throws a 404. But for 6+ the search results continues to be displayed.