local_time::posix_time_zone_base::calc_zone Misuses String Iterator

[Rob Stewart <robert.stewart@…>]

calc_zone() starts like this:

void calc_zone(const string_type& obj){

const char_type empty_string[2] = {'\0'}; stringstream_type ss(empty_string); typename string_type::const_iterator sit = obj.begin(); string_type l_std_zone_abbrev, l_dst_zone_abbrev;

get 'std' name/abbrev while(std::isalpha(*sit)){

ss << *sit++;

}

Notice that sit is never compared against obj.end(). This code apparently assumes that some non-alpha character will appear, if only a null terminator. However, a checked STL flags *sit, in the conditional, as an error when trying to dereference the end iterator.

The particular example I encountered in which it failed was when obj was a std::string, of length 9, containing "Australia". There is no non-alpha character before advancing the iterator to the end, so operator *() fails with: "string iterator not dereferencable" in MSVC 8 (DevStudio 2005).

I suggest changing the loop to the following:

typename string_type::const_iterator it(obj.begin()); typename string_type::const_iterator end(obj.end()); get 'std' name/abbrev while (it != send && std::isalpha(*it)) {

ss << *it++;

}

(You can reuse end in the rest of the function, while you're at it!)

[Rob Stewart <robert.stewart@…>]

Wow! Trac really munged the code I entered and I forgot to s/send/end/ in the suggested change.

Later releases (I happened to have looked at the code from 1.33 when I copied the above) already do the equivalent of eit (obj_end). However, the bug remains, so here's the change needed for the trunk:

while (it != obj_end && std::isalpha(*it)) {

ss << *it++;

}

[Rob Stewart <robert.stewart@…>]

I was just creating a local diff for 1.39 and realized that the iterator sit has been changed to a pointer, thus bypassing the checked iterator problem of the earlier version.

Sorry for the noise!