Bug 129081 - ASSERT in FrameLoader::shouldInterruptLoadForXFrameOptions
Summary: ASSERT in FrameLoader::shouldInterruptLoadForXFrameOptions
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Pratik Solanki
Keywords: InRadar
Depends on:
Reported: 2014-02-19 21:36 PST by Pratik Solanki
Modified: 2014-02-20 10:18 PST (History)
6 users (show)

See Also:

Patch (1.61 KB, patch)
2014-02-19 21:41 PST, Pratik Solanki
ap: review+
ap: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pratik Solanki 2014-02-19 21:36:58 PST
Malformed servers that don't pass a value in X-Frame-Options can trigger this assert. e.g. on <http://mweb.cbssports.com/ncaab/eye-on-college-basketball/24436548/night-court-marcus-foster-stars-xavier-and-iowa-get-key-wins> there's a load of <https://vine.co/v/MWmZ7L31emm/card> in an iframe. The headers for that URL are

$ curl -I https://vine.co/v/MWmZ7L31emm/card
HTTP/1.1 200 OK
Cache-Control: max-age=1800
Content-Type: text/html; charset=utf-8
Date: Thu, 20 Feb 2014 05:36:30 GMT
Strict-Transport-Security: max-age=631138519
X-Content-Type-Options: nosniff
X-XSS-Protection: 1; mode=block
Connection: keep-alive

The empty X-Frame-Options triggers the assert.
Comment 1 Pratik Solanki 2014-02-19 21:37:15 PST
Comment 2 Pratik Solanki 2014-02-19 21:41:13 PST
Created attachment 224705 [details]
Comment 3 Alexey Proskuryakov 2014-02-19 23:03:57 PST
Comment on attachment 224705 [details]

This breaks gcc build. I think that we need an ASSERT_NOT_REACHED() and return at the end.

/mnt/eflews/webkit/WebKit/Source/WebCore/loader/FrameLoader.cpp: In member function 'bool WebCore::FrameLoader::shouldInterruptLoadForXFrameOptions(const WTF::String&, const WebCore::URL&, long unsigned int)':
/mnt/eflews/webkit/WebKit/Source/WebCore/loader/FrameLoader.cpp:3074:1: error: control reaches end of non-void function [-Werror=return-type]
cc1plus: all warnings being treated as errors
Comment 4 Pratik Solanki 2014-02-20 10:16:47 PST
(In reply to comment #3)
> (From update of attachment 224705 [details])
> This breaks gcc build. I think that we need an ASSERT_NOT_REACHED() and return at the end.

Ok. Will add that and land the patch. Thanks for the review.
Comment 5 Pratik Solanki 2014-02-20 10:18:38 PST
Committed r164435: <http://trac.webkit.org/changeset/164435>