Skip to content

Commit

Permalink
atob() clones the output more than necessary
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=258789
rdar://111993365

Reviewed by David Kilzer.

Right now several users of `base64Decode` take the `Vector` returned and immediately convert it to a `String`. This
conversion requires a copy as the `Vector` returned by `base64Decode` uses `VectorBufferMalloc` but `String` requires
that the Vector backing store is malloced with `StringImplMalloc`. This patch simply adds a `base64DecodeToString` that
calls `base64DecodeInternal` with a new template parameter telling `base64DecodeInternal` to use `StringImplMalloc`. Thus,
avoiding the extra string copy.

* Source/JavaScriptCore/jsc.cpp:
(JSC_DEFINE_HOST_FUNCTION):
* Source/WTF/wtf/text/Base64.cpp:
(WTF::base64DecodeInternal):
(WTF::base64DecodeToString):
* Source/WTF/wtf/text/Base64.h:
* Source/WTF/wtf/text/WTFString.h:
* Source/WebCore/page/Base64Utilities.cpp:
(WebCore::Base64Utilities::atob):
* Source/WebCore/page/Page.cpp:
(WebCore::Page::userStyleSheetLocationChanged):

Canonical link: https://commits.webkit.org/266016@main
  • Loading branch information
kmiller68 committed Jul 13, 2023
1 parent 0e3ae70 commit 985e100
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 14 deletions.
1 change: 1 addition & 0 deletions JSTests/stress/atob-btoa.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,4 @@ assert(atob(btoa(null)) === "null");
assert(atob(btoa(undefined)) === "undefined");
shouldThrow(() => { btoa("嗨"); }, "Error: Invalid character in argument for btoa.");
shouldThrow(() => { btoa(); }, "Error: Missing input for btoa.");
assert(atob("") === "");
6 changes: 3 additions & 3 deletions Source/JavaScriptCore/jsc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1408,11 +1408,11 @@ JSC_DEFINE_HOST_FUNCTION(functionAtob, (JSGlobalObject* globalObject, CallFrame*
if (encodedString.isNull())
return JSValue::encode(jsEmptyString(vm));

auto decodedData = base64Decode(encodedString, Base64DecodeMode::DefaultValidatePaddingAndIgnoreWhitespace);
if (!decodedData)
auto decodedString = base64DecodeToString(encodedString, Base64DecodeMode::DefaultValidatePaddingAndIgnoreWhitespace);
if (decodedString.isNull())
return JSValue::encode(throwException(globalObject, scope, createError(globalObject, "Invalid character in argument for atob."_s)));

return JSValue::encode(jsString(vm, String(decodedData->data(), decodedData->size())));
return JSValue::encode(jsString(vm, decodedString));
}

JSC_DEFINE_HOST_FUNCTION(functionBtoa, (JSGlobalObject* globalObject, CallFrame* callFrame))
Expand Down
23 changes: 19 additions & 4 deletions Source/WTF/wtf/text/Base64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,16 @@ String base64EncodeToStringReturnNullIfOverflow(std::span<const std::byte> input
return tryMakeString(base64Encoded(input, mode));
}

template<typename T> static std::optional<Vector<uint8_t>> base64DecodeInternal(std::span<const T> inputDataBuffer, Base64DecodeMode mode)
template<typename T, typename Malloc = VectorBufferMalloc>
static std::optional<Vector<uint8_t, 0, CrashOnOverflow, 16, Malloc>> base64DecodeInternal(std::span<const T> inputDataBuffer, Base64DecodeMode mode)
{
if (!inputDataBuffer.size())
return Vector<uint8_t> { };
return Vector<uint8_t, 0, CrashOnOverflow, 16, Malloc> { };

auto decodeMap = (mode == Base64DecodeMode::URL) ? base64URLDecMap : base64DecMap;
auto validatePadding = mode == Base64DecodeMode::DefaultValidatePadding || mode == Base64DecodeMode::DefaultValidatePaddingAndIgnoreWhitespace;

Vector<uint8_t> destination(inputDataBuffer.size());
Vector<uint8_t, 0, CrashOnOverflow, 16, Malloc> destination(inputDataBuffer.size());

unsigned equalsSignCount = 0;
unsigned destinationLength = 0;
Expand Down Expand Up @@ -212,7 +213,7 @@ template<typename T> static std::optional<Vector<uint8_t>> base64DecodeInternal(
if (!destinationLength) {
if (equalsSignCount)
return std::nullopt;
return Vector<uint8_t> { };
return Vector<uint8_t, 0, CrashOnOverflow, 16, Malloc> { };
}

// The should be no padding if length is a multiple of 4.
Expand Down Expand Up @@ -267,4 +268,18 @@ std::optional<Vector<uint8_t>> base64Decode(StringView input, Base64DecodeMode m
return base64DecodeInternal(std::span(input.characters16(), length), mode);
}

String base64DecodeToString(StringView input, Base64DecodeMode mode)\
{
auto toString = [&] (auto optionalBuffer) {
if (!optionalBuffer)
return nullString();
return String::adopt(WTFMove(*optionalBuffer));
};

unsigned length = input.length();
if (!length || input.is8Bit())
return toString(base64DecodeInternal<LChar, StringImplMalloc>(std::span(input.characters8(), length), mode));
return toString(base64DecodeInternal<UChar, StringImplMalloc>(std::span(input.characters16(), length), mode));
}

} // namespace WTF
2 changes: 2 additions & 0 deletions Source/WTF/wtf/text/Base64.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ std::optional<Vector<uint8_t>> base64Decode(std::span<const uint8_t>, Base64Deco
std::optional<Vector<uint8_t>> base64Decode(std::span<const char>, Base64DecodeMode = Base64DecodeMode::DefaultIgnorePadding);
std::optional<Vector<uint8_t>> base64Decode(const void*, unsigned, Base64DecodeMode = Base64DecodeMode::DefaultIgnorePadding);

WTF_EXPORT_PRIVATE String base64DecodeToString(StringView, Base64DecodeMode = Base64DecodeMode::DefaultIgnorePadding);

// All the same functions modified for base64url, as defined in RFC 4648.
// This format uses '-' and '_' instead of '+' and '/' respectively.

Expand Down
4 changes: 2 additions & 2 deletions Source/WTF/wtf/text/WTFString.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ class String final {

static String adopt(StringBuffer<LChar>&& buffer) { return StringImpl::adopt(WTFMove(buffer)); }
static String adopt(StringBuffer<UChar>&& buffer) { return StringImpl::adopt(WTFMove(buffer)); }
template<typename CharacterType, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
static String adopt(Vector<CharacterType, inlineCapacity, OverflowHandler, minCapacity>&& vector) { return StringImpl::adopt(WTFMove(vector)); }
template<typename CharacterType, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity, typename Malloc>
static String adopt(Vector<CharacterType, inlineCapacity, OverflowHandler, minCapacity, Malloc>&& vector) { return StringImpl::adopt(WTFMove(vector)); }

bool isNull() const { return !m_impl; }
bool isEmpty() const { return !m_impl || m_impl->isEmpty(); }
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/page/Base64Utilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ ExceptionOr<String> Base64Utilities::atob(const String& encodedString)
if (encodedString.isNull())
return String();

auto decodedData = base64Decode(encodedString, Base64DecodeMode::DefaultValidatePaddingAndIgnoreWhitespace);
if (!decodedData)
auto decodedData = base64DecodeToString(encodedString, Base64DecodeMode::DefaultValidatePaddingAndIgnoreWhitespace);
if (decodedData.isNull())
return Exception { InvalidCharacterError };

return String(decodedData->data(), decodedData->size());
return decodedData;
}

}
5 changes: 3 additions & 2 deletions Source/WebCore/page/Page.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2270,8 +2270,9 @@ void Page::userStyleSheetLocationChanged()
if (url.protocolIsData() && url.string().startsWith("data:text/css;charset=utf-8;base64,"_s)) {
m_didLoadUserStyleSheet = true;

if (auto styleSheetAsUTF8 = base64Decode(PAL::decodeURLEscapeSequences(StringView(url.string()).substring(35)), Base64DecodeMode::DefaultValidatePaddingAndIgnoreWhitespace))
m_userStyleSheet = String::fromUTF8(styleSheetAsUTF8->data(), styleSheetAsUTF8->size());
String styleSheetAsBase64 = base64DecodeToString(PAL::decodeURLEscapeSequences(StringView(url.string()).substring(35)), Base64DecodeMode::DefaultValidatePaddingAndIgnoreWhitespace);
if (!styleSheetAsBase64.isNull())
m_userStyleSheet = styleSheetAsBase64;
}

forEachDocument([] (Document& document) {
Expand Down

0 comments on commit 985e100

Please sign in to comment.