Add base64 decoder and change existing base64 encoder.

sdk/lib/crypto/crypto_utils.dart

Index: sdk/lib/crypto/crypto_utils.dart
diff --git a/sdk/lib/crypto/crypto_utils.dart b/sdk/lib/crypto/crypto_utils.dart
index 4c09dff4b00333c7044870a7d3078b5aae78d29a..2b61b0b7eccaa2acc0cbb40a68b24049545c81bc 100644
--- a/sdk/lib/crypto/crypto_utils.dart
+++ b/sdk/lib/crypto/crypto_utils.dart
@@ -4,25 +4,6 @@
 
 part of dart.crypto;
 
-class _LineWrappingStringBuffer {
-  _LineWrappingStringBuffer(int this._lineLength) : _sb = new StringBuffer();
-
-  void write(String s) {
-    if (_lineLength != null && _currentLineLength == _lineLength) {
-      _sb.write('\r\n');
-      _currentLineLength = 0;
-    }
-    _sb.write(s);
-    _currentLineLength++;
-  }
-
-  String toString() => _sb.toString();
-
-  int _lineLength;
-  StringBuffer _sb;
-  int _currentLineLength = 0;
-}
-
 abstract class _CryptoUtils {
   static String bytesToHex(List<int> bytes) {
     var result = new StringBuffer();
@@ -32,45 +13,133 @@ abstract class _CryptoUtils {
     return result.toString();
   }
 
-  static String bytesToBase64(List<int> bytes, [int lineLength]) {
-    final table =
-        const [ 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L',
-                'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X',
-                'Y', 'Z', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
-                'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v',
-                'w', 'x', 'y', 'z', '0', '1', '2', '3', '4', '5', '6', '7',
-                '8', '9', '+', '/' ];
-
-    var result = new _LineWrappingStringBuffer(lineLength);
-
-    // Encode all full 24-bit blocks.
-    var i = 0;
-    for (; (i + 2) < bytes.length; i += 3) {
-      var b0 = bytes[i] & 0xff;
-      var b1 = bytes[i + 1] & 0xff;
-      var b2 = bytes[i + 2] & 0xff;
-      result.write(table[b0 >> 2]);
-      result.write(table[((b0 << 4) | (b1 >> 4)) & 0x3f]);
-      result.write(table[((b1 << 2) | (b2 >> 6)) & 0x3f]);
-      result.write(table[b2 & 0x3f]);
+  static final int PAD = '='.codeUnitAt(0);
+  static final int CR = '\r'.codeUnitAt(0);
+  static final int LF = '\n'.codeUnitAt(0);
+
+  static final _encodeTable =
+      "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/".codeUnits;

[Lasse Reichstein Nielsen, 2013/03/21 14:06:38]

Consider not extracting code units here, and just call 'codeUnitAt(i)' where you
use it. The codeUnits object is just a wrapper around the string, and it does
exactly the same thing, just with more overhead.

Do add a type to the declaration (String or List<int> depending on the above).

[mdakin1, 2013/03/21 15:27:15]

Converted to String. But I am curious, I thought "codeUnits" would return a copy
of strings characters in List<int> form,  similar to java's toCharArray method.

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

It just returns a read-only list view of the code units, backed by the string.
Since the string is immutable, it is cheaper (in memory) to just use the string
instead of creating a new list. 
If you really need a new list (e.g., because you want change it), you can call
toList on that list.

[mdakin1, 2013/03/25 17:05:02]

Acknowledged.

+
+  static final _encodeTableUrlSafe =
+      "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_".codeUnits;

[Lasse Reichstein Nielsen, 2013/03/21 14:06:38]

Ditto.

[mdakin1, 2013/03/21 15:27:15]

Done.

+
+  static const List<int> _decodeTable =

[Lasse Reichstein Nielsen, 2013/03/21 14:06:38]

Add a comment, it doesn't have to be dartdoc, to explain the
-2 and -1. 
I can guess it, but it's still better to not have to guess.

[mdakin1, 2013/03/21 15:27:15]

Done.

+      const [ -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -1, -2, -2, -1, -2, -2,
+              -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2,
+              -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, 62, -2, 62, -2, 63,
+              52, 53, 54, 55, 56, 57, 58, 59, 60, 61, -2, -2, -2, 0, -2, -2,
+              -2, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,

[Lasse Reichstein Nielsen, 2013/03/21 14:06:38]

Consider padding with spaces here so every number has the same size (and yes, it
requires re-breaking every line after here :)

[mdakin1, 2013/03/21 15:27:15]

Done.

+              17, 18, 19, 20, 21, 22, 23, 24, 25, -2, -2, -2, -2, 63, -2, 26,
+              27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42,
+              43, 44, 45, 46, 47, 48, 49, 50, 51, -2, -2, -2, -2, -2, -2, -2,
+              -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2,
+              -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2,
+              -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2,
+              -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2,
+              -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2,
+              -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2,
+              -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2,
+              -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2 ];
+
+  static String bytesToBase64(List<int> bytes, [bool urlSafe, bool addLineSeparator]) {

[Lasse Reichstein Nielsen, 2013/03/21 14:06:38]

Long line needs to be broken.

[mdakin1, 2013/03/21 15:27:15]

Done.

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

Default for urlSafe and addLineSeparator is null.
It will probably always be called with non-null values, but maybe make the
default false in any case.

[mdakin1, 2013/03/25 17:05:02]

Done.

+    int len = bytes != null ? bytes.length : 0;

[Lasse Reichstein Nielsen, 2013/03/21 14:06:38]

Consider not accepting null. It may just be hiding an error.
The caller can check for null if he knows it's possible and just not call at all
in that case.

[mdakin1, 2013/03/21 15:27:15]

I removed the null check, but not sure how to handle null input.

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

That would make bytes.length throw, which is perfectly fine.
In general, I don't say anything if a typed parameter does not accept null, and
instead document what it means if it does accept null. Throwing the equivalent
of a null-pointer-error here will ensure that the null doesn't go any further.

[mdakin1, 2013/03/25 17:05:02]

Acknowledged. So basically, if it accepts null document it, let it throw if it
does not and got a null parameter instead.

[floitsch, 2013/03/25 17:45:37]

correct.

+    if (len == 0) {
+      return "";
     }
+    final List<int> lookup = urlSafe ? _encodeTable : _encodeTableUrlSafe;
+    // Size of 24 bit chunks.
+    final int cLen = len ~/ 3 * 3;

[Lasse Reichstein Nielsen, 2013/03/21 14:06:38]

cLen is a non-obvious abbreviated name. Consider giving it a longer name
(characterLength, chunkCount for just "len ~/ 3" or similar).

[mdakin1, 2013/03/21 15:27:15]

Done.

+    final int padLen = len - cLen;

[Lasse Reichstein Nielsen, 2013/03/21 14:06:38]

padLength.

Is this done this way because it's faster than
  int padLen = len % 3;
  int cLen = len - padLen;
?
I wouldn't be surprised if it is.

[mdakin1, 2013/03/21 15:27:15]

Done. So obvious :/

+    // Size of base output.
+    int oLen = ((len ~/ 3) * 4) + (padLen > 0 ? 4 : 0);
+    // Add extra for line separators.
+    if (addLineSeparator) {
+      oLen += (oLen - 1) ~/ 76 << 1;

[Lasse Reichstein Nielsen, 2013/03/21 14:06:38]

Parentheses around the left-hand-side of <<. Nobody remembers whether ~/ or <<
has higher priority :)

[mdakin1, 2013/03/21 15:27:15]

Done.

+    }
+    List<int> out = new List<int>(oLen);
 
-    // Deal with the last non-full block if any and add padding '='.
-    if (i == bytes.length - 1) {
-      var b0 = bytes[i] & 0xff;
-      result.write(table[b0 >> 2]);
-      result.write(table[(b0 << 4) & 0x3f]);
-      result.write('=');
-      result.write('=');
-    } else if (i == bytes.length - 2) {
-      var b0 = bytes[i] & 0xff;
-      var b1 = bytes[i + 1] & 0xff;
-      result.write(table[b0 >> 2]);
-      result.write(table[((b0 << 4) | (b1 >> 4)) & 0x3f]);
-      result.write(table[(b1 << 2) & 0x3f]);
-      result.write('=');
+    // Encode 24 bit chunks.
+    int j = 0, i = 0, cc = 0;
+    while (i < cLen) {
+      int x = bytes[i++] << 16 | bytes[i++] << 8 | bytes[i++];
+      out[j++] = lookup[x >> 18];
+      out[j++] = lookup[(x >> 12) & 0x3F];
+      out[j++] = lookup[(x >> 6)  & 0x3F];
+      out[j++] = lookup[x & 0x3f];
+      // Add optional line separator for each 76 char output.
+      if (addLineSeparator && ++cc == 19 && j < oLen - 2) {
+          out[j++] = CR;
+          out[j++] = LF;
+          cc = 0;
+      }
     }
 
-    return result.toString();
+    // If input length if not a multiple of 3, encode remaining add padding.
+    if (padLen > 0) {
+      int x = bytes[cLen] << 10  | (padLen == 2 ? bytes[len - 1] << 2 : 0);

[Lasse Reichstein Nielsen, 2013/03/21 14:06:38]

More parentheses! Does << bind harder than | ? Who'd have thunk?!

There's an extra space after 10. 

[mdakin1, 2013/03/21 15:27:15]

Done.

+      out[j++] = lookup[(x >> 12) & 0x3F];
+      out[j++] = lookup[(x >> 6) & 0x3F];
+      out[j++] = padLen == 2 ? lookup[x & 0x3f] : PAD;
+      out[j++] = PAD;
+    }
+
+    return new String.fromCharCodes(out);
   }
+
+  static List<int> base64StringToBytes(String input, [bool ignoreErrors]) {
+    int len = input != null ? input.length : 0;

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

Again, feel free to throw on null. 

[mdakin1, 2013/03/25 17:05:02]

Done.

+    if (len == 0) {
+      return new List<int>(0);
+    }
+
+    // Count \r \n and  illegal chars,

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

// Count '\r', '\n' and illegal characters.

Makes it more obvious that \r and \n are referring to characters. Start with
capital letter on next line.

[mdakin1, 2013/03/25 17:05:02]

Done.

+    // for illegal chars, if [ignoreErrors] is false, throw an exception.
+    int extrasLen = 0;
+    for (int i = 0; i < len; i++) {
+      int c = _decodeTable[input.codeUnitAt(i)];
+      if (c < 0) {
+          if (ignoreErrors) {
+            extrasLen++;

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

So if c is -1, we increment extrasLen if we ignoreErrors, but not if we don't,
but we don't throw in either case. That doesn't look right.

[mdakin1, 2013/03/25 17:05:02]

Done.

+          } else if (c == -2) {
+            throw new FormatException(
+                'Invalid character ${new String.fromCharCode(input.codeUnitAt(i))}');

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

${input[i]}  should be equivalent, and shorter, which is good since the line is
too long :)

[mdakin1, 2013/03/25 17:05:02]

Done.

+          }
+      }
+    }
+
+    if ((len - extrasLen) % 4 != 0) {

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

Is this requirement documented anywhere?
Is it tested? Both with and without ignore errors :)

[mdakin1, 2013/03/25 17:05:02]

Not really documented, but in theory if you count out the \r \n and illegal
characters, remaining should be multiple of four otherwise the inner decoding
loop would throw.

However there is a case to consider, if there are 6 pad chars instead of 2, it
will still be a multiple of 4 with 4 extra pad chars but an invalid input. I can
add a check to padLength as well to catch that.

Another question is, if ignoreErrors is not specified, why is this throwing an
error, maybe we can change the parameter to "ignoreInvalidCharacters" and make
its scope narrower.

[floitsch, 2013/03/25 17:45:37]

I would be fine with "ignoreInvalidCharacters". It doesn't seem to be a commonly
used flag.

[mdakin1, 2013/03/26 09:52:17]

Done.

+      throw new FormatException('Invalid input.');

[floitsch, 2013/03/25 17:45:37]

I wonder if this shouldn't contain the input.
In any case it should probably say, why it is invalid.

[mdakin1, 2013/03/26 09:52:17]

Added input to error, explanation is a little bit dodgy but probably ok for now.

+    }
+
+    // Count pad characters, ignore illeal characters at the end.
+    int pad = 0;
+    for (int i = len; i > 1 && _decodeTable[input.codeUnitAt(--i)] <= 0;) {
+      if (input.codeUnitAt(i) == PAD) {
+        pad++;
+      }
+    }
+    int oLen = ((len - extrasLen) * 6 >> 3) - pad;

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

More parentheses. Anything near a >> needs to be parenthesized :)
A more telling name!

[mdakin1, 2013/03/25 17:05:02]

Done.

+    List<int> out = new List<int>(oLen);
+
+    for (int i = 0, o = 0; o < oLen;) {
+      int x = 0;
+      int j = 18;

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

How about:
for (int j = 4; j > 0;) {
  int c = _decodeTable[input.codeUnitAt(i++)];
  if (c >= 0) {  
    x = (x << 6) | c;
    j--;
  }
}
It's really the same, but the shift is by a constant and not dependent on the
value of c. I.e., I think it may be marginally faster :)

[mdakin1, 2013/03/25 17:05:02]

Done.
Much better. Tested it, this version is indeed faster by ~%2-3. thanks.

+      // Accumulate 4 valid 6 bit base64 characters into an int.
+      while (j >= 0) {
+        int c = _decodeTable[input.codeUnitAt(i++)];
+        if (c >= 0) {
+          x |= c << j;
+          j -= 6;
+        }
+      }
+      out[o++] = (x >> 16);
+      if (o < oLen) {
+        out[o++] = (x >> 8) & 0xFF;
+        if (o < oLen)
+          out[o++] = x & 0xFF;

[Lasse Reichstein Nielsen, 2013/03/22 09:16:56]

Always use braces around then/else branches for a multi-line if. I.e., only omit
them if it's all on one line.

[mdakin1, 2013/03/25 17:05:02]

Done.

+      }
+    }
+    return out;
+  }
+
 }