[Webkit-unassigned] [Bug 21288] Implement HTML5's sandbox attribute for iframes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=21288





--- Comment #17 from Sam Weinig <sam at webkit.org>  2009-11-05 11:57:11 PDT ---
(From update of attachment 42473)
> -String Document::cookie() const
> +String Document::cookie(ExceptionCode &ec) const

Nit: the & should go next to ExceptionCode, not ec.

>                   attribute [ConvertNullToNullString] DOMString cookie
> -                     /*setter raises (DOMException)*/;
> +                     getter raises (DOMException),
> +                     setter raises (/*DOMException*/);

The "setter raises (/*DOMException*/)" is wrong here and should be removed.


> Index: WebCore/html/HTMLFrameOwnerElement.h
> ===================================================================
> --- WebCore/html/HTMLFrameOwnerElement.h	(revision 50452)
> +++ WebCore/html/HTMLFrameOwnerElement.h	(working copy)
> @@ -32,6 +32,8 @@ class Frame;
>  class SVGDocument;
>  #endif
>  
> +typedef uint8_t SandboxFlags;
> +
>  class HTMLFrameOwnerElement : public HTMLElement {
>  public:
>      virtual ~HTMLFrameOwnerElement();
> @@ -46,17 +48,36 @@ public:
>  
>      virtual ScrollbarMode scrollingMode() const { return ScrollbarAuto; }
>  
> +    static const SandboxFlags SandboxFlagsNone = 0;
> +    static const SandboxFlags SandboxFlagsAll = 0x1f;
> +    static const SandboxFlags SandboxFlagsNavigation = 0x10;
> +    static const SandboxFlags SandboxFlagsPlugins = 0x08;
> +    static const SandboxFlags SandboxFlagsOrigin = 0x04;
> +    static const SandboxFlags SandboxFlagsForms = 0x02;
> +    static const SandboxFlags SandboxFlagsScripts = 0x01;

This seems better suited for an enum.

>  
> Index: WebCore/html/HTMLIFrameElement.cpp
> ===================================================================
> --- WebCore/html/HTMLIFrameElement.cpp	(revision 50452)
> +++ WebCore/html/HTMLIFrameElement.cpp	(working copy)
> @@ -88,6 +88,42 @@ void HTMLIFrameElement::parseMappedAttri
>          if (!attr->isNull() && !attr->value().toInt())
>              // Add a rule that nulls out our border width.
>              addCSSLength(attr, CSSPropertyBorderWidth, "0");
> +    } else if (attr->name() == sandboxAttr) {
> +        SandboxFlags newSandboxFlagsDecl;
> +        // Parse the unordered set of unique space-separated tokens.
> +        if (attr->isNull())
> +            newSandboxFlagsDecl = SandboxFlagsNone;
> +        else {
> +            newSandboxFlagsDecl = SandboxFlagsAll;
> +            const UChar* characters = attr->value().characters();
> +            unsigned length = attr->value().length();
> +            unsigned start = 0;
> +            while (true) {
> +                while (start < length && isIFrameSandboxStringWhitespace(characters[start]))
> +                    ++start;
> +                if (start >= length)
> +                    break;
> +                unsigned end = start + 1;
> +                while (end < length && !isIFrameSandboxStringWhitespace(characters[end]))
> +                    ++end;
> +
> +                // Turn off the corresponding sandbox flag if it's set as "allowed".
> +                AtomicString sandboxToken = AtomicString(characters + start, end - start);
> +                if (equalIgnoringCase(sandboxToken, "allow-same-origin"))
> +                    newSandboxFlagsDecl &= ~SandboxFlagsOrigin;
> +                else if (equalIgnoringCase(sandboxToken, "allow-forms"))
> +                    newSandboxFlagsDecl &= ~SandboxFlagsForms;
> +                else if (equalIgnoringCase(sandboxToken, "allow-scripts"))
> +                    newSandboxFlagsDecl &= ~SandboxFlagsScripts;
> +
> +                start = end + 1;
> +            }

Please split this parsing out into its own function?

> Index: WebCore/html/HTMLIFrameElement.h
> ===================================================================
> --- WebCore/html/HTMLIFrameElement.h	(revision 50452)
> +++ WebCore/html/HTMLIFrameElement.h	(working copy)
> @@ -54,6 +54,11 @@ private:
>      AtomicString m_name;
>  };
>  
> +inline bool isIFrameSandboxStringWhitespace(UChar c)
> +{
> +    return c == ' ' || c == '\r' || c == '\n' || c == '\t' || c == '\f';
> +}


Does this need to be in the header?  It seems to be only used by one .cpp file.


> Index: WebCore/inspector/InspectorController.cpp
> ===================================================================
> --- WebCore/inspector/InspectorController.cpp	(revision 50452)
> +++ WebCore/inspector/InspectorController.cpp	(working copy)
> @@ -1181,10 +1181,15 @@ void InspectorController::getCookies(lon
>              Vector<Cookie> docCookiesList;
>              rawCookiesImplemented = getRawCookies(document, document->cookieURL(), docCookiesList);
>              
> -            if (!rawCookiesImplemented)
> +            if (!rawCookiesImplemented) {
>                  // FIXME: We need duplication checking for the String representation of cookies.
> -                stringCookiesList += document->cookie();
> -            else {
> +
> +                ExceptionCode ec = 0;
> +                // Ignore exception thrown by cookie() below -- the only case an exception is
> +                // thrown here is when the iframe is sandboxed, which will never happen here
> +                // because "doc" is the document of the main frame of the page.
> +                stringCookiesList += document->cookie(ec);

Please add an ASSERT that an exception was not thrown.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.