Bug 22431 – Implement <access> support in WML

• Status: RESOLVED
• Resolution: FIXED
• Component: XML

[Nikolas Zimmermann]

Support the <access> element. It just contains two attribute values that need to be propagated to WMLPageState.

[Nikolas Zimmermann]

Created attachment 25383 [details]
Initial patch

[Nikolas Zimmermann]

Created attachment 25384 [details]
Updated patch

Missed one file, updating patch.

[Eric Seidel (no email)]

So WML has it's own security model?  I'm unclear on what "calls" this would allow, once you give a certain other URL "access" to this deck.

[Nikolas Zimmermann]

Exactly, some quotes from the WML spec to clarify:

<quote>
The access element specifies access control information for the entire deck.  It is an error for a 
deck to contain more than one access element.  If a deck does not include an access element, 
access control is disabled. When access control is disabled, cards in any deck can access this deck.

A deck's domain and path attributes specify which other decks may access it.  As the 
user agent navigates from one deck to another, it performs access control checks to 
determine whether the destination deck allows access from the current deck. 
</quote>

Does this clarify the situation?

[Eric Seidel (no email)]

(In reply to comment #4)
So you can control from what referrers you're allowed to access a deck, client side!?  That seems like a lame-duck security feature that no one would ever use.  I would expect servers to serve different pages based on referrers.

[Nikolas Zimmermann]

Honestly, I can't name an example what needs this. But I haven't written the actual implementation. I bet Yichao/Charles Wei may know some use cases. We only implemented things that are needed. There's even a test in manual-tests/wml/ that checks this behaviour.

I'd prefer to just get it in. It doesn't affect WebKit/HTML in any way.

[Eric Seidel (no email)]

Comment on attachment 25384 [details]
Updated patch


> +        if (Page* page = document()->page())
> +            page->wmlPageState()->setDeckAccessDomain(attr->value());

> +        if (Page* page = document()->page()) 
> +            page->wmlPageState()->setDeckAccessPath(attr->value());

I think these calls would be better named "restrictAccessToDeckToDomain" and "restrictAccessToDeckToPath"

identifying that adding a new restriction clears all others, and that this is not what domains or paths the current deck can acess, but rather, what domains and paths can acesss the deck.  The names are still not perfect, but I think they're better than what were there before.

That should probably be "containsWMLVariableReference()"
 
> +bool valueContainsVariableReference(const String& text)
> +{
> +    int start = text.find('$');
> +    if (start == -1)
> +        return false;
> +
> +    int end = start + 1;
> +    while (text[end] == '$')
> +        ++end;
> +
> +    return ((end-start) % 2) != 0;
> +}

Also, that function will do the following:
"$" true
"$$" false
"$$$" true
"$$ foobar $bar" false

I think it needs more testing...

>      enum WMLVariableEscapingMode {
>          WMLVariableEscaping_None = 0,
> -        WMLVariableEscaping_Escape = 1,
> -        WMLVariableEscaping_Unescape = 2
> +        WMLVariableEscaping_Escape,
> +        WMLVariableEscaping_Unescape
>      };

WebKit style would say those should be WMLVariableEscapingNone, no _ I think.

r- for the lack of variable testing (and thus incorrectness of said function).  I"m not sure it's really a big deal that it's wrong.  Otherwise the patch looked fine.  Please rename the functions noted to something better. :)

[Nikolas Zimmermann]

Created attachment 25447 [details]
Updated patch v2

[Nikolas Zimmermann]

Created attachment 25448 [details]
Updated patch v3

Oops, uploaded wrong patch.

[Nikolas Zimmermann]

In 'Updated patch v3' it says ' for (unsigned i = startPos; i < length; ++i) {' - it should read 'startPos + 1' - not uploading a new patch though.

[Nikolas Zimmermann]

Landed in r38733.