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