Browse Source

Send parameters as urlencoded by default.

Unless there are files, request bodies should be urlencoded, and not
multipart/form-data. This increase compatibility with servers built with
PHP. Only when files are included should the default type be set to
multipart/form-data.

Refs #6840
Mark Story 10 years ago
parent
commit
02228fc635

+ 1 - 1
src/Network/Http/Adapter/Stream.php

@@ -182,7 +182,7 @@ class Stream
         if (is_array($content)) {
             $formData = new FormData();
             $formData->addMany($content);
-            $type = 'multipart/form-data; boundary="' . $formData->boundary() . '"';
+            $type = $formData->contentType();
             $request->header('Content-Type', $type);
             $this->_contextOptions['content'] = (string)$formData;
             return;

+ 3 - 1
src/Network/Http/Client.php

@@ -55,7 +55,9 @@ use Cake\Utility\Hash;
  * ### Sending request bodies
  *
  * By default any POST/PUT/PATCH/DELETE request with $data will
- * send their data as `multipart/form-data`.
+ * send their data as `application/x-www-form-urlencoded` unless
+ * there are attached files. In that case `multipart/form-data`
+ * will be used.
  *
  * When sending request bodies you can use the `type` option to
  * set the Content-Type for the request:

+ 50 - 7
src/Network/Http/FormData.php

@@ -35,6 +35,13 @@ class FormData implements Countable
     protected $_boundary;
 
     /**
+     * Whether or not this formdata object has attached files.
+     *
+     * @var boolean
+     */
+    protected $_hasFile = false;
+
+    /**
      * The parts in the form data.
      *
      * @var array
@@ -125,6 +132,8 @@ class FormData implements Countable
      */
     public function addFile($name, $value)
     {
+        $this->_hasFile = true;
+
         $filename = false;
         $contentType = 'application/octet-stream';
         if (is_resource($value)) {
@@ -176,6 +185,33 @@ class FormData implements Countable
     }
 
     /**
+     * Check whether or not the current payload
+     * has any files.
+     *
+     * @return boolean
+     */
+    public function hasFile()
+    {
+        return $this->_hasFile;
+    }
+
+    /**
+     * Get the content type for this payload.
+     *
+     * If this object contains files, `multipart/form-data` will be used,
+     * otherwise `application/x-www-form-urlencoded` will be used.
+     *
+     * @return string
+     */
+    public function contentType()
+    {
+        if (!$this->hasFile()) {
+            return 'application/x-www-form-urlencoded';
+        }
+        return 'multipart/form-data; boundary="' . $this->boundary() . '"';
+    }
+
+    /**
      * Converts the FormData and its parts into a string suitable
      * for use in an HTTP request.
      *
@@ -183,14 +219,21 @@ class FormData implements Countable
      */
     public function __toString()
     {
-        $boundary = $this->boundary();
-        $out = '';
+        if ($this->hasFile()) {
+            $boundary = $this->boundary();
+            $out = '';
+            foreach ($this->_parts as $part) {
+                $out .= "--$boundary\r\n";
+                $out .= (string)$part;
+                $out .= "\r\n";
+            }
+            $out .= "--$boundary--\r\n\r\n";
+            return $out;
+        }
+        $data = [];
         foreach ($this->_parts as $part) {
-            $out .= "--$boundary\r\n";
-            $out .= (string)$part;
-            $out .= "\r\n";
+            $data[$part->name()] = $part->value();
         }
-        $out .= "--$boundary--\r\n\r\n";
-        return $out;
+        return http_build_query($data);
     }
 }

+ 20 - 0
src/Network/Http/FormData/Part.php

@@ -165,6 +165,26 @@ class Part
     }
 
     /**
+     * Get the part name.
+     *
+     * @return string
+     */
+    public function name()
+    {
+        return $this->_name;
+    }
+
+    /**
+     * Get the value.
+     *
+     * @return string
+     */
+    public function value()
+    {
+        return $this->_value;
+    }
+
+    /**
      * Convert the part into a string.
      *
      * Creates a string suitable for use in HTTP requests.

+ 29 - 3
tests/TestCase/Network/Http/Adapter/StreamTest.php

@@ -137,11 +137,37 @@ class StreamTest extends TestCase
         $expected = [
             'Connection: close',
             'User-Agent: CakePHP',
-            'Content-Type: multipart/form-data; boundary="',
+            'Content-Type: application/x-www-form-urlencoded',
         ];
         $this->assertStringStartsWith(implode("\r\n", $expected), $result['header']);
-        $this->assertContains('Content-Disposition: form-data; name="a"', $result['content']);
-        $this->assertContains('my value', $result['content']);
+        $this->assertContains('a=my+value', $result['content']);
+        $this->assertContains('my+value', $result['content']);
+    }
+
+    /**
+     * Test send() + context options with array content.
+     *
+     * @return void
+     */
+    public function testSendContextContentArrayFiles()
+    {
+        $request = new Request();
+        $request->url('http://localhost')
+            ->header([
+                'Content-Type' => 'application/json'
+            ])
+            ->body(['upload' => fopen(CORE_PATH . 'VERSION.txt', 'r')]);
+
+        $this->stream->send($request, []);
+        $result = $this->stream->contextOptions();
+        $expected = [
+            'Connection: close',
+            'User-Agent: CakePHP',
+            'Content-Type: multipart/form-data',
+        ];
+        $this->assertStringStartsWith(implode("\r\n", $expected), $result['header']);
+        $this->assertContains('name="upload"', $result['content']);
+        $this->assertContains('filename="VERSION.txt"', $result['content']);
     }
 
     /**

+ 5 - 46
tests/TestCase/Network/Http/FormDataTest.php

@@ -66,28 +66,8 @@ class FormDataTest extends TestCase
         $this->assertCount(4, $data);
         $boundary = $data->boundary();
         $result = (string)$data;
-        $expected = [
-            '--' . $boundary,
-            'Content-Disposition: form-data; name="test"',
-            '',
-            'value',
-            '--' . $boundary,
-            'Content-Disposition: form-data; name="empty"',
-            '',
-            '',
-            '--' . $boundary,
-            'Content-Disposition: form-data; name="int"',
-            '',
-            '1',
-            '--' . $boundary,
-            'Content-Disposition: form-data; name="float"',
-            '',
-            '2.3',
-            '--' . $boundary . '--',
-            '',
-            '',
-        ];
-        $this->assertEquals(implode("\r\n", $expected), $result);
+        $expected = 'test=value&empty=&int=1&float=2.3';
+        $this->assertEquals($expected, $result);
     }
 
     /**
@@ -103,31 +83,10 @@ class FormDataTest extends TestCase
             'published' => 'Y',
             'tags' => ['blog', 'cakephp']
         ]);
-        $boundary = $data->boundary();
         $result = (string)$data;
-
-        $expected = [
-            '--' . $boundary,
-            'Content-Disposition: form-data; name="Article[title]"',
-            '',
-            'first post',
-            '--' . $boundary,
-            'Content-Disposition: form-data; name="Article[published]"',
-            '',
-            'Y',
-            '--' . $boundary,
-            'Content-Disposition: form-data; name="Article[tags][0]"',
-            '',
-            'blog',
-            '--' . $boundary,
-            'Content-Disposition: form-data; name="Article[tags][1]"',
-            '',
-            'cakephp',
-            '--' . $boundary . '--',
-            '',
-            '',
-        ];
-        $this->assertEquals(implode("\r\n", $expected), $result);
+        $expected = 'Article%5Btitle%5D=first+post&Article%5Bpublished%5D=Y&' .
+            'Article%5Btags%5D%5B0%5D=blog&Article%5Btags%5D%5B1%5D=cakephp';
+        $this->assertEquals($expected, $result);
     }
 
     /**