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 edf2c15f82829b4e631f0f0fe688ad1274974835..1a3e2221467cb0eebe8431b757716c6aea126e52 100644
--- a/Source/core/html/HTMLFormElement.cpp
+++ b/Source/core/html/HTMLFormElement.cpp
@@ -72,6 +72,7 @@ HTMLFormElement::HTMLFormElement(Document& document)
     , m_wasUserSubmitted(false)
     , m_isInResetFunction(false)
     , m_wasDemoted(false)
+    , m_insecureSubmissionReported(false)
     , m_pendingAutocompleteEventsQueue(GenericEventQueue::create(this))
 {
     ScriptWrappable::init(this);
@@ -346,6 +347,13 @@ void HTMLFormElement::submit(Event* event, bool activateSubmitButton, bool proce
     if (!view || !frame || !frame->page())
         return;
 
+    // Mixed content with form submission to insecure "action"
+    KURL actionURL = document().completeURL(m_attributes.action().isEmpty() ? document().url().string() : m_attributes.action());

[jww, 2014/06/05 03:49:46]

You should probably factor all of this out into a utility method called
getActionURL() or something, since you're repeating this logic elsewhere.

[mhm, 2014/06/05 17:04:28]

Done.

+    if (!frame->loader().mixedContentChecker()->canDisplayInsecureContent(document().securityOrigin(), actionURL)) {
+        m_insecureSubmissionReported = true;
+        return;
+    }
+
     m_wasUserSubmitted = processingUserGesture;
 
     RefPtrWillBeRawPtr<HTMLFormControlElement> firstSuccessfulSubmitButton = nullptr;
@@ -747,6 +755,14 @@ void HTMLFormElement::finishParsingChildren()
     HTMLElement::finishParsingChildren();
     document().formController().restoreControlStateIn(*this);
     m_didFinishParsingChildren = true;
+
+    // If the post is pointing to insecure "action" location from a secure page
+    // it is mixed content.
+    KURL actionURL = document().completeURL(m_attributes.action().isEmpty() ? document().url().string() : m_attributes.action());

[jww, 2014/06/05 03:49:46]

See comment above about factoring this logic out.

[mhm, 2014/06/05 17:04:28]

Done.

+    if (!m_insecureSubmissionReported && MixedContentChecker::isMixedContent(document().securityOrigin(), actionURL)) {

[jww, 2014/06/05 03:49:46]

Although it's technically not needed, for clarify, can you add a #include for
MixedContentChecker.h at the top of this file?

[mhm, 2014/06/05 17:04:28]

Done.

+        document().frame()->loader().mixedContentChecker()->canDisplayInsecureContent(document().securityOrigin(), actionURL);
+        m_insecureSubmissionReported = true;
+    }
 }
 
 void HTMLFormElement::copyNonAttributePropertiesFromElement(const Element& source)
@@ -791,4 +807,19 @@ void HTMLFormElement::setDemoted(bool demoted)
     m_wasDemoted = demoted;
 }
 
+void HTMLFormElement::attributeChanged(const QualifiedName& name, const AtomicString& newValue, AttributeModificationReason)

[jww, 2014/06/05 03:49:46]

Does this get called when the object is initially created? That is, during
parsing, when the action attribute is set, does that call attributeChanged? If
so, then the logic in finishParsingChildren() is redundant and can be removed.

[mhm, 2014/06/05 17:04:28]

Done.

[mhm, 2014/06/05 17:04:28]

Yes it does get called :-)

Removed redundant code.

On 2014/06/05 03:49:46, jww wrote:

+{
+    Element::attributeChanged(name, newValue);
+
+    if (name == actionAttr) {
+        // If the new action attribute is pointing to insecure "action" location from a secure page
+        // it is mixed content.
+        KURL actionURL = document().completeURL(m_attributes.action().isEmpty() ? document().url().string() : m_attributes.action());

[jww, 2014/06/05 03:49:46]

Assuming you don't remove the logic in finishParsingChildren, this logic looks
identical to the logic there, so you probably want to factor this out into a
utility method as well.

[mhm, 2014/06/05 17:04:28]

Done.

+        if (!m_insecureSubmissionReported && MixedContentChecker::isMixedContent(document().securityOrigin(), actionURL)) {
+            document().frame()->loader().mixedContentChecker()->canDisplayInsecureContent(document().securityOrigin(), actionURL);
+            m_insecureSubmissionReported = true;
+        }
+    }
+}
+
 } // namespace