-
Notifications
You must be signed in to change notification settings - Fork 86
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
Upper and Lower do not handle invalid UTF-8 #2148
Comments
#2156 adds a hack to make sure we don't hang in this case, but we should fix this properly by ensuring STRING types are always valid UTF-8, which is a much bigger task. This is compliant with what postgres does. We should still keep this check just in case, it's always better to throw an unexpected error than crash. |
Solved in #2208 |
After a bunch of investigation, here's the state of things:
This implies that #2208 would fix the issue. However, I would instead propose that we treat all data in CSV and other files as BLOB, and then call an appropriate cast function. We already have DECODE for the latter task. It'd be good to make UTF-8 checks centralized, instead of duplicating and inevitably forgetting them here and there in the codebase. Finally, our current LIST_SLICE function doesn't respect UTF-8, and thus means that this issue is not fixed by #2208.
|
|
I don't know, because that seems silly. It definitely makes sense to implement BLOB -> String. I suppose BLOB -> INT64 only makes sense internally. We don't need a UTF-8 check for those cases, anything non-ascii isn't a valid integer anyway. On second thought I'm no longer sure what to do. Maybe having per-filetype checks is reasonable since they are so different (and we don't want to pay the price of a copy). |
My 2 cents is to have per filetype checks but I'll leave the solution to the implementer. I suggest we prioritize this. Calling upper and lower functions on UTF-8 could be a common scenario. |
Test for all string functions on utf-8 is done in #3287 |
If we call
Upper()
orLower()
with binary data (anything non-UTF-8), our application simply hangs.This is because we don't check the return value of
utf8proc_codepoint
, which is-1
when UTF-8 is invalid. Our size goes unmodified, and we loop forever. Worse, the assert we have simply does nothing, since (as far as I can tell), it doesn't check that the codepoint is non-negative.Reproduce with:
LOAD FROM "test.csv" RETURN upper(column0);
Where you can create test.csv by running:
python -c "open('test.csv', 'wb').write(b'\xff')"
The text was updated successfully, but these errors were encountered: