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

[BIND 10 Development]

#838: "string iterator is not dereferencable" issue
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  ocean
  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 ocean):

 Replying to [comment:16 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).
 >
 Consider the input is a value in range between [-128, 0), it will be
 mapped
 to [0x80, 0xFF) after {{{static_cast<unsigned char>(c)}}}, which are
 control and Latin-1 supplement in Unicode. For example, 0xA0 is
 defined as "non-break space", so whether isspace(0xA0) returned true or
 false
 depends on the implementation. I have tested on my pc that it returns 0
 for
 [0x80, 0xFF], but I'm afraid that future or other implementation may break
 it.

 > 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.
 >
 OK, I will check it.

 > 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.

 Yes, I agree:)

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