Changeset 138676 in webkit

Timestamp:

Jan 2, 2013, 4:26:37 PM (12 years ago)

Author:

ap@apple.com

Message:

[WK2 NetworkProcess] Uploading fails if a client modified the request
​https://bugs.webkit.org/show_bug.cgi?id=105965

Reviewed by Brady Eidson.

Associating streams with additional information via a side HashMap does not work,
because the stream we can see is an internal one, with address and other information
not matching a stream that we created.

It's surprising that this design issue was not causing major trouble without NetworkProcess.

• platform/network/cf/FormDataStreamCFNet.cpp: (WebCore): Renamed FormContext to FormCreationContext, because this type only used for creation. Later, we use FormStreamFields as context. (WebCore::closeCurrentStream): Moved a star. (WebCore::formCreate): We no longer have a side map to keep updated. (WebCore::formFinalize): Ditto. (WebCore::formCopyProperty): Added an accessor to get FormData pointer from a stream. (WebCore::setHTTPBody): Ditto. (WebCore::httpBodyFromStream): Use the new property to get FormData pointer.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/network/cf/FormDataStreamCFNet.cpp (modified) (8 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-02  Alexey Proskuryakov  <ap@apple.com>
+
+        [WK2 NetworkProcess] Uploading fails if a client modified the request
+        https://bugs.webkit.org/show_bug.cgi?id=105965
+
+        Reviewed by Brady Eidson.
+
+        Associating streams with additional information via a side HashMap does not work,
+        because the stream we can see is an internal one, with address and other information
+        not matching a stream that we created.
+
+        It's surprising that this design issue was not causing major trouble without NetworkProcess.
+
+        * platform/network/cf/FormDataStreamCFNet.cpp:
+        (WebCore): Renamed FormContext to FormCreationContext, because this type only used
+        for creation. Later, we use FormStreamFields as context.
+        (WebCore::closeCurrentStream): Moved a star.
+        (WebCore::formCreate): We no longer have a side map to keep updated.
+        (WebCore::formFinalize): Ditto.
+        (WebCore::formCopyProperty): Added an accessor to get FormData pointer from a stream.
+        (WebCore::setHTTPBody): Ditto.
+        (WebCore::httpBodyFromStream): Use the new property to get FormData pointer.
+
 2013-01-02  Elliott Sprehn  <esprehn@chromium.org>
 

trunk/Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp

@@ -87 +87 @@
 namespace WebCore {
 
-static Mutex& streamFieldsMapMutex()
-{
-    DEFINE_STATIC_LOCAL(Mutex, staticMutex, ());
-    return staticMutex;
-}
-
 static void formEventCallback(CFReadStreamRef stream, CFStreamEventType type, void* context);
 
-struct FormContext {
+static CFStringRef formDataPointerPropertyName = CFSTR("WebKitFormDataPointer");
+
+struct FormCreationContext {
     RefPtr<FormData> formData;
     unsigned long long streamLength;
@@ -114 +110 @@
 };
 
-typedef HashMap<CFReadStreamRef, FormStreamFields*> StreamFieldsMap;
-static StreamFieldsMap& streamFieldsMap()
-{
-    DEFINE_STATIC_LOCAL(StreamFieldsMap, streamFieldsMap, ());
-    return streamFieldsMap;
-}
-
-static void closeCurrentStream(FormStreamFields *form)
+static void closeCurrentStream(FormStreamFields* form)
 {
     if (form->currentStream) {
@@ -206 +195 @@
 static void* formCreate(CFReadStreamRef stream, void* context)
 {
-    FormContext* formContext = static_cast<FormContext*>(context);
+    FormCreationContext* formContext = static_cast<FormCreationContext*>(context);
 
     FormStreamFields* newInfo = new FormStreamFields;
@@ -225 +214 @@
         newInfo->remainingElements.append(newInfo->formData->elements()[size - i - 1]);
 
-    MutexLocker locker(streamFieldsMapMutex());
-    ASSERT(!streamFieldsMap().contains(stream));
-    streamFieldsMap().add(stream, newInfo);
-
     return newInfo;
 }
@@ -242 +227 @@
 {
     FormStreamFields* form = static_cast<FormStreamFields*>(context);
-
-    MutexLocker locker(streamFieldsMapMutex());
-
     ASSERT(form->formStream == stream);
-    ASSERT(streamFieldsMap().get(stream) == context);
-
-    // Do this right away because the CFReadStreamRef is being deallocated.
-    // We can't wait to remove this from the map until we finish finalizing
-    // on the main thread because in theory the freed memory could be reused
-    // for a new CFReadStream before that runs.
-    streamFieldsMap().remove(stream);
 
     callOnMainThread(formFinishFinalizationOnMainThread, form);
@@ -326 +301 @@
 
     closeCurrentStream(form);
+}
+
+static CFTypeRef formCopyProperty(CFReadStreamRef, CFStringRef propertyName, void *context)
+{
+    FormStreamFields* form = static_cast<FormStreamFields*>(context);
+
+    if (kCFCompareEqualTo != CFStringCompare(propertyName, formDataPointerPropertyName, 0))
+        return 0;
+
+    long formDataAsNumber = static_cast<long>(reinterpret_cast<intptr_t>(form->formData.get()));
+    return CFNumberCreate(0, kCFNumberLongType, &formDataAsNumber);
 }
 
@@ -427 +413 @@
 
     // Pass the length along with the formData so it does not have to be recomputed.
-    FormContext formContext = { formData.release(), length };
-
-    CFReadStreamCallBacksV1 callBacks = { 1, formCreate, formFinalize, 0, formOpen, 0, formRead, 0, formCanRead, formClose, 0, 0, 0, formSchedule, formUnschedule
+    FormCreationContext formContext = { formData.release(), length };
+
+    CFReadStreamCallBacksV1 callBacks = { 1, formCreate, formFinalize, 0, formOpen, 0, formRead, 0, formCanRead, formClose, formCopyProperty, 0, 0, formSchedule, formUnschedule
     };
     RetainPtr<CFReadStreamRef> stream = adoptCF(CFReadStreamCreate(0, static_cast<const void*>(&callBacks), &formContext));
@@ -441 +427 @@
         return 0;
 
-    MutexLocker locker(streamFieldsMapMutex());
-    FormStreamFields* formStream = streamFieldsMap().get(stream);
-    if (!formStream)
+    // Passing the pointer as property appears to be the only way to associate a stream with FormData.
+    // A new stream is always created in CFURLRequestCopyHTTPRequestBodyStream (or -[NSURLRequest HTTPBodyStream]),
+    // so a side HashMap wouldn't work.
+    // Even the stream's context pointer is different from the one we returned from formCreate().
+
+    RetainPtr<CFNumberRef> formDataPointerAsCFNumber = adoptCF(static_cast<CFNumberRef>(CFReadStreamCopyProperty(stream, formDataPointerPropertyName)));
+    if (!formDataPointerAsCFNumber)
         return 0;
-    return formStream->formData.get();
+
+    long formDataPointerAsNumber;
+    if (!CFNumberGetValue(formDataPointerAsCFNumber.get(), kCFNumberLongType, &formDataPointerAsNumber))
+        return 0;
+
+    return reinterpret_cast<FormData*>(static_cast<intptr_t>(formDataPointerAsNumber));
 }