tahonermann / text_view

A C++ concepts and range based character encoding and code point enumeration library
MIT License
122 stars 14 forks source link

Potential UB on exception in `otext_cursor::write` #28

Closed CaseyCarter closed 7 years ago

CaseyCarter commented 7 years ago

otext_cursor has two write members, both stylistically similar to:

void write(const character_type_t<encoding_type> &value) {
    iterator_type tmp{current};
    int encoded_code_units = 0;
    encoding_type::encode(state(), tmp, value, encoded_code_units);
    current = tmp;
}

Note that the function copies the adapted iterator member current, updates the copy by passing a reference to encoding_type::encode, and then stores its value back into current. Presumably the intent of this delayed update is to leave current unmodified if encoding_type::encode throws an exception.

However, iterator_type is only required to be single-pass: if it is truly a single pass (input or output but not forward) iterator, the "old" value in current is not required to be valid after encoding_type::encode increments tmp (or even simply writes a value through tmp in the output case). The exception safe approach may introduce undefined behavior.

I would solve this problem by creating an RAII guard class that stores a reference to an iterator and a copy of its value - only if that iterator's type models ForwardIterator - and moves the value back into the original object unless released prior to destruction.

tahonermann commented 7 years ago

Yup, good find, Casey, thank you! Your suggested solution sounds exactly right as well.

tahonermann commented 7 years ago

Fixed by commit 6b499433ea8a46a76df7cacb97863ece921f16bf.