Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
StringImpl::copyCharacters incorrectly uses memcpy on destination
pointers that may be null https://bugs.webkit.org/show_bug.cgi?id=246260 We should assert StringImpl::copyCharacters does not discard information https://bugs.webkit.org/show_bug.cgi?id=205355 rdar://problem/100962334 Reviewed by Yusuke Suzuki. After studying the call sites of StringImpl::copyCharacters and its implementation, it is clear that many callers can pass nullptr for either the source or destination when the length passed in is 0. Since std::memcpy behavior is undefined in such cases, need to change the implementation of StringImpl::copyCharacters to not use it in those cases. In the end, decided to check for zero length even though I advised against that previously. Visiting all the call sites of StringImpl::copyCharacters led to discovering some other anomalies that are fixed with improved coding idioms, no major changes. * Source/JavaScriptCore/API/OpaqueJSString.cpp: (OpaqueJSString::characters): Updated for name change of StringView::getCharacters. * Source/JavaScriptCore/jit/ExecutableAllocationFuzz.cpp: Added include of NeverDestroyed.h. * Source/JavaScriptCore/parser/Lexer.cpp: (JSC::Lexer<T>::parseCommentDirectiveValue): Use String::make8Bit instead of StringImpl::createUninitialized, for code that is more straightforward. Also wondering why we don't need this optimization in more places. * Source/JavaScriptCore/runtime/IntlLocale.cpp: (JSC::LocaleIDBuilder::setKeywordValue): Use StringView::getCharacters. * Source/JavaScriptCore/runtime/IntlObjectInlines.h: (JSC::ListFormatInput::ListFormatInput): Use String::convertTo16Bit. This eliminates the need for m_retainedUpconvertedStrings. * Source/JavaScriptCore/runtime/JSString.cpp: (JSC::JSRopeString::resolveRopeInternal const): Tweaked the comment wording for clarity. Use StringView::getCharacters. (JSC::JSRopeString::resolveRopeInternalNoSubstring const): Ditto. (JSC::JSRopeString::resolveRopeSlowCase const): Ditto. * Source/JavaScriptCore/runtime/JSStringJoiner.cpp: (JSC::appendStringToData): Updated for name change of StringView::getCharacters. * Source/JavaScriptCore/runtime/StringPrototype.cpp: (JSC::jsSpliceSubstrings): Removed unneeded checks for zero length; adding a branch to optimize that case isn't valuable and it's not needed for correctness. * Source/JavaScriptCore/runtime/StringPrototypeInlines.h: (JSC::jsSpliceSubstringsWithSeparators): Ditto. Also use StringView::getCharacters. * Source/JavaScriptCore/tools/JSDollarVM.cpp: (JSC::functionMake16BitStringIfPossible): Use String::convertTo16Bit. * Source/JavaScriptCore/jit/ExecutableAllocationFuzz.cpp: Added include of NeverDestroyed.h. * Source/WTF/wtf/DateMath.cpp: (WTF::validateTimeZone): Use StringView::upconvertedCharacters. * Source/WTF/wtf/LogInitialization.cpp: Added include of NeverDestroyed.h. * Source/WTF/wtf/Logger.cpp: Ditto. * Source/WTF/wtf/SortedArrayMap.h: Added support for a case-sensitive version of PackedASCIILowerCodes, named PackedASCIILiteral, and for using a span of characters as the key in the "packed" case without first converting it to a StringView and then having to check is8Bit at runtime. * Source/WTF/wtf/Threading.cpp: Added include of NeverDestroyed.h. * Source/WTF/wtf/generic/RunLoopGeneric.cpp: Ditto. * Source/WTF/wtf/linux/RealTimeThreads.cpp: Ditto. * Source/WTF/wtf/text/ASCIIFastPath.h: (WTF::copyLCharsFromUCharSource): Deleted. Moved into StringImpl::copyCharacters. * Source/WTF/wtf/text/AtomString.h: Removed unneeded includes and the constructor that takes a Vector. Removed unneeded explicit implementations of copy and move constructors and assignment operators that are the same as the defaults. Added a lookUp function. * Source/WTF/wtf/text/Base64.h: Removed the overloads that take a String so we don't implicitly create a String when a StringView will do. * Source/WTF/wtf/text/StringConcatenate.h: Updated for name change of StringView::getCharacters. * Source/WTF/wtf/text/StringImpl.h: (WTF::StringImpl::copyCharacters): Used overloading instead of unnecessarily complex template programming. Made the version that uses memcpy do a zero-length check so we aren't relying on undefined behavior of memcpy. Moved the code from copyLCharsFromUCharSource here. (WTF::StringImpl::removeCharactersImpl): Removed unneeded zero-length check before calling StringImpl::copyCharacters. * Source/WTF/wtf/text/StringView.h: Renamed getCharactersWithUpconvert to just getCharacters since it can be used in many cases that don't involve upconversion. Also implemented it as a template since there seems no disadvantage to doing so. Added getCharacters8, getCharacters16, span8, and span16. * Source/WTF/wtf/text/IntegerToStringConversion.h: Added a missing include. * Source/WTF/wtf/text/WTFString.cpp: (WTF::String::charactersWithoutNullTermination const): Removed unnecessary use of size_t. (WTF::String::make8Bit): Renamed from make8BitFrom16BitSource, changed the argument type from size_t to unsigned. Changed to return an empty string rather than a null string by removing a special case for zero length. (WTF::String::convertTo16Bit): Added. Replaces make16BitFrom8BitSource, since all callers can do better by converting a String in place, and we can significantly simplify them since this efficiently does nothing if characters are already 16-bit. (WTF::fromUTF8Impl): Rewrote the "crash if size is too big" to use RELEASE_ASSERT for clarity. (WTF::String::fromUTF8WithLatin1Fallback): Added a missing RELEASE_ASSERT for the fallback case, just like the one above. * Source/WTF/wtf/text/WTFString.h: Updated for the above changes. Removed some stray semicolons. Removed the constructor that takes a Vector. * Source/WTF/wtf/text/icu/UTextProviderLatin1.cpp: (WTF::uTextLatin1Access): Removed unneeded static_cast. (WTF::uTextLatin1Extract): Ditto. (WTF::textLatin1ContextAwareMoveInPrimaryContext): Ditto. * Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp: (WebCore::ThreadableWebSocketChannelClientWrapper::setSubprotocol): Updated for name change of StringView::getCharacters. (WebCore::ThreadableWebSocketChannelClientWrapper::setExtensions): Ditto. * Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp: (WebCore::CanvasRenderingContext2DBase::normalizeSpaces): Ditto. * Source/WebCore/html/HTMLImageElement.cpp: (WebCore::HTMLImageElement::hasLazyLoadableAttributeValue): Take a StringView. * Source/WebCore/html/HTMLImageElement.h: Updated for the above. * Source/WebCore/html/LinkRelAttribute.cpp: (WebCore::LinkRelAttribute::LinkRelAttribute): Moved most initialization into the class definition. Take a StringView, and don't allocate any String as part of parsing. * Source/WebCore/html/LinkRelAttribute.h: Updated for the above. * Source/WebCore/html/parser/AtomHTMLToken.h: Updated to use String::make8Bit. Removed the overload of make8BitFrom16BitSource that took a Vector directly since this was the only place it was used, and such functions bloat the String interface with little benefit. * Source/WebCore/html/parser/HTMLMetaCharsetParser.cpp: (WebCore::extractCharset): Take StringView. (WebCore::HTMLMetaCharsetParser::processMeta): Pass StringView to encodingFromMetaAttributes. (WebCore::HTMLMetaCharsetParser::encodingFromMetaAttributes): Take StringView. (WebCore::HTMLMetaCharsetParser::checkForMetaCharset): Use AtomString::lookUp since we only need to handle existing known tags, not any that aren't already in the atom string table. * Source/WebCore/html/parser/HTMLMetaCharsetParser.h: Update encodingFromMetaAttributes to take StringView. * Source/WebCore/html/parser/HTMLPreloadScanner.cpp: (WebCore::TokenPreloadScanner::tagIdFor): Use a sorted array map instead of contructing an atom string. (WebCore::TokenPreloadScanner::StartTagScanner::processAttributes): Use AtomString::lookUp and StringView. (WebCore::TokenPreloadScanner::StartTagScanner::processImageAndScriptAttribute): Use StringView for attribute values, making strings only when the attribute name indicates we should store the value. (WebCore::TokenPreloadScanner::StartTagScanner::processAttribute): Ditto. (WebCore::TokenPreloadScanner::StartTagScanner::setURLToLoad): Take a StringView, also use a separate named function instead of a mysterious boolean argument. (WebCore::TokenPreloadScanner::StartTagScanner::setURLToLoadAllowingReplacement): More of the same. (WebCore::HTMLPreloadScanner::scan): Use AtomString::lookUp since we only need to handle existing known tags, not any that aren't already in the atom string table. * Source/WebCore/html/parser/HTMLSrcsetParser.cpp: (WebCore::bestFitSourceForImageAttributes): Take StringView. * Source/WebCore/html/parser/HTMLSrcsetParser.h: Update for the above. * Source/WebCore/html/parser/HTMLTreeBuilder.cpp: (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::makeString const): Updated for name change of String::make8Bit. * Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp: (WebCore::Layout::endsWithSoftWrapOpportunity): Use String::convertTo16Bit. * Source/WebCore/platform/glib/ApplicationGLib.cpp: Added include of NeverDestroyed.h. * Source/WebCore/platform/graphics/ComplexTextController.cpp: (WebCore::ComplexTextController::collectComplexTextRuns): Use String::convertTo16Bit. * Source/WebCore/page/PageSerializer.cpp: (WebCore::isCharsetSpecifyingNode): Handle the type of the argument in a simpler way, and use HTMLMetaElement instead of metaTag. Use StringView instead of atom strings for encodingFromMetaAttributes. * Source/WebCore/platform/graphics/StringTruncator.cpp: (WebCore::centerTruncateToBuffer): Updated for name change of StringView::getCharacters. (WebCore::rightTruncateToBuffer): Ditto. (WebCore::rightClipToCharacterBuffer): Ditto. (WebCore::rightClipToWordBuffer): Ditto. (WebCore::leftTruncateToBuffer): Ditto. (WebCore::truncateString): Ditto. * Source/WebCore/platform/graphics/TextRun.h: Added textAsString. There's one new call site that takes advantage of taking the string out. We could also just consider changing the text funtion to return a const String&, but to do that safely we'd have to make sure no callers are using substring or similar functions. * Source/WebCore/platform/graphics/gstreamer/eme/GStreamerEMEUtilities.h: (WebCore::GStreamerEMEUtilities::uuidToKeySystem): Removed the peculiar use of NeverDestroyed here. Since ASCIILiteral is just a pointer, used it as a return type instead of const ASCIILiteral&. * Source/WebCore/platform/graphics/win/FontCacheWin.cpp: (WebCore::FontCache::systemFallbackForCharacters): Updated for name change of StringView::getCharacters. Would be more elegant to add a platform-specific StringView::getWideCharactersWithNullTermination function since this pattern repeats. (WebCore::createGDIFont): Ditto. (WebCore::FontCache::getFontSelectionCapabilitiesInFamily): Ditto. * Source/WebCore/platform/mediastream/CaptureDevice.h: Added include of NeverDestroyed.h. * Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp: (WebCore::createGlobalData): Updated for name change of StringView::getCharacters. * Source/WebCore/platform/win/PasteboardWin.cpp: (WebCore::fileSystemPathFromURLOrTitle): Ditto. (WebCore::Pasteboard::writeURLToDataObject): Ditto. (WebCore::createGlobalImageFileDescriptor): Ditto. * Source/WebKitLegacy/win/DOMCoreClasses.cpp: (DOMElement::font): Updated for name change of StringView::getCharacters. Would be more elegant to add a platform-specific StringView::getWideCharacters functionsince this pattern repeats. * Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp: Added include of NeverDestroyed.h. * Source/WebKit/Platform/LogInitialization.cpp: Ditto. * Source/WebKit/Shared/API/c/WKString.cpp: (WKStringGetCharacters): Updated for name change of StringView::getCharacters. * Source/WebKit/UIProcess/Inspector/mac/WebInspectorUIProxyMac.mm: (WebKit::WebInspectorUIProxy::showSavePanel): Make the String to pass to base64Decode explicitly from the NSString. * Source/WebKit/webpushd/PushService.mm: (WebPushD::base64URLDecode): Added. Makes the String to pass explicitly from the NSString passed in. (WebPushD::base64Decode): Ditto. * Source/WebKitLegacy/win/WebKitGraphics.cpp: (CenterTruncateStringToWidth): Ditto. (RightTruncateStringToWidth): Ditto. * Source/WebKitLegacy/win/WebView.cpp: (WebView::onIMERequestReconvertString): Ditto. * Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp: Added include of NeverDestroyed.h. Canonical link: https://commits.webkit.org/255600@main
- Loading branch information