BIND 10 #256: bugs in base32.cc

BIND 10 Development do-not-reply at isc.org
Sun Jul 11 19:26:15 UTC 2010


#256: bugs in base32.cc
--------------------------------+-------------------------------------------
      Reporter:  jinmei         |        Owner:  jinmei                     
          Type:  defect         |       Status:  new                        
      Priority:  major          |    Milestone:  06. 4th Incremental Release
     Component:  DNSPacket API  |   Resolution:                             
      Keywords:                 |    Sensitive:  0                          
Estimatedhours:                 |        Hours:                             
      Billable:  1              |   Totalhours:                             
      Internal:  0              |  
--------------------------------+-------------------------------------------
Changes (by jinmei):

  * internal:  => 0
  * billable:  => 1


Comment:

 branches/trac256 is ready for review.  The diff can be retrieved by:
 {{{
 svn diff -r 2481 svn+ssh://bind10.isc.org/svn/bind10/branches/trac256
 }}}

 With this fix and trac #288 the sunstudio build will fully compile and
 pass tests.

 I've used this opportunity to clean up some other things (see below),
 which make the diff relatively big, but function-wise the essential
 changes are as follows:

  - dns/util/base_n.cc
  - dns/util/base{16,32hex}_from_binary.h
  - dns/util/binary_from_base{16,32hex}.h
  - dns/tests/base{16,32hex}_unittest.cc

 Rather than trying to identify and fix each and every bug of the
 original base32 (which should have been called base32hex btw, so I
 renamed it in this patch) I chose to generalize the base64 wrapper for
 boost base64-from-binary and binary-from-base64, and use the same code
 base for all base64, base32hex and "hex" (which used the base16
 encoding).  This way we can centralize the source of bug (if any), and
 can be confident that if it works with one encoding algorithm it
 should work with others equally.

 base_n.cc is the generalized wrapper implementation.  It's essentially
 the same as the original base64.cc, but employs internal class
 templates to support different encoding algorithm with different
 parameters.  The implementation may not look so trivial, so I added
 relatively detailed comments on how it generally works.

 With this change, we need to provide translation table for base32hex
 and base16 encodings (because they are not included in boost).
 baseXX_from_binary and binary_from_baseXX define these tables.  They
 should be a pretty straightforward variation of the boost counterpart:
 boost/archive/iterators/base64_from_binary.hpp and
 boost/archive/iterators/binary_from_base64.hpp.  So, for review, I
 believe we can simply perform sanity checks on the table definition,
 rather than checking the tricky template definitions.

 Other notes about base-XX changes:
   - I've also updated base32hex and base16 unit tests.
   - With this change, the original base32.cc and hex.cc were removed.
   - I introduced a generic exception class defined in exceptions.h for
   errors in the base-XX processing, rather than the overly specialized
   BaseXX exception classes.

 Other cleanup: in this change I made a separate subdirectory 'util'
 under lib/dns for internal utility tools used in libdns++ internally,
 and move existing source files that fall into this category to this
 new directory.  baseXX files were also moved to util.

 Proposed !ChangeLog entry:
 {{{
   75.?  [bug]           jinmei
         lib/dns: Fixed miscellaneous bugs in the base32 (hex) and hex
         (base16) implementation, including incorrect padding handling,
         parser failure in decoding with a SunStudio build, missing
         validation on the length of encoded hex string.  Test cases were
         more detailed to identify these bugs and confirm the fix.  Also
         renamed the incorrect term of "base32" to "base32hex".  This
         changed the API, but they are not intended to be used outside
         libdns++, so we don't consider it a backward incompatible change.
         (Trac #256, rTBD)
 }}}

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


More information about the bind10-tickets mailing list