Give CString an equality operator to fix a bug in HTMLFormElement::formData
authoraroben <aroben@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 1 Jul 2007 04:53:25 +0000 (04:53 +0000)
committeraroben <aroben@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 1 Jul 2007 04:53:25 +0000 (04:53 +0000)
WebCore:

        Give CString an equality operator to fix a bug in HTMLFormElement::formData

        The bug was spotted by MSVC /W4. The problem was that we were comparing
        a CString to a char* using ==, but CString had no equality operator.
        The result was that the CString was being cast to a const char* and a
        pointer comparison was being done, which would (essentially) always
        return false.

        There are two parts to the fix: get rid of CString's const char*
        casting operator, and add an equality operator. Previous uses of the
        casting operator have been changed to use CString::data().

        Test: http/misc/isindex-formdata.html

        Reviewed by Oliver.

        * dom/XMLTokenizer.cpp:
        (WebCore::parseXMLDocumentFragment):
        * html/HTMLDocument.cpp:
        (WebCore::HTMLDocument::determineParseMode):
        * html/HTMLFormElement.cpp:
        (WebCore::HTMLFormElement::formData):
        * loader/TextResourceDecoder.cpp:
        (WebCore::TextResourceDecoder::checkForCSSCharset):
        * platform/CString.cpp:
        (WebCore::operator==):
        * platform/CString.h:
        * platform/KURL.cpp:
        (WebCore::encodeRelativeString):
        * platform/StringImpl.cpp:
        (WebCore::StringImpl::toDouble):
        * platform/network/cf/FormDataStreamCFNet.cpp:
        (WebCore::setHTTPBody):
        * platform/network/mac/FormDataStreamMac.mm:
        (WebCore::setHTTPBody):
        * platform/win/ClipboardUtilitiesWin.cpp:
        (WebCore::markupToCF_HTML):
        * plugins/win/PluginPackageWin.cpp:
        (WebCore::splitString):
        * plugins/win/PluginStreamWin.cpp:
        (WebCore::PluginStreamWin::startStream):
        (WebCore::PluginStreamWin::destroyStream):
        (WebCore::PluginStreamWin::sendJavaScriptStream):
        (WebCore::PluginStreamWin::didFinishLoading):
        * plugins/win/PluginViewWin.cpp:
        (WebCore::PluginViewWin::start):
        (WebCore::createUTF8String):
        (WebCore::PluginViewWin::userAgent):
        * xml/XSLStyleSheet.cpp:
        (WebCore::XSLStyleSheet::locateStylesheetSubResource):
        * xml/XSLTProcessor.cpp:
        (WebCore::xsltParamArrayFromParameterMap):

LayoutTests:

        New test that makes sure we handle putting isindex into form data correctly.

        Reviewed by Oliver.

        * http/tests/misc/isindex-formdata-expected.txt: Added.
        * http/tests/misc/isindex-formdata.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@23906 268f45cc-cd09-0410-ab3c-d52691b4dbfc

19 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/misc/isindex-formdata-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/misc/isindex-formdata.html [new file with mode: 0755]
WebCore/ChangeLog
WebCore/dom/XMLTokenizer.cpp
WebCore/html/HTMLDocument.cpp
WebCore/html/HTMLFormElement.cpp
WebCore/loader/TextResourceDecoder.cpp
WebCore/platform/CString.cpp
WebCore/platform/CString.h
WebCore/platform/KURL.cpp
WebCore/platform/StringImpl.cpp
WebCore/platform/network/cf/FormDataStreamCFNet.cpp
WebCore/platform/network/mac/FormDataStreamMac.mm
WebCore/platform/win/ClipboardUtilitiesWin.cpp
WebCore/plugins/win/PluginStreamWin.cpp
WebCore/plugins/win/PluginViewWin.cpp
WebCore/xml/XSLStyleSheet.cpp
WebCore/xml/XSLTProcessor.cpp

index 8da9e68..fa367f8 100644 (file)
@@ -1,3 +1,12 @@
+2007-06-29  Adam Roben  <aroben@apple.com>
+
+        New test that makes sure we handle putting isindex into form data correctly.
+
+        Reviewed by Oliver.
+
+        * http/tests/misc/isindex-formdata-expected.txt: Added.
+        * http/tests/misc/isindex-formdata.html: Added.
+
 2007-06-29  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by Harrison.
diff --git a/LayoutTests/http/tests/misc/isindex-formdata-expected.txt b/LayoutTests/http/tests/misc/isindex-formdata-expected.txt
new file mode 100644 (file)
index 0000000..403dc1a
--- /dev/null
@@ -0,0 +1,5 @@
+This page tests that we correctly put the value of an <isindex> element into the form data.
+
+This is a searchable index. Enter search keywords: 
+PASS
+
diff --git a/LayoutTests/http/tests/misc/isindex-formdata.html b/LayoutTests/http/tests/misc/isindex-formdata.html
new file mode 100755 (executable)
index 0000000..c6da948
--- /dev/null
@@ -0,0 +1,36 @@
+<script>
+function test()
+{
+    if (!location.search.length) {
+        if (window.layoutTestController) {
+            layoutTestController.dumpAsText();
+            layoutTestController.waitUntilDone();
+        }
+
+        document.getElementById("isindex").value = "This is a test";
+        document.forms[0].submit();
+    } else {
+        var expected = "?This+is+a+test";
+        if (location.search != expected)
+            log("FAIL: Expected \"" + expected + "\" but got \"" + location.search + "\"");
+        else
+            log("PASS");
+
+        if (window.layoutTestController)
+            layoutTestController.notifyDone();
+    }
+}
+
+function log(msg)
+{
+    document.getElementById("log").appendChild(document.createTextNode(msg + "\n"));
+}
+
+window.onload = test;
+
+</script>
+<p>This page tests that we correctly put the value of an <tt>&lt;isindex&gt;</tt> element into the form data.</p>
+<form method="GET" action="isindex-formdata.html">
+    <isindex id="isindex"></isindex>
+</form>
+<pre id="log"></pre>
index 2572748..2218a2d 100644 (file)
@@ -1,5 +1,60 @@
 2007-06-30  Adam Roben  <aroben@apple.com>
 
+        Give CString an equality operator to fix a bug in HTMLFormElement::formData
+
+        The bug was spotted by MSVC /W4. The problem was that we were comparing
+        a CString to a char* using ==, but CString had no equality operator.
+        The result was that the CString was being cast to a const char* and a
+        pointer comparison was being done, which would (essentially) always
+        return false.
+
+        There are two parts to the fix: get rid of CString's const char*
+        casting operator, and add an equality operator. Previous uses of the
+        casting operator have been changed to use CString::data().
+
+        Test: http/misc/isindex-formdata.html
+
+        Reviewed by Oliver.
+
+        * dom/XMLTokenizer.cpp:
+        (WebCore::parseXMLDocumentFragment):
+        * html/HTMLDocument.cpp:
+        (WebCore::HTMLDocument::determineParseMode):
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::formData):
+        * loader/TextResourceDecoder.cpp:
+        (WebCore::TextResourceDecoder::checkForCSSCharset):
+        * platform/CString.cpp:
+        (WebCore::operator==):
+        * platform/CString.h:
+        * platform/KURL.cpp:
+        (WebCore::encodeRelativeString):
+        * platform/StringImpl.cpp:
+        (WebCore::StringImpl::toDouble):
+        * platform/network/cf/FormDataStreamCFNet.cpp:
+        (WebCore::setHTTPBody):
+        * platform/network/mac/FormDataStreamMac.mm:
+        (WebCore::setHTTPBody):
+        * platform/win/ClipboardUtilitiesWin.cpp:
+        (WebCore::markupToCF_HTML):
+        * plugins/win/PluginPackageWin.cpp:
+        (WebCore::splitString):
+        * plugins/win/PluginStreamWin.cpp:
+        (WebCore::PluginStreamWin::startStream):
+        (WebCore::PluginStreamWin::destroyStream):
+        (WebCore::PluginStreamWin::sendJavaScriptStream):
+        (WebCore::PluginStreamWin::didFinishLoading):
+        * plugins/win/PluginViewWin.cpp:
+        (WebCore::PluginViewWin::start):
+        (WebCore::createUTF8String):
+        (WebCore::PluginViewWin::userAgent):
+        * xml/XSLStyleSheet.cpp:
+        (WebCore::XSLStyleSheet::locateStylesheetSubResource):
+        * xml/XSLTProcessor.cpp:
+        (WebCore::xsltParamArrayFromParameterMap):
+
+2007-06-29  Adam Roben  <aroben@apple.com>
+
         Initialize two variables that MSVC /W4 isn't smart enough to realize always get initialized
 
         Reviewed by John.
index 6e3a86a..ba0453e 100644 (file)
@@ -1423,7 +1423,7 @@ bool parseXMLDocumentFragment(const String& string, DocumentFragment* fragment,
     sax.warning = balancedWarningHandler;
     sax.initialized = XML_SAX2_MAGIC;
     
-    int result = xmlParseBalancedChunkMemory(0, &sax, &tokenizer, 0, (const xmlChar*)(const char*)(string.utf8()), 0);
+    int result = xmlParseBalancedChunkMemory(0, &sax, &tokenizer, 0, (const xmlChar*)string.utf8().data(), 0);
     return result == 0;
 }
 
index 7c68858..8097e33 100644 (file)
@@ -558,7 +558,7 @@ void HTMLDocument::determineParseMode(const String& str)
             CString pubIDStr = lowerPubID.latin1();
            
             // Look up the entry in our gperf-generated table.
-            const PubIDInfo* doctypeEntry = findDoctypeEntry(pubIDStr, pubIDStr.length());
+            const PubIDInfo* doctypeEntry = findDoctypeEntry(pubIDStr.data(), pubIDStr.length());
             if (!doctypeEntry) {
                 // The DOCTYPE is not in the list.  Assume strict mode.
                 pMode = Strict;
index a7fad39..cb8f89c 100644 (file)
@@ -283,7 +283,7 @@ PassRefPtr<FormData> HTMLFormElement::formData(const char* boundary) const
                         hstr += "; filename=\"";
                         int start = path.reverseFind('/') + 1;
                         int length = path.length() - start;
-                        hstr += encoding.encode(reinterpret_cast<const UChar*>(path.characters() + start), length, true);
+                        hstr += encoding.encode(reinterpret_cast<const UChar*>(path.characters() + start), length, true).data();
                         hstr += "\"";
 
                         if (!static_cast<HTMLInputElement*>(current)->value().isEmpty()) {
@@ -301,7 +301,7 @@ PassRefPtr<FormData> HTMLFormElement::formData(const char* boundary) const
                     result->appendData(hstr.data(), hstr.length());
                     const FormDataListItem& item = lst.list()[j + 1];
                     if (size_t dataSize = item.m_data.length())
-                        result->appendData(item.m_data, dataSize);
+                        result->appendData(item.m_data.data(), dataSize);
                     else if (!item.m_path.isEmpty())
                         result->appendFile(item.m_path);
                     result->appendData("\r\n", 2);
index 2657ca0..6b6aa7d 100644 (file)
@@ -421,7 +421,7 @@ bool TextResourceDecoder::checkForCSSCharset(const char* data, size_t len, bool&
                     return false;
 
                 if (*pos == ';')
-                    setEncoding(TextEncoding(encodingName), EncodingFromCSSCharset);
+                    setEncoding(TextEncoding(encodingName.data()), EncodingFromCSSCharset);
             }
         }
         m_checkedForCSSCharset = true;
index 7c33adb..4300b29 100644 (file)
@@ -99,4 +99,13 @@ void CString::copyBufferIfNeeded()
     memcpy(m_buffer->data(), m_temp->data(), len);
 }
 
+bool operator==(const CString& a, const CString& b)
+{
+    if (a.isNull() != b.isNull())
+        return false;
+    if (a.length() != b.length())
+        return false;
+    return !strncmp(a.data(), b.data(), min(a.length(), b.length()));
+}
+
 }
index ddd2d57..41ee8cf 100644 (file)
@@ -29,6 +29,8 @@
 #include "Shared.h"
 #include <wtf/Vector.h>
 
+using std::min;
+
 namespace WebCore {
 
     class DeprecatedCString;
@@ -57,8 +59,6 @@ namespace WebCore {
         char* mutableData();
         unsigned length() const;
 
-        operator const char*() const { return data(); }        
-
         bool isNull() const { return !m_buffer; }
 
         CString(const DeprecatedCString&);
@@ -70,6 +70,9 @@ namespace WebCore {
         RefPtr<CStringBuffer> m_buffer;
     };
 
+    bool operator==(const CString& a, const CString& b);
+    inline bool operator!=(const CString& a, const CString& b) { return !(a == b); }
+
 }
 
 #endif // CString_h
index d8b2743..4fa6a7d 100644 (file)
@@ -1446,7 +1446,7 @@ static char *encodeRelativeString(const KURL &base, const DeprecatedString &rel,
         CString decoded = pathEncoding.encode(reinterpret_cast<const UChar*>(s.unicode()), s.length());
         int decodedLength = decoded.length();
         strBuffer = static_cast<char *>(fastMalloc(decodedLength + 1));
-        memcpy(strBuffer, decoded, decodedLength);
+        memcpy(strBuffer, decoded.data(), decodedLength);
         strBuffer[decodedLength] = 0;
     } else {
         int length = s.length();
@@ -1455,8 +1455,8 @@ static char *encodeRelativeString(const KURL &base, const DeprecatedString &rel,
         int pathDecodedLength = pathDecoded.length();
         int otherDecodedLength = otherDecoded.length();
         strBuffer = static_cast<char *>(fastMalloc(pathDecodedLength + otherDecodedLength + 1));
-        memcpy(strBuffer, pathDecoded, pathDecodedLength);
-        memcpy(strBuffer + pathDecodedLength, otherDecoded, otherDecodedLength);
+        memcpy(strBuffer, pathDecoded.data(), pathDecodedLength);
+        memcpy(strBuffer + pathDecodedLength, otherDecoded.data(), otherDecodedLength);
         strBuffer[pathDecodedLength + otherDecodedLength] = 0;
     }
 
index 86329ad..5ae3558 100644 (file)
@@ -636,7 +636,7 @@ double StringImpl::toDouble(bool* ok) const
     }
     char *end;
     CString latin1String = Latin1Encoding().encode(characters(), length());
-    double val = kjs_strtod(latin1String, &end);
+    double val = kjs_strtod(latin1String.data(), &end);
     if (ok)
         *ok = end == 0 || *end == '\0';
     return val;
index c7fb735..f834e7d 100644 (file)
@@ -341,7 +341,7 @@ void setHTTPBody(CFMutableURLRequestRef request, PassRefPtr<FormData> formData)
             length += element.m_data.size();
         else {
             struct _stat64i32 sb;
-            int statResult = _stat(element.m_filename.utf8(), &sb);
+            int statResult = _stat(element.m_filename.utf8().data(), &sb);
             if (statResult == 0 && (sb.st_mode & S_IFMT) == S_IFREG)
                 length += sb.st_size;
             else
index e3abc3e..c88fb81 100644 (file)
@@ -327,7 +327,7 @@ void setHTTPBody(NSMutableURLRequest *request, PassRefPtr<FormData> formData)
             length += element.m_data.size();
         else {
             struct stat sb;
-            int statResult = stat(element.m_filename.utf8(), &sb);
+            int statResult = stat(element.m_filename.utf8().data(), &sb);
             if (statResult == 0 && (sb.st_mode & S_IFMT) == S_IFREG)
                 length += sb.st_size;
         }
index 9644349..e8d9eab 100644 (file)
@@ -158,7 +158,7 @@ DeprecatedCString markupToCF_HTML(const String& markup, const String& srcURL)
 
     bool shouldFillSourceURL = !srcURL.isEmpty() && (srcURL != "about:blank");
     if (shouldFillSourceURL)
-        sourceURL.append(srcURL.utf8());
+        sourceURL.append(srcURL.utf8().data());
 
     DeprecatedCString startMarkup    ("\n<HTML>\n<BODY>\n<!--StartFragment-->\n");
     DeprecatedCString endMarkup      ("\n<!--EndFragment-->\n</BODY>\n</HTML>");
index 1653353..fd8ded2 100644 (file)
@@ -137,7 +137,7 @@ void PluginStreamWin::startStream()
     // Protect the stream if destroystream is called from within the newstream handler
     RefPtr<PluginStreamWin> protect(this);
 
-    NPError npErr = m_pluginFuncs->newstream(m_instance, (NPMIMEType)(const char*)mimeTypeStr, &m_stream, false, &m_transferMode);
+    NPError npErr = m_pluginFuncs->newstream(m_instance, (NPMIMEType)mimeTypeStr.data(), &m_stream, false, &m_transferMode);
     
     // If the stream was destroyed in the call to newstream we return
     if (m_reason != WebReasonNone)
@@ -186,10 +186,10 @@ void PluginStreamWin::destroyStream()
 
     if (m_stream.ndata != 0) {
         if (m_reason == NPRES_DONE && (m_transferMode == NP_ASFILE || m_transferMode == NP_ASFILEONLY)) {
-            ASSERT(m_path != 0);
+            ASSERT(!m_path.isNull());
 
-            m_pluginFuncs->asfile(m_instance, &m_stream, m_path);
-            DeleteFileA(m_path);
+            m_pluginFuncs->asfile(m_instance, &m_stream, m_path.data());
+            DeleteFileA(m_path.data());
         }
 
         NPError npErr;
@@ -278,7 +278,7 @@ void PluginStreamWin::sendJavaScriptStream(const KURL& requestURL, const CString
     if (m_streamState == StreamStopped)
         return;
 
-    didReceiveData(0, resultString, resultString.length());
+    didReceiveData(0, resultString.data(), resultString.length());
     if (m_streamState == StreamStopped)
         return;
 
@@ -329,7 +329,7 @@ void PluginStreamWin::didFinishLoading(SubresourceLoader* loader)
 
     m_loader = 0;
 
-    if ((m_transferMode == NP_ASFILE || m_transferMode == NP_ASFILEONLY) && !m_path) {
+    if ((m_transferMode == NP_ASFILE || m_transferMode == NP_ASFILEONLY) && m_path.isNull()) {
         char tempPath[MAX_PATH];
 
         if (GetTempPathA(sizeof(tempPath), tempPath) == 0) {
index 5a055b5..d69a9a9 100644 (file)
@@ -469,7 +469,7 @@ bool PluginViewWin::start()
     PluginViewWin::setCurrentPluginView(this);
     {
         KJS::JSLock::DropAllLocks dropAllLocks;
-        npErr = m_plugin->pluginFuncs()->newp((NPMIMEType)(const char*)m_mimeType, m_instance, m_mode, m_paramCount, m_paramNames, m_paramValues, NULL);
+        npErr = m_plugin->pluginFuncs()->newp((NPMIMEType)m_mimeType.data(), m_instance, m_mode, m_paramCount, m_paramNames, m_paramValues, NULL);
         LOG_NPERROR(npErr);
     }
     PluginViewWin::setCurrentPluginView(0);
@@ -534,7 +534,7 @@ static char* createUTF8String(const String& str)
     CString cstr = str.utf8();
     char* result = reinterpret_cast<char*>(fastMalloc(cstr.length() + 1));
 
-    strncpy(result, cstr, cstr.length() + 1);
+    strncpy(result, cstr.data(), cstr.length() + 1);
 
     return result;
 }
@@ -995,7 +995,7 @@ const char* PluginViewWin::userAgent()
 
     if (m_userAgent.isNull())
         m_userAgent = m_parentFrame->loader()->userAgent(m_url).utf8();
-    return m_userAgent;
+    return m_userAgent.data();
 }
 
 void PluginViewWin::status(const char* message)
index 938d3dd..0991162 100644 (file)
@@ -232,7 +232,7 @@ xmlDocPtr XSLStyleSheet::locateStylesheetSubResource(xmlDocPtr parentDoc, const
                 // with the URI argument.
                 CString importHref = import->href().utf8();
                 xmlChar* base = xmlNodeGetBase(parentDoc, (xmlNodePtr)parentDoc);
-                xmlChar* childURI = xmlBuildURI((const xmlChar*)(const char*)importHref, base);
+                xmlChar* childURI = xmlBuildURI((const xmlChar*)importHref.data(), base);
                 bool equalURIs = xmlStrEqual(uri, childURI);
                 xmlFree(base);
                 xmlFree(childURI);
index 5cce02b..5462445 100644 (file)
@@ -168,8 +168,8 @@ static const char **xsltParamArrayFromParameterMap(XSLTProcessor::ParameterMap&
     XSLTProcessor::ParameterMap::iterator end = parameters.end();
     unsigned index = 0;
     for (XSLTProcessor::ParameterMap::iterator it = parameters.begin(); it != end; ++it) {
-        parameterArray[index++] = strdup(it->first.utf8());
-        parameterArray[index++] = strdup(it->second.utf8());
+        parameterArray[index++] = strdup(it->first.utf8().data());
+        parameterArray[index++] = strdup(it->second.utf8().data());
     }
     parameterArray[index] = 0;