BIND 10 #838: "string iterator is not dereferencable" issue

[BIND 10 Development]

#838: "string iterator is not dereferencable" issue
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  fdupont                            |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20110517
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  Unclassified                       |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  0.0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:15 ocean]:

 > > In any case, lacking the environment, it's not very clear to me what
 > > is exactly wrong with the original implementation (calling operator*
 > > at the end() of the data is wrong of course, but I don't understand
 > > how that situation happened).  Please explain more details about the
 > > problem.
 > >
 > > Thanks,
 >
 > This is triggered by the following sequence.
 [snip]

 Okay, thanks.  I now think I fully understand the problem.

 I also successfully reproduced the same (I believe) problem on our
 Solaris buildbox machine with the STLPort debug mode.

 Now, comments about the fix:

 It seems to be the correct fix to throw an exception in
 DecodeNormalizer::operator*().  But we can then make another cleanup:
 we now don't need to check whether *base_ is BASE_PADDING_CHAR.  It
 was an (incorrect) attempt to avoid the situation that the base_ ==
 base_end_ check now does.

 Also,

 > 1. Given input string of " " which is a one char string with 0x20 (the
 space) character.

 with your fix, the input of " " will result in the exception, which is
 not correct.  It should be handled just like "".  On looking at it
 closely, I noticed the previous code doesn't handle the case where the
 input begins with a space (this is a different problem than the
 iterator bug itself).

 Finally, regarding the conversion of char to signed int for isspace():
 I don't think checking it with >0 is the cleanest fix.  Even though I
 generally want to avoid relying on casts, it seems to be one of the
 (rare) cases where a cast makes most sense (and in this case the case
 should be safe).

 I've just committed my counter proposal based on the above comments
 and pushed it.  I also made a couple of unrelated (but STL-related)
 bugs in the test code.  Please check it (it's just a "proposal" and
 I've pushed it so that we can easily share it.)  Also, please consider
 adding more tests to base64 and hex tests that highlight the problem
 as I did it for base32hex.

 Oh, and really finally, a higher level point.  Wheter or not we create
 a separate ticket, I believe we should fix the problems Francis
 reported for Windows.  I agree with Francis on this point; it's not an
 issue specific for Windows, but real bugs that just happen to be
 revealed more clearly in a Windows environment (in fact, we can
 reproduce some of the problems on Solaris + STLPort).  Priority is
 another issue, and on that point the fact we are not seriously
 thinking Windows support can matter.

-- 
Ticket URL: <http://bind10.isc.org/ticket/838#comment:16>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development