Use checked string conversions

[Samyak2]

Description

Currently, strings are converted to BoundedString by simply using .into(). This is an unchecked conversion and will truncate the string to fit in the length - which is currently 63.

Steps to Reproduce

Expected Behavior

The conversions should be checked and an explicit error must be returned if the string is invalid (longer than 63). Having it silently fail without warnings makes a good foot gun :)

Actual Behavior

Reproduces How Often

Versions

Additional Information

[billy1624]

What your proposed changes on this? I guess we probably want to use this failable ArrayString::try_from_str method?

But then how do we adapt the existing API? I.e. force user to use this failable conversion instead of an simple into conversion which fail silently.

Maybe we can turn BoundedString into a concrete struct. Then, we can implement TryFrom<T> for it.

• https://github.com/SeaQL/sql-assembly/blob/328d17ae040e6997793f919250b4a7f7c5263dac/src/identifier.rs#L7-L8

[Samyak2]

What your proposed changes on this? I guess we probably want to use this failable ArrayString::try_from_str method?

Yes

But then how do we adapt the existing API? I.e. force user to use this failable conversion instead of an simple into conversion which fail silently.

Maybe we can turn BoundedString into a concrete struct. Then, we can implement TryFrom<T> for it.

Oh, I hadn't thought of that. That's a good idea.

I was just thinking of replacing into() with try_from_str() in the current codebase, since the main public API for this will be SQL anyway. But I think it's better to eliminate the footgun entirely.