Bug 146494

Summary: REGRESSION (185319): Reproducible crash in WebHistoryItem launching FluidApp
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, beidson, darin
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: All   
Attachments:
Description Flags
Patch v1 darin: review+

Description Brady Eidson 2015-06-30 22:35:17 PDT
REGRESSION (185319): Reproducible crash in WebHistoryItem launching FluidApp

Steps to reproduce:
1 - Download FluidApp from http://fluidapp.com
2 - Create an app from a website (I used webkit.org)
3 - Launch that WebApp

Crash:
1   0x1034112e0 WTFCrash
2   0x104dbe7d9 WTF::CrashOnOverflow::crash()
3   0x104dbe7a9 WTF::CrashOnOverflow::overflowed()
4   0x104dbf471 WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul>::at(unsigned long)
5   0x104e51edd WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul>::operator[](unsigned long)
6   0x104eb909b -[WebHistoryItem initFromDictionaryRepresentation:]
7   0x104eb0897 -[WebHistoryPrivate loadHistoryGutsFromURL:savedItemsCount:collectDiscardedItemsInto:error:]
8   0x104eb0a8c -[WebHistoryPrivate loadFromURL:collectDiscardedItemsInto:error:]
9   0x104eb17db -[WebHistory loadFromURL:error:]
10  0x100019c11 -[FUHistoryController setUpHistory]
11  0x1000198ca -[FUHistoryController init]
12  0x100019807 +[FUHistoryController instance]
13  0x10000e293 -[FUApplication finishLaunching]
14  0x7fff8b010a21 -[NSApplication run]
15  0x7fff8af8d354 NSApplicationMain
16  0x1000017c4 start

The code in question:
        auto redirectURLsVector = std::make_unique<Vector<String>>();

        for (NSUInteger i = 0; i < size; ++i) {
            id redirectURL = [redirectURLs objectAtIndex:i];
            if (![redirectURL isKindOfClass:[NSString class]])
                continue;

            (*redirectURLsVector)[i] = (NSString *)redirectURL;
        }

We create a Vector of size 0 then immediately reference the first element.

That's obviously wrong.

In radar as rdar://problem/21598293
Comment 1 Brady Eidson 2015-06-30 22:38:01 PDT
Created attachment 255899 [details]
Patch v1
Comment 2 Brady Eidson 2015-06-30 22:39:01 PDT
Was introduced in http://trac.webkit.org/changeset/185319
Comment 3 Darin Adler 2015-07-01 09:25:15 PDT
Comment on attachment 255899 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=255899&action=review

> Source/WebKit/mac/History/WebHistoryItem.mm:363
>          NSUInteger size = [redirectURLs count];

After making the thing below a modern Objective-C for loop this can be called right in the reserveInitialCapacity call below.

> Source/WebKit/mac/History/WebHistoryItem.mm:365
> +        redirectURLsVector->reserveCapacity(size);

This should be reserveInitialCapacity.

> Source/WebKit/mac/History/WebHistoryItem.mm:368
>          for (NSUInteger i = 0; i < size; ++i) {
>              id redirectURL = [redirectURLs objectAtIndex:i];

This should be a modern Objective-C for loop:

    for (id redirectURL in redirectURLs) {

> Source/WebKit/mac/History/WebHistoryItem.mm:372
> +            redirectURLsVector->append((NSString *)redirectURL);

This should be uncheckedAppend.
Comment 4 Brady Eidson 2015-07-01 10:14:28 PDT
https://trac.webkit.org/changeset/186179