Deck access control (a very important security feature in the WML world) is broken. Affects manual-tests/wml/deck-access-control.wml.
Created attachment 33943 [details] Initial patch
Comment on attachment 33943 [details] Initial patch Does WML execute script? Is it possible to change these values? If so, setting to an invalid value will not clear domain or path since you if (value.isEmpty()) return before setting. Is that intentional behavior? What does this do, and why? if (!initialized) { 58 if (WMLPageState* wmlPageState = wmlPageStateForDocument(this)) { 59 if (Page* page = wmlPageState->page()) 60 page->goBack(); 61 } 62 } Why not use early return there? typo: 71 // Remember that we'e successfully entered the deck Seems: bool initialized is the wrong name. It's accessAllowed or something like that. Change seems large. Too large for my little brain at this moment. maybe we could/should add the layout tests first... r- hoping that we could split this to make it easier to review.
(In reply to comment #2) > (From update of attachment 33943 [details]) > Does WML execute script? Is it possible to change these values? If so, > setting to an invalid value will not clear domain or path since you if > (value.isEmpty()) return before setting. Is that intentional behavior? No, WML doesn't have any JS support at all. WMLScript support is not desired to be implemented. So it's all static. > What does this do, and why? This is deck-access control. A deck is a document, a card is contained within a specific deck. Suppose Card 1 of Deck A tries to enter Card 1 of Deck B. Deck B contains "<access domain='not-existant.com'...>", then Deck A is not allowed to enter Deck B. Deck B immediately goes back to the invoking card, and an error is displayed on the original page, that it failed to enter the target deck (task-execution-failure). This is specified WML behaviour. > if (!initialized) { > 58 if (WMLPageState* wmlPageState = wmlPageStateForDocument(this)) { > 59 if (Page* page = wmlPageState->page()) > 60 page->goBack(); > 61 } > 62 } > Why not use early return there? Fixed. > > typo: > 71 // Remember that we'e successfully entered the deck > > Seems: > bool initialized > is the wrong name. It's accessAllowed or something like that. Fixed. Uploading a slighly revised version soon, which also fixes a bug regarding intrinsic event execution upon task-execution-failure.
Created attachment 34433 [details] Updated patch
Created attachment 34434 [details] Updated patch - incorporating Georges comments
Sending LayoutTests/ChangeLog Adding LayoutTests/http/tests/wml/access-target-domain-deny-expected.txt Adding LayoutTests/http/tests/wml/access-target-domain-deny.html Adding LayoutTests/http/tests/wml/access-target-expected.txt Adding LayoutTests/http/tests/wml/access-target-path-deny-expected.txt Adding LayoutTests/http/tests/wml/access-target-path-deny.html Adding LayoutTests/http/tests/wml/access-target.html Adding LayoutTests/http/tests/wml/resources/access-target-domain-deny.js Adding LayoutTests/http/tests/wml/resources/access-target-domain-deny.wml Adding LayoutTests/http/tests/wml/resources/access-target-path-deny.js Adding LayoutTests/http/tests/wml/resources/access-target-path-deny.wml Adding LayoutTests/http/tests/wml/resources/access-target.js Adding LayoutTests/http/tests/wml/resources/access-target.wml Adding LayoutTests/http/tests/wml/resources/locked-deck.wml Adding LayoutTests/http/tests/wml/resources/unreachable-domain.wml Adding LayoutTests/http/tests/wml/resources/unreachable-path.wml Adding LayoutTests/wml/access-target-deny-expected.txt Adding LayoutTests/wml/access-target-deny.html Adding LayoutTests/wml/access-target-expected.txt Adding LayoutTests/wml/access-target.html Adding LayoutTests/wml/resources/access-target-deny.js Adding LayoutTests/wml/resources/access-target-deny.wml Adding LayoutTests/wml/resources/access-target.js Adding LayoutTests/wml/resources/access-target.wml Adding LayoutTests/wml/resources/locked-deck.wml Sending LayoutTests/wml/variable-reference-invalid-character-expected.txt Sending WebCore/ChangeLog Sending WebCore/inspector/ConsoleMessage.h Sending WebCore/inspector/InspectorController.h Sending WebCore/loader/FrameLoader.cpp Sending WebCore/loader/FrameLoaderTypes.h Sending WebCore/page/Console.cpp Sending WebCore/page/Console.h Sending WebCore/page/Console.idl Sending WebCore/wml/WMLAccessElement.cpp Sending WebCore/wml/WMLAccessElement.h Sending WebCore/wml/WMLCardElement.cpp Sending WebCore/wml/WMLDocument.cpp Sending WebCore/wml/WMLErrorHandling.cpp Sending WebCore/wml/WMLPageState.cpp Sending WebCore/wml/WMLPageState.h Sending WebKit/mac/ChangeLog Sending WebKit/mac/WebView/WebFramePrivate.h Sending WebKit/win/ChangeLog Sending WebKit/win/Interfaces/IWebFramePrivate.idl Transmitting file data ............................................. Committed revision 46966. http://trac.webkit.org/changeset/46966
Guys, is this console.lastWMLErrorMessage web-facing? Or is it only used in the layout tests? If latter, let us pass it via the layoutTestController instead. The reason I ask is that I saw some nasty ifdef(INSPECTOR) checks within WebCore/inspector/ConsoleMessage.cpp, while my assumption was that ConsoleMessage.cpp was inspector's property (and hence resides in the inspector folder). I'd also like to remove InspectorController::consoleMessages() as a whole so that other clients would not use the interface. Your code seems to be the only client.