[fpc-devel] Base64 decoding stream in the FCL

[Michael Van Canneyt]

On Mon, 12 Feb 2007, Bram Kuijvenhoven wrote:

> Hi list!
> 
> The Base64 decoding stream in the Base64 unit in the FCL appears to be broken.
> In particular
> - it does not handle whitespace (or other characters not from the bas64
> alphabet), and

It was not designed to do this.

> - Read does not return the correct number of bytes read when at the end of the
> stream, and
> - the meaning of the EOF is a little bit unclear
> 
> These bugs can be circumvented by filtering out whitespace first and calling
> Size [which works!] to determine the actual stream size, but this is of course
> ugly, slower and not working on non-seekable streams.

The non-seekable streams should remain supported.

> I saw there already was a small fix after 2.0.4, but there is going on quite
> some more.
> 
> Note that the source code mentions RFC2045, which is actually about MIME, but
> also mentions a Base64 Content Transfer Encoding, but that RFC3548 is a
> preferred source, as it deals with base64 encoding in general. (URLs for RFCs:
> http://www.ietf.org/rfc/rfc3548.txt, http://www.ietf.org/rfc/rfc2045.txt.)
> 
> I'd be happy to send a patch for the TBase64Decoding class:
> - instead of supporting certain 'magical' Seek operations used in the
> implementation of TSream.GetSize, override GetSize
> - ignore characters not in base64 alphabet and not being '='
> - try to fix EOF: it should true iff all bytes have been Read

Feel free to do so, but keep in mind the non-seekable streams issue.


> 
> Any comments or suggestions are welcome!
> 
> 
> I also recommend some tests are added to test the base64 decode class. It
> seems to be rather untested or unreviewed at the moment (no offense!), as I
> saw e.g. (at line 215)
> 
>              for j := i to 3 do
>                ReadBuf[i] := ReadBuf[i + 1];
> 
> which obviously must be not what the author meant.

If you test it, please try adding tests in fpcunit format.

> I haven't looked at the base64 encoding class in depth; it might have problems
> too.

Hopefully not :-)

Michael.