buffers: honor length argument in base64 decoder

Honor the length argument in `buf.write(s, 0, buf.length, 'base64')`. Before
this commit, the length argument was ignored. The decoder would keep writing
until it hit the end of the buffer. Since most buffers in Node are slices of
a parent buffer (the slab), this bug would overwrite the content of adjacent
buffers.

The bug is trivially demonstrated with the following test case:

    var assert = require('assert');
    var a = Buffer(3);
    var b = Buffer('xxx');
    a.write('aaaaaaaa', 'base64');
    assert.equal(b.toString(), 'xxx');

This commit coincidentally also fixes a bug where Buffer._charsWritten was not
updated for zero length buffers.

src/node_buffer.cc

@@ -584,17 +584,6 @@ Handle<Value> Buffer::AsciiWrite(const Arguments &args) {
 Handle<Value> Buffer::Base64Write(const Arguments &args) {
   HandleScope scope;
 
-  assert(unbase64('/') == 63);
-  assert(unbase64('+') == 62);
-  assert(unbase64('T') == 19);
-  assert(unbase64('Z') == 25);
-  assert(unbase64('t') == 45);
-  assert(unbase64('z') == 51);
-
-  assert(unbase64(' ') == -2);
-  assert(unbase64('\n') == -2);
-  assert(unbase64('\r') == -2);
-
   Buffer *buffer = ObjectWrap::Unwrap<Buffer>(args.This());
 
   if (!args[0]->IsString()) {
@@ -604,67 +593,52 @@ Handle<Value> Buffer::Base64Write(const Arguments &args) {
 
   String::AsciiValue s(args[0]->ToString());
   size_t offset = args[1]->Int32Value();
-
-  // handle zero-length buffers graciously
-  if (offset == 0 && buffer->length_ == 0) {
-    return scope.Close(Integer::New(0));
-  }
+  size_t max_length = args[2]->IsUndefined() ? buffer->length_ - offset
+                                             : args[2]->Uint32Value();
+  max_length = MIN(s.length(), MIN(buffer->length_ - offset, max_length));
 
   if (offset >= buffer->length_) {
     return ThrowException(Exception::TypeError(String::New(
             "Offset is out of bounds")));
   }
 
-  const size_t size = base64_decoded_size(*s, s.length());
-  if (size > buffer->length_ - offset) {
-    // throw exception, don't silently truncate
-    return ThrowException(Exception::TypeError(String::New(
-            "Buffer too small")));
-  }
-
   char a, b, c, d;
   char* start = buffer->data_ + offset;
   char* dst = start;
-  const char *src = *s;
-  const char *const srcEnd = src + s.length();
+  char* const dstEnd = dst + max_length;
+  const char* src = *s;
+  const char* const srcEnd = src + s.length();
 
-  while (src < srcEnd) {
+  while (src < srcEnd && dst < dstEnd) {
     int remaining = srcEnd - src;
 
-    while (unbase64(*src) < 0 && src < srcEnd) {
-      src++;
-      remaining--;
-    }
+    while (unbase64(*src) < 0 && src < srcEnd) src++, remaining--;
     if (remaining == 0 || *src == '=') break;
     a = unbase64(*src++);
 
-    while (unbase64(*src) < 0 && src < srcEnd) {
-      src++;
-      remaining--;
-    }
+    while (unbase64(*src) < 0 && src < srcEnd) src++, remaining--;
     if (remaining <= 1 || *src == '=') break;
     b = unbase64(*src++);
-    *dst++ = (a << 2) | ((b & 0x30) >> 4);
 
-    while (unbase64(*src) < 0 && src < srcEnd) {
-      src++;
-      remaining--;
-    }
+    *dst++ = (a << 2) | ((b & 0x30) >> 4);
+    if (dst == dstEnd) break;
+
+    while (unbase64(*src) < 0 && src < srcEnd) src++, remaining--;
     if (remaining <= 2 || *src == '=') break;
     c = unbase64(*src++);
-    *dst++ = ((b & 0x0F) << 4) | ((c & 0x3C) >> 2);
 
-    while (unbase64(*src) < 0 && src < srcEnd) {
-      src++;
-      remaining--;
-    }
+    *dst++ = ((b & 0x0F) << 4) | ((c & 0x3C) >> 2);
+    if (dst == dstEnd) break;
+
+    while (unbase64(*src) < 0 && src < srcEnd) src++, remaining--;
     if (remaining <= 3 || *src == '=') break;
     d = unbase64(*src++);
+
     *dst++ = ((c & 0x03) << 6) | (d & 0x3F);
   }
 
   constructor_template->GetFunction()->Set(chars_written_sym,
-                                           Integer::New(s.length()));
+                                           Integer::New(dst - start));
 
   return scope.Close(Integer::New(dst - start));
 }
@@ -759,6 +733,17 @@ bool Buffer::HasInstance(v8::Handle<v8::Value> val) {
 void Buffer::Initialize(Handle<Object> target) {
   HandleScope scope;
 
+  // sanity checks
+  assert(unbase64('/') == 63);
+  assert(unbase64('+') == 62);
+  assert(unbase64('T') == 19);
+  assert(unbase64('Z') == 25);
+  assert(unbase64('t') == 45);
+  assert(unbase64('z') == 51);
+  assert(unbase64(' ') == -2);
+  assert(unbase64('\n') == -2);
+  assert(unbase64('\r') == -2);
+
   length_symbol = Persistent<String>::New(String::NewSymbol("length"));
   chars_written_sym = Persistent<String>::New(String::NewSymbol("_charsWritten"));
 

test/simple/test-buffer.js

@@ -687,7 +687,7 @@ assert.equal(Buffer._charsWritten, 9);
 buf.write('0123456789', 'binary');
 assert.equal(Buffer._charsWritten, 9);
 buf.write('123456', 'base64');
-assert.equal(Buffer._charsWritten, 6);
+assert.equal(Buffer._charsWritten, 4);
 buf.write('00010203040506070809', 'hex');
 assert.equal(Buffer._charsWritten, 18);
 
@@ -703,3 +703,14 @@ assert.equal(Buffer({length: 'BAM'}).length, 0);
 // Make sure that strings are not coerced to numbers.
 assert.equal(Buffer('99').length, 2);
 assert.equal(Buffer('13.37').length, 5);
+
+// Ensure that the length argument is respected.
+'ascii utf8 hex base64 binary'.split(' ').forEach(function(enc) {
+  assert.equal(Buffer(1).write('aaaaaa', 0, 1, enc), 1);
+});
+
+// Regression test, guard against buffer overrun in the base64 decoder.
+var a = Buffer(3);
+var b = Buffer('xxx');
+a.write('aaaaaaaa', 'base64');
+assert.equal(b.toString(), 'xxx');