WindowBase64::atob() does not remove space characters from input

Source/core/frame/DOMWindowBase64.cpp

Index: Source/core/frame/DOMWindowBase64.cpp
diff --git a/Source/core/frame/DOMWindowBase64.cpp b/Source/core/frame/DOMWindowBase64.cpp
index d2c178728dfcaf44d11712c9522645cb1877ffd2..f62bc2fe5cf89e407dd8e44e20a848c76883f9ef 100644
--- a/Source/core/frame/DOMWindowBase64.cpp
+++ b/Source/core/frame/DOMWindowBase64.cpp
@@ -35,6 +35,7 @@
 
 #include "bindings/v8/ExceptionState.h"
 #include "core/dom/ExceptionCode.h"
+#include "core/html/parser/HTMLParserIdioms.h"
 #include "wtf/text/Base64.h"
 
 namespace WebCore {
@@ -63,9 +64,15 @@ String atob(void*, const String& encodedString, ExceptionState& es)
         es.throwDOMException(InvalidCharacterError, "'atob' failed: The string to be decoded contains characters outside of the Latin1 range.");
         return String();
     }
-
     Vector<char> out;
-    if (!base64Decode(encodedString, out, Base64FailOnInvalidCharacterOrExcessPadding)) {
+    bool ok;
+    if (encodedString.find(isHTMLSpace<UChar>) != NotFoundError) {

[arv (Not doing code reviews), 2013/11/04 19:54:53]

Can we change base64Decode to do the right thing so we don't have to go through
the string twice?

Maybe add another entry to the the Base64DecodePolicy?

It seems a bit strange that we have 4 different policies. Based on what I see we
only use 3 of them and Base64FailOnInvalidCharacterOrExcessPadding is only used
by atob, so maybe we can change that?

[Inactive, 2013/11/04 20:01:37]

We could indeed add an Base64DecodePolicy entry like
"FailOnOnInvalidCharacterOrExcessPaddingButAllowSpace" to do what we can in
atob(). The thing is that we would still need to go over the string twice.

Padding validation depends on the string length, and therefore assumes that
white spaces have already been removed in an earlier step. Also note that there
could be spaces in the padding itself which would be an issue as well for
validating the padding.

[tkent, 2013/11/05 02:21:56]

Ok, Implementation cost of the new flag looks high. It's ok to go ahead without
the new flag, and we should have a FIXME comment about the inefficiency.

Also, we can just do encodedString.removeCharacters() without
encodedString.find() because removeCharacters() doesn't update the StringImpl if
there are no matched characters.

+        String encodedStringWithoutSpace = encodedString.removeCharacters(isHTMLSpace<UChar>);
+        ok = base64Decode(encodedStringWithoutSpace, out, Base64FailOnInvalidCharacterOrExcessPadding);
+    } else {
+        ok = base64Decode(encodedString, out, Base64FailOnInvalidCharacterOrExcessPadding);
+    }
+    if (!ok) {
         es.throwDOMException(InvalidCharacterError, "'atob' failed: The string to be decoded is not correctly encoded.");
         return String();
     }