String::concat(char*, size_t) doesn't support non-null terminated views

[paulocsanz]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Problem Description

If a non-null terminated char* view (like std::array<char, 32> md5 as storage) is passed to String::concat it currently crashes, because it's triggering UB by reading after the end of the buffer. This is the only way the current String API could allow adding a non-null terminated string to String, all the rest depends on strlen. So it's kinda important to the non-null terminated view use case.

[mcspr]

If a non-null terminated char* view (like std::array<char, 32> md5 as storage) is passed to String::concat it currently crashes

Could you share the specific crash condition though? idk if this has anything to do with nullchars, as we can read whatever with memcpy, if it is in the valid memory range? e.g. if array is at 0x40000000 - 32, reading past 0x40000000 will cause the crash unless it does aligned reads

edit: rephrasing - memmove_P resolves to either plain memmove or memcpy_P, depending on the pointer address value. Original pointer selects memmove, but reading past 0x4.... requires the other variant.

[paulocsanz]

I'm not sure, but I was doing something close to that, we don't use the stack at the 4kb unused sys stack, so we put some std::arrays in its place and the array that seemed to be the problem was the last one in the struct, but it didn't even reach 0x3FFFF000, so it may just be a misdiagnose of mine.