[stdlib] Fix unsafe String.write_bytes()

[martinvuyk]

Fix an unsafe assumption that the span is null terminated.

[JoeLoser]

Thanks for the fix! A few questions:

1. Is there a unit test we can add to expose this bug?
2. Should we add a debug_assert to ensure the argument to write_bytes is not null terminated per the doc string?

[martinvuyk]

I added a debug assert for the null termination, still there can be the case of data with a 0 somewhere in the middle but I'll leave a # TODO until PR #3686 gets merged which can count faster. Do you think it will be necessary to ensure no zeroes in the middle ? it might slow down the whole CI since string writing is something that gets used a lot.

Is there a unit test we can add to expose this bug?

I don't think there is one, since this was a call to a private method

[JoeLoser]

LGTM when CI is passing

[JoeLoser]

I added a debug assert for the null termination, still there can be the case of data with a 0 somewhere in the middle but I'll leave a # TODO until PR #3686 gets merged which can count faster. Do you think it will be necessary to ensure no zeroes in the middle ? it might slow down the whole CI since string writing is something that gets used a lot.

Is there a unit test we can add to expose this bug?

I don't think there is one, since this was a call to a private method

I think what you have is fine wrt the debug_assert safety check - thanks for adding that! Makes sense re the unit test.

[martinvuyk]

I don't think there is one, since this was a call to a private method

I mean theoretically we could, just need to find a way for the allocation to be just on the limit of a page and try to access that to get a segfault 😆

[martinvuyk]

@JoeLoser one problem, in Python unlike C you might actually have a string "\x00" which would be List[Byte](0, 0) and this debug assert would complain.

>>> print(repr("\0"), bytes("\0", "utf8"))
'\x00' b'\x00'

luckily we had tests that caught this

[JoeLoser]

!sync

[modularbot]

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

[modularbot]

Landed in 194b24b! Thank you for your contribution 🎉