Implementing mixed content for forms posting to insecure location from secure ones

Source/core/html/HTMLFormElement.cpp

Index: Source/core/html/HTMLFormElement.cpp
diff --git a/Source/core/html/HTMLFormElement.cpp b/Source/core/html/HTMLFormElement.cpp
index 566e8c19e40f2aa73d99201f720d2b2f8ff7090b..7c562b73903a6f299a4f3f412c0c12d666f3bb50 100644
--- a/Source/core/html/HTMLFormElement.cpp
+++ b/Source/core/html/HTMLFormElement.cpp
@@ -37,6 +37,10 @@
 #include "core/events/Event.h"
 #include "core/events/GenericEventQueue.h"
 #include "core/events/ScopedEventQueue.h"
+#include "core/frame/DOMWindow.h"
+#include "core/frame/LocalFrame.h"
+#include "core/frame/UseCounter.h"
+#include "core/frame/csp/ContentSecurityPolicy.h"
 #include "core/html/HTMLCollection.h"
 #include "core/html/HTMLDialogElement.h"
 #include "core/html/HTMLImageElement.h"
@@ -46,12 +50,10 @@
 #include "core/html/forms/FormController.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
-#include "core/frame/DOMWindow.h"
-#include "core/frame/LocalFrame.h"
-#include "core/frame/UseCounter.h"
-#include "core/frame/csp/ContentSecurityPolicy.h"
+#include "core/loader/MixedContentChecker.h"
 #include "core/rendering/RenderTextControl.h"
 #include "platform/UserGestureIndicator.h"
+#include "wtf/text/AtomicString.h"
 
 using namespace std;
 
@@ -59,6 +61,15 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
+namespace {
+
+KURL getActionURL(const Document& document, const String& action)
+{
+    return action.isEmpty() ? document.url() : document.completeURL(action);
+}
+
+} // namespace
+
 HTMLFormElement::HTMLFormElement(Document& document)
     : HTMLElement(formTag, document)
 #if !ENABLE(OILPAN)
@@ -391,6 +402,12 @@ void HTMLFormElement::scheduleFormSubmission(PassRefPtr<FormSubmission> submissi
         return;
     }
 
+    KURL actionURL = getActionURL(document(), m_attributes.action());

[abarth-chromium, 2014/06/09 16:31:37]

You no longer need to call getActionURL because empty Strings have already been
weeded out on line 397.

Also, all the rest of this code gets the URL from submission->action() rather
than from m_attributes.action().  We should probably match them.

[mhm, 2014/06/09 20:20:35]

Done.

+    if (MixedContentChecker::isMixedContent(document().securityOrigin(), actionURL))
+        UseCounter::count(document(), UseCounter::MixedContentSubmittedForm);
+    if (!document().frame()->loader().mixedContentChecker()->canSubmitToInsecureForm(document().securityOrigin(), actionURL))
+        return;
+
     if (protocolIsJavaScript(submission->action())) {
         if (!document().contentSecurityPolicy()->allowFormAction(KURL(submission->action())))

[abarth-chromium, 2014/06/09 16:31:37]

It looks like the correct way to get a KURL from submission->action() is to pass
the string directly to the KURL constructor rather than to call completeURL on
it.  Please refactor this code so that we only parse the URL once in this
function.

Please consider changing the type of m_action in FormSubmission to KURL.  I
doubt there's a reason to keep it as a String.

[mhm, 2014/06/09 20:20:35]

Done.

             return;
@@ -790,4 +807,20 @@ void HTMLFormElement::setDemoted(bool demoted)
     m_wasDemoted = demoted;
 }
 
+void HTMLFormElement::attributeChanged(const QualifiedName& name, const AtomicString& newValue, AttributeModificationReason)
+{
+    Element::attributeChanged(name, newValue);
+    if (name == actionAttr) {
+        // If the new action attribute is pointing to insecure "action" location from a secure page
+        // it is marked as "passive" mixed content. In other words, it will just
+        // show a console warning unless the user override the preferences to
+        // block all mixed content.
+        KURL actionURL = getActionURL(document(), m_attributes.action());
+        if (MixedContentChecker::isMixedContent(document().securityOrigin(), actionURL)) {
+            document().frame()->loader().mixedContentChecker()->canSubmitToInsecureForm(document().securityOrigin(), actionURL);
+            UseCounter::count(document(), UseCounter::MixedContentForm);
+        }
+    }
+}

[abarth-chromium, 2014/06/09 16:31:37]

Please remove this part of the change.  I don't think we need to count this
information and it makes the engine slower.

[mhm, 2014/06/09 20:20:35]

Done.

+
 } // namespace