From 2eb34a7b2c6137e68ff36b5d6740c3355bd3a7c6 Mon Sep 17 00:00:00 2001 From: Master Yoda Date: Sat, 10 Nov 2018 01:49:23 -0800 Subject: [PATCH 1/3] Refactor HTTP\UploadedFile for better unit testingz --- system/HTTP/Files/UploadedFile.php | 129 ++++++++++++++--------------- 1 file changed, 64 insertions(+), 65 deletions(-) diff --git a/system/HTTP/Files/UploadedFile.php b/system/HTTP/Files/UploadedFile.php index c3a7a69166f9..352ccec4344c 100644 --- a/system/HTTP/Files/UploadedFile.php +++ b/system/HTTP/Files/UploadedFile.php @@ -29,12 +29,12 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. * - * @package CodeIgniter - * @author CodeIgniter Dev Team - * @copyright 2014-2018 British Columbia Institute of Technology (https://bcit.ca/) - * @license https://opensource.org/licenses/MIT MIT License - * @link https://codeigniter.com - * @since Version 3.0.0 + * @package CodeIgniter + * @author CodeIgniter Dev Team + * @copyright 2014-2018 British Columbia Institute of Technology (https://bcit.ca/) + * @license https://opensource.org/licenses/MIT MIT License + * @link https://codeigniter.com + * @since Version 3.0.0 * @filesource */ use CodeIgniter\Files\File; @@ -83,14 +83,14 @@ class UploadedFile extends File implements UploadedFileInterface * The error constant of the upload * (one of PHP's UPLOADERRXXX constants) * - * @var int + * @var integer */ protected $error; /** * Whether the file has been moved already or not. * - * @var bool + * @var boolean */ protected $hasMoved = false; @@ -99,20 +99,20 @@ class UploadedFile extends File implements UploadedFileInterface /** * Accepts the file information as would be filled in from the $_FILES array. * - * @param string $path The temporary location of the uploaded file. - * @param string $originalName The client-provided filename. - * @param string $mimeType The type of file as provided by PHP - * @param int $size The size of the file, in bytes - * @param int $error The error constant of the upload (one of PHP's UPLOADERRXXX constants) + * @param string $path The temporary location of the uploaded file. + * @param string $originalName The client-provided filename. + * @param string $mimeType The type of file as provided by PHP + * @param integer $size The size of the file, in bytes + * @param integer $error The error constant of the upload (one of PHP's UPLOADERRXXX constants) */ public function __construct(string $path, string $originalName, string $mimeType = null, int $size = null, int $error = null) { - $this->path = $path; - $this->name = $originalName; - $this->originalName = $originalName; - $this->originalMimeType = $mimeType; - $this->size = $size; - $this->error = $error; + $this->path = $path; + $this->name = $originalName; + $this->originalName = $originalName; + $this->originalMimeType = $mimeType; + $this->size = $size; + $this->error = $error; parent::__construct($path, false); } @@ -141,12 +141,12 @@ public function __construct(string $path, string $originalName, string $mimeType * @see http://php.net/is_uploaded_file * @see http://php.net/move_uploaded_file * - * @param string $targetPath Path to which to move the uploaded file. - * @param string $name the name to rename the file to. - * @param bool $overwrite State for indicating whether to overwrite the previously generated file with the same - * name or not. + * @param string $targetPath Path to which to move the uploaded file. + * @param string $name the name to rename the file to. + * @param boolean $overwrite State for indicating whether to overwrite the previously generated file with the same + * name or not. * - * @return bool + * @return boolean * * @throws \InvalidArgumentException if the $path specified is invalid. * @throws \RuntimeException on any error during the move operation. @@ -161,13 +161,13 @@ public function move(string $targetPath, string $name = null, bool $overwrite = throw HTTPException::forAlreadyMoved(); } - if ( ! $this->isValid()) + if (! $this->isValid()) { throw HTTPException::forInvalidFile(); } - $targetPath = rtrim($targetPath, '/') . '/'; - $name = is_null($name) ? $this->getName() : $name; + $targetPath = rtrim($targetPath, '/') . '/'; + $name = is_null($name) ? $this->getName() : $name; $destination = $overwrite ? $targetPath . $name : $this->getDestination($targetPath . $name); try @@ -183,9 +183,9 @@ public function move(string $targetPath, string $name = null, bool $overwrite = @chmod($targetPath, 0777 & ~umask()); // Success, so store our new information - $this->path = $targetPath; - $this->name = basename($destination); - $this->hasMoved = true; + $this->path = $targetPath; + $this->name = basename($destination); + $this->hasMoved = true; return true; } @@ -200,11 +200,11 @@ public function move(string $targetPath, string $name = null, bool $overwrite = */ protected function setPath($path) { - if ( ! is_dir($path)) + if (! is_dir($path)) { mkdir($path, 0777, true); //create the index.html file - if ( ! file_exists($path . 'index.html')) + if (! file_exists($path . 'index.html')) { $file = fopen($path . 'index.html', 'x+'); fclose($file); @@ -220,7 +220,7 @@ protected function setPath($path) * the move() method will not work and certain properties, like * the tempName, will no longer be available. * - * @return bool + * @return boolean */ public function hasMoved(): bool { @@ -240,8 +240,8 @@ public function hasMoved(): bool * Implementations SHOULD return the value stored in the "error" key of * the file in the $_FILES array. * - * @see http://php.net/manual/en/features.file-upload.errors.php - * @return int One of PHP's UPLOAD_ERR_XXX constants. + * @see http://php.net/manual/en/features.file-upload.errors.php + * @return integer One of PHP's UPLOAD_ERR_XXX constants. */ public function getError(): int { @@ -265,14 +265,14 @@ public function getError(): int public function getErrorString() { static $errors = [ - UPLOAD_ERR_OK => 'The file uploaded with success.', - UPLOAD_ERR_INI_SIZE => 'The file "%s" exceeds your upload_max_filesize ini directive.', - UPLOAD_ERR_FORM_SIZE => 'The file "%s" exceeds the upload limit defined in your form.', - UPLOAD_ERR_PARTIAL => 'The file "%s" was only partially uploaded.', - UPLOAD_ERR_NO_FILE => 'No file was uploaded.', - UPLOAD_ERR_CANT_WRITE => 'The file "%s" could not be written on disk.', - UPLOAD_ERR_NO_TMP_DIR => 'File could not be uploaded: missing temporary directory.', - UPLOAD_ERR_EXTENSION => 'File upload was stopped by a PHP extension.', + UPLOAD_ERR_OK => 'The file uploaded with success.', + UPLOAD_ERR_INI_SIZE => 'The file "%s" exceeds your upload_max_filesize ini directive.', + UPLOAD_ERR_FORM_SIZE => 'The file "%s" exceeds the upload limit defined in your form.', + UPLOAD_ERR_PARTIAL => 'The file "%s" was only partially uploaded.', + UPLOAD_ERR_NO_FILE => 'No file was uploaded.', + UPLOAD_ERR_CANT_WRITE => 'The file "%s" could not be written on disk.', + UPLOAD_ERR_NO_TMP_DIR => 'File could not be uploaded: missing temporary directory.', + UPLOAD_ERR_EXTENSION => 'File upload was stopped by a PHP extension.', ]; $error = is_null($this->error) ? UPLOAD_ERR_OK : $this->error; @@ -343,14 +343,14 @@ public function getTempName(): string * Is simply an alias for guessExtension for a safer method * than simply relying on the provided extension. * Additionaly it will return clientExtension in case if there are - * other extensions withe the same mime type. + * other extensions withe the same mime type. */ - public function getExtension() : string + public function getExtension(): string { return $this->guessExtension(); } - public function guessExtension() : ?string + public function guessExtension(): ?string { return \Config\Mimes::guessExtensionFromType($this->getMimeType(), $this->getClientExtension()); } @@ -375,7 +375,7 @@ public function getClientExtension(): string * Returns whether the file was uploaded successfully, based on whether * it was uploaded via HTTP and has no errors. * - * @return bool + * @return boolean */ public function isValid(): bool { @@ -383,24 +383,23 @@ public function isValid(): bool } /** - * Save the uploaded file to a new location. - * - * By default, upload files are saved in writable/uploads directory. The YYYYMMDD folder - * and random file name will be created. - * - * @param string $folderName the folder name to writable/uploads directory. - * @param string $fileName the name to rename the file to. - * @return string file full path - */ - public function store($folderName = null, $fileName = null) : string - { - $folderName = $folderName ?? date('Ymd'); - $fileName = $fileName ?? $this->getRandomName(); - - // Move the uploaded file to a new location. - if ($this->move(WRITEPATH . 'uploads/' . $folderName, $fileName)) { - return $folderName . DIRECTORY_SEPARATOR . $this->name; - } + * Save the uploaded file to a new location. + * + * By default, upload files are saved in writable/uploads directory. The YYYYMMDD folder + * and random file name will be created. + * + * @param string $folderName the folder name to writable/uploads directory. + * @param string $fileName the name to rename the file to. + * @return string file full path + */ + public function store($folderName = null, $fileName = null): string + { + $folderName = $folderName ?? date('Ymd'); + $fileName = $fileName ?? $this->getRandomName(); + + // Move the uploaded file to a new location. + return ($this->move(WRITEPATH . 'uploads/' . $folderName, $fileName)) ? + $folderName . DIRECTORY_SEPARATOR . $this->name : null; } //-------------------------------------------------------------------- From 00a70917a0d6491d3cfea15a30020c643ff8a7fd Mon Sep 17 00:00:00 2001 From: Master Yoda Date: Sat, 10 Nov 2018 02:30:31 -0800 Subject: [PATCH 2/3] Beef up HTTP\URITest --- tests/system/HTTP/URITest.php | 225 ++++++++++++++++++++++++++++------ 1 file changed, 185 insertions(+), 40 deletions(-) diff --git a/tests/system/HTTP/URITest.php b/tests/system/HTTP/URITest.php index 7e2d95a63734..eabedb24927b 100644 --- a/tests/system/HTTP/URITest.php +++ b/tests/system/HTTP/URITest.php @@ -15,7 +15,6 @@ public function setUp() public function tearDown() { - } //-------------------------------------------------------------------- @@ -57,7 +56,7 @@ public function testSegmentsIsPopulatedRightForMultipleSegments() public function testSegmentOutOfRange() { $this->expectException(HTTPException::class); - $url = "http://abc.com/a123/b/c"; + $url = 'http://abc.com/a123/b/c'; $uri = new URI($url); $uri->getSegment(22); } @@ -104,7 +103,7 @@ public function testEmptyUri() public function testMalformedUri() { $this->expectException(HTTPException::class); - $url = "http://abc:a123"; + $url = 'http://abc:a123'; $uri = new URI($url); } @@ -275,12 +274,30 @@ public function testSetPathSetsValue() public function invalidPaths() { return [ - 'dot-segment' => ['/./path/to/nowhere', '/path/to/nowhere'], - 'double-dots' => ['/../path/to/nowhere', '/path/to/nowhere'], - 'start-dot' => ['./path/to/nowhere', '/path/to/nowhere'], - 'start-double' => ['../path/to/nowhere', '/path/to/nowhere'], - 'decoded' => ['../%41path', '/Apath'], - 'encoded' => ['/path^here', '/path%5Ehere'], + 'dot-segment' => [ + '/./path/to/nowhere', + '/path/to/nowhere', + ], + 'double-dots' => [ + '/../path/to/nowhere', + '/path/to/nowhere', + ], + 'start-dot' => [ + './path/to/nowhere', + '/path/to/nowhere', + ], + 'start-double' => [ + '../path/to/nowhere', + '/path/to/nowhere', + ], + 'decoded' => [ + '../%41path', + '/Apath', + ], + 'encoded' => [ + '/path^here', + '/path%5Ehere', + ], ]; } @@ -356,10 +373,22 @@ public function testSetQueryThrowsErrorWhenFragmentPresent() public function authorityInfo() { return [ - 'host-only' => ['http://foo.com/bar', 'foo.com'], - 'host-port' => ['http://foo.com:3000/bar', 'foo.com:3000'], - 'user-host' => ['http://me@foo.com/bar', 'me@foo.com'], - 'user-host-port' => ['http://me@foo.com:3000/bar', 'me@foo.com:3000'], + 'host-only' => [ + 'http://foo.com/bar', + 'foo.com', + ], + 'host-port' => [ + 'http://foo.com:3000/bar', + 'foo.com:3000', + ], + 'user-host' => [ + 'http://me@foo.com/bar', + 'me@foo.com', + ], + 'user-host-port' => [ + 'http://me@foo.com:3000/bar', + 'me@foo.com:3000', + ], ]; } @@ -379,8 +408,14 @@ public function testAuthorityReturnsExceptedValues($url, $expected) public function defaultPorts() { return [ - 'http' => ['http', 80], - 'https' => ['https', 443], + 'http' => [ + 'http', + 80, + ], + 'https' => [ + 'https', + 443, + ], ]; } @@ -416,22 +451,70 @@ public function testSetAuthorityReconstitutes() public function defaultDots() { return [ - ['/foo/..', '/'], - ['//foo//..', '/'], - ['/foo/../..', '/'], - ['/foo/../.', '/'], - ['/./foo/..', '/'], - ['/./foo', '/foo'], - ['/./foo/', '/foo/'], - ['/./foo/bar/baz/pho/../..', '/foo/bar'], - ['*', '*'], - ['/foo', '/foo'], - ['/abc/123/../foo/', '/abc/foo/'], - ['/a/b/c/./../../g', '/a/g'], - ['/b/c/./../../g', '/g'], - ['/b/c/./../../g', '/g'], - ['/c/./../../g', '/g'], - ['/./../../g', '/g'], + [ + '/foo/..', + '/', + ], + [ + '//foo//..', + '/', + ], + [ + '/foo/../..', + '/', + ], + [ + '/foo/../.', + '/', + ], + [ + '/./foo/..', + '/', + ], + [ + '/./foo', + '/foo', + ], + [ + '/./foo/', + '/foo/', + ], + [ + '/./foo/bar/baz/pho/../..', + '/foo/bar', + ], + [ + '*', + '*', + ], + [ + '/foo', + '/foo', + ], + [ + '/abc/123/../foo/', + '/abc/foo/', + ], + [ + '/a/b/c/./../../g', + '/a/g', + ], + [ + '/b/c/./../../g', + '/g', + ], + [ + '/b/c/./../../g', + '/g', + ], + [ + '/c/./../../g', + '/g', + ], + [ + '/./../../g', + '/g', + ], ]; } @@ -451,12 +534,30 @@ public function testRemoveDotSegments($path, $expected) public function defaultResolutions() { return [ - ['g', 'http://a/b/c/g'], - ['g/', 'http://a/b/c/g/'], - ['/g', 'http://a/g'], - ['#s', 'http://a/b/c/d#s'], - ['http://abc.com/x', 'http://abc.com/x'], - ['?fruit=banana', 'http://a/b/c/d?fruit=banana'], + [ + 'g', + 'http://a/b/c/g', + ], + [ + 'g/', + 'http://a/b/c/g/', + ], + [ + '/g', + 'http://a/g', + ], + [ + '#s', + 'http://a/b/c/d#s', + ], + [ + 'http://abc.com/x', + 'http://abc.com/x', + ], + [ + '?fruit=banana', + 'http://a/b/c/d?fruit=banana', + ], ]; } @@ -476,7 +577,7 @@ public function testResolveRelativeURI($rel, $expected) /** * @dataProvider defaultResolutions - * @group single + * @group single */ public function testResolveRelativeURIHTTPS($rel, $expected) { @@ -491,6 +592,17 @@ public function testResolveRelativeURIHTTPS($rel, $expected) $this->assertEquals($expected, (string) $new); } + public function testResolveRelativeURIWithNoBase() + { + $base = 'http://a'; + + $uri = new URI($base); + + $new = $uri->resolveRelativeURI('x'); + + $this->assertEquals('http://a/x', (string) $new); + } + //-------------------------------------------------------------------- public function testAddQueryVar() @@ -513,7 +625,7 @@ public function testSetQueryDecode() { $base = 'http://example.com/foo'; - $uri = new URI($base); + $uri = new URI($base); $encoded = urlencode('you+alice+to+the+little'); $uri->setQuery("q={$encoded}"); @@ -591,12 +703,24 @@ public function testGetQueryOnly() $uri = new URI($base); $this->assertEquals('bar=baz', $uri->getQuery(['only' => ['bar']])); + $this->assertEquals('foo=bar&baz=foz', $uri->getQuery(['except' => 'bar'])); + } + + //-------------------------------------------------------------------- + + public function testGetQueryWithStrings() + { + $base = 'http://example.com/foo?foo=bar&bar=baz&baz=foz'; + + $uri = new URI($base); + + $this->assertEquals('bar=baz', $uri->getQuery(['only' => 'bar'])); } //-------------------------------------------------------------------- /** - * @see https://github.com/bcit-ci/CodeIgniter4/issues/331 + * @see https://github.com/bcit-ci/CodeIgniter4/issues/331 * @group single */ public function testNoExtraSlashes() @@ -606,4 +730,25 @@ public function testNoExtraSlashes() $this->assertEquals('http://localtest.me/subfolder', (string) (new URI('localtest.me/subfolder'))); } + //-------------------------------------------------------------------- + + public function testSetSegment() + { + $base = 'http://example.com/foo/bar/baz'; + + $uri = new URI($base); + $uri->setSegment(2, 'banana'); + + $this->assertEquals('foo/banana/baz', $uri->getPath()); + } + + public function testSetBadSegment() + { + $this->expectException(HTTPException::class); + $base = 'http://example.com/foo/bar/baz'; + + $uri = new URI($base); + $uri->setSegment(6, 'banana'); + } + } From 1f4aefd6c56710dbefe656dbd9788044aae1fe3a Mon Sep 17 00:00:00 2001 From: Master Yoda Date: Sun, 11 Nov 2018 08:56:48 -0800 Subject: [PATCH 3/3] Beef up HTTP\Response testing --- system/HTTP/Response.php | 161 ++++++++++---------- tests/_support/HTTP/MockResponse.php | 18 ++- tests/system/HTTP/RedirectResponseTest.php | 36 +++-- tests/system/HTTP/ResponseTest.php | 169 ++++++++++++++++++--- 4 files changed, 270 insertions(+), 114 deletions(-) diff --git a/system/HTTP/Response.php b/system/HTTP/Response.php index 127f667c21ce..e0d528f7f5bb 100644 --- a/system/HTTP/Response.php +++ b/system/HTTP/Response.php @@ -1,4 +1,5 @@ - 'Misdirected Request', // http://www.iana.org/go/rfc7540 Section 9.1.2 422 => 'Unprocessable Entity', // http://www.iana.org/go/rfc4918 423 => 'Locked', // http://www.iana.org/go/rfc4918 - 424 => 'Failed Dependency', // http://www.iana.org/go/rfc4918 - 425 => 'Too Early', // https://datatracker.ietf.org/doc/draft-ietf-httpbis-replay/ + 424 => 'Failed Dependency', // http://www.iana.org/go/rfc4918 + 425 => 'Too Early', // https://datatracker.ietf.org/doc/draft-ietf-httpbis-replay/ 426 => 'Upgrade Required', 428 => 'Precondition Required', // 1.1; http://www.ietf.org/rfc/rfc6585.txt 429 => 'Too Many Requests', // 1.1; http://www.ietf.org/rfc/rfc6585.txt @@ -154,14 +155,14 @@ class Response extends Message implements ResponseInterface /** * The current status code for this response. * - * @var int + * @var integer */ protected $statusCode = 200; /** * Whether Content Security Policy is being enforced. * - * @var bool + * @var boolean */ protected $CSPEnabled = false; @@ -196,14 +197,14 @@ class Response extends Message implements ResponseInterface /** * Cookie will only be set if a secure HTTPS connection exists. * - * @var bool + * @var boolean */ protected $cookieSecure = false; /** * Cookie will only be accessible via HTTP(S) (no javascript) * - * @var bool + * @var boolean */ protected $cookieHTTPOnly = false; @@ -217,7 +218,7 @@ class Response extends Message implements ResponseInterface /** * If true, will not write output. Useful during testing. * - * @var bool + * @var boolean */ protected $pretend = false; @@ -264,7 +265,7 @@ public function __construct($config) /** * Turns "pretend" mode on or off to aid in testing. * - * @param bool $pretend + * @param boolean $pretend * * @return $this */ @@ -281,7 +282,7 @@ public function pretend(bool $pretend = true) * The status code is a 3-digit integer result code of the getServer's attempt * to understand and satisfy the request. * - * @return int Status code. + * @return integer Status code. */ public function getStatusCode(): int { @@ -304,10 +305,10 @@ public function getStatusCode(): int * @see http://tools.ietf.org/html/rfc7231#section-6 * @see http://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml * - * @param int $code The 3-digit integer result code to set. - * @param string $reason The reason phrase to use with the - * provided status code; if none is provided, will - * default to the IANA name. + * @param integer $code The 3-digit integer result code to set. + * @param string $reason The reason phrase to use with the + * provided status code; if none is provided, will + * default to the IANA name. * * @return self * @throws \InvalidArgumentException For invalid status code arguments. @@ -360,7 +361,6 @@ public function getReason(): string return $this->reason; } - //-------------------------------------------------------------------- //-------------------------------------------------------------------- // Convenience Methods //-------------------------------------------------------------------- @@ -376,7 +376,7 @@ public function setDate(\DateTime $date) { $date->setTimezone(new \DateTimeZone('UTC')); - $this->setHeader('Date', $date->format('D, d M Y H:i:s').' GMT'); + $this->setHeader('Date', $date->format('D, d M Y H:i:s') . ' GMT'); return $this; } @@ -397,7 +397,7 @@ public function setContentType(string $mime, string $charset = 'UTF-8') // add charset attribute if not already there and provided as parm if ((strpos($mime, 'charset=') < 1) && ! empty($charset)) { - $mime .= '; charset='.$charset; + $mime .= '; charset=' . $charset; } $this->removeHeader('Content-Type'); // replace existing content type @@ -433,9 +433,9 @@ public function getJSON() { $body = $this->body; - if ($this->bodyFormat != 'json') + if ($this->bodyFormat !== 'json') { - $config = config(Format::class); + $config = config(Format::class); $formatter = $config->getFormatter('application/json'); $body = $formatter->format($body); @@ -471,9 +471,9 @@ public function getXML() { $body = $this->body; - if ($this->bodyFormat != 'xml') + if ($this->bodyFormat !== 'xml') { - $config = config(Format::class); + $config = config(Format::class); $formatter = $config->getFormatter('application/xml'); $body = $formatter->format($body); @@ -488,7 +488,7 @@ public function getXML() * Handles conversion of the of the data into the appropriate format, * and sets the correct Content-Type header for our response. * - * @param $body + * @param $body * @param string $format Valid: json, xml * * @return mixed @@ -512,7 +512,6 @@ protected function formatBody($body, string $format) } //-------------------------------------------------------------------- - //-------------------------------------------------------------------- // Cache Control Methods // @@ -611,7 +610,7 @@ public function setLastModified($date) if ($date instanceof \DateTime) { $date->setTimezone(new \DateTimeZone('UTC')); - $this->setHeader('Last-Modified', $date->format('D, d M Y H:i:s').' GMT'); + $this->setHeader('Last-Modified', $date->format('D, d M Y H:i:s') . ' GMT'); } elseif (is_string($date)) { @@ -638,9 +637,10 @@ public function send() if ($this->CSPEnabled === true) { $this->CSP->finalize($this); - }else{ - - $this->body = str_replace(['{csp-style-nonce}','{csp-script-nonce}'], '', $this->body); + } + else + { + $this->body = str_replace(['{csp-style-nonce}', '{csp-script-nonce}'], '', $this->body); } $this->sendHeaders(); @@ -673,13 +673,12 @@ public function sendHeaders() } // HTTP Status - header(sprintf('HTTP/%s %s %s', $this->protocolVersion, $this->statusCode, $this->reason), true, - $this->statusCode); + header(sprintf('HTTP/%s %s %s', $this->protocolVersion, $this->statusCode, $this->reason), true, $this->statusCode); // Send all of our headers foreach ($this->getHeaders() as $name => $values) { - header($name.': '.$this->getHeaderLine($name), false, $this->statusCode); + header($name . ': ' . $this->getHeaderLine($name), false, $this->statusCode); } return $this; @@ -714,39 +713,41 @@ public function getBody() /** * Perform a redirect to a new URL, in two flavors: header or location. * - * @param string $uri The URI to redirect to - * @param string $method - * @param int $code The type of redirection, defaults to 302 + * @param string $uri The URI to redirect to + * @param string $method + * @param integer $code The type of redirection, defaults to 302 * * @return $this * @throws \CodeIgniter\HTTP\RedirectException */ public function redirect(string $uri, string $method = 'auto', int $code = null) { + // Assume 302 status code response; override if needed + if (empty($code)) + { + $code = 302; + } + // IIS environment likely? Use 'refresh' for better compatibility - if ($method === 'auto' && isset($_SERVER['SERVER_SOFTWARE']) - && strpos($_SERVER['SERVER_SOFTWARE'], 'Microsoft-IIS') !== false) + if ($method === 'auto' && isset($_SERVER['SERVER_SOFTWARE']) && strpos($_SERVER['SERVER_SOFTWARE'], 'Microsoft-IIS') !== false) { $method = 'refresh'; } - elseif ($method !== 'refresh' && (empty($code) || ! is_numeric($code))) + + // override status code for HTTP/1.1 & higher + // reference: http://en.wikipedia.org/wiki/Post/Redirect/Get + if (isset($_SERVER['SERVER_PROTOCOL'], $_SERVER['REQUEST_METHOD']) && $this->getProtocolVersion() >= 1.1) { - if (isset($_SERVER['SERVER_PROTOCOL'], $_SERVER['REQUEST_METHOD']) && $this->getProtocolVersion() >= 1.1) + if ($method !== 'refresh') { - $code = ($_SERVER['REQUEST_METHOD'] !== 'GET') ? 303 - // reference: http://en.wikipedia.org/wiki/Post/Redirect/Get - : 307; - } - else - { - $code = 302; + $code = ($_SERVER['REQUEST_METHOD'] !== 'GET') ? 303 : 307; } } switch ($method) { case 'refresh': - $this->setHeader('Refresh', '0;url='.$uri); + $this->setHeader('Refresh', '0;url=' . $uri); break; default: $this->setHeader('Location', $uri); @@ -768,14 +769,14 @@ public function redirect(string $uri, string $method = 'auto', int $code = null) * Accepts an arbitrary number of binds (up to 7) or an associateive * array in the first parameter containing all the values. * - * @param string|array $name Cookie name or array containing binds - * @param string $value Cookie value - * @param string $expire Cookie expiration time in seconds - * @param string $domain Cookie domain (e.g.: '.yourdomain.com') - * @param string $path Cookie path (default: '/') - * @param string $prefix Cookie name prefix - * @param bool|false $secure Whether to only transfer cookies via SSL - * @param bool|false $httponly Whether only make the cookie accessible via HTTP (no javascript) + * @param string|array $name Cookie name or array containing binds + * @param string $value Cookie value + * @param string $expire Cookie expiration time in seconds + * @param string $domain Cookie domain (e.g.: '.yourdomain.com') + * @param string $path Cookie path (default: '/') + * @param string $prefix Cookie name prefix + * @param boolean|false $secure Whether to only transfer cookies via SSL + * @param boolean|false $httponly Whether only make the cookie accessible via HTTP (no javascript) */ public function setCookie( $name, @@ -786,7 +787,8 @@ public function setCookie( $prefix = '', $secure = false, $httponly = false - ) { + ) + { if (is_array($name)) { // always leave 'name' in last place, as the loop will break otherwise, due to $$item @@ -804,7 +806,7 @@ public function setCookie( $prefix = $this->cookiePrefix; } - if ($domain == '' && $this->cookieDomain != '') + if ($domain === '' && $this->cookieDomain !== '') { $domain = $this->cookieDomain; } @@ -826,15 +828,15 @@ public function setCookie( if (! is_numeric($expire)) { - $expire = time()-86500; + $expire = time() - 86500; } else { - $expire = ($expire > 0) ? time()+$expire : 0; + $expire = ($expire > 0) ? time() + $expire : 0; } $this->cookies[] = [ - 'name' => $prefix.$name, + 'name' => $prefix . $name, 'value' => $value, 'expires' => $expire, 'path' => $path, @@ -855,7 +857,7 @@ public function setCookie( * @param null $value * @param string $prefix * - * @return bool + * @return boolean */ public function hasCookie(string $name, $value = null, string $prefix = '') { @@ -864,11 +866,11 @@ public function hasCookie(string $name, $value = null, string $prefix = '') $prefix = $this->cookiePrefix; } - $name = $prefix.$name; + $name = $prefix . $name; foreach ($this->cookies as $cookie) { - if ($cookie['name'] != $prefix.$name) + if ($cookie['name'] !== $prefix . $name) { continue; } @@ -878,7 +880,7 @@ public function hasCookie(string $name, $value = null, string $prefix = '') return true; } - return $cookie['value'] == $value; + return $cookie['value'] === $value; } return false; @@ -899,11 +901,11 @@ public function getCookie(string $name, string $prefix = '') $prefix = $this->cookiePrefix; } - $name = $prefix.$name; + $name = $prefix . $name; foreach ($this->cookies as $cookie) { - if ($cookie['name'] == $name) + if ($cookie['name'] === $name) { return $cookie; } @@ -913,7 +915,7 @@ public function getCookie(string $name, string $prefix = '') /** * Sets a cookie to be deleted when the response is sent. * - * @param $name + * @param $name * @param string $domain * @param string $path * @param string $prefix @@ -925,13 +927,13 @@ public function deleteCookie($name, string $domain = '', string $path = '/', str $prefix = $this->cookiePrefix; } - $name = $prefix.$name; + $name = $prefix . $name; foreach ($this->cookies as &$cookie) { - if ($cookie['name'] == $name) + if ($cookie['name'] === $name) { - $cookie['value'] = ''; + $cookie['value'] = ''; $cookie['expires'] = ''; break; @@ -966,16 +968,15 @@ protected function sendCookies() * Generates the headers that force a download to happen. And * sends the file to the browser. * - * @param string $filename The path to the file to send - * @param string $data The data to be downloaded - * @param bool $setMime Whether to try and send the actual MIME type + * @param string $filename The path to the file to send + * @param string $data The data to be downloaded + * @param boolean $setMime Whether to try and send the actual MIME type */ public function download(string $filename = '', $data = '', bool $setMime = false) { if ($filename === '' || $data === '') { - // @todo: Should I throw an exception? - return; + return null; } $filepath = ''; diff --git a/tests/_support/HTTP/MockResponse.php b/tests/_support/HTTP/MockResponse.php index 8b2331c006c2..8f31840b4b5d 100755 --- a/tests/_support/HTTP/MockResponse.php +++ b/tests/_support/HTTP/MockResponse.php @@ -1,4 +1,5 @@ -pretend; + } + + // artificial error for testing + public function misbehave() + { + $this->statusCode = 0; + } + } diff --git a/tests/system/HTTP/RedirectResponseTest.php b/tests/system/HTTP/RedirectResponseTest.php index f44f763c4ee3..a9d24d621066 100644 --- a/tests/system/HTTP/RedirectResponseTest.php +++ b/tests/system/HTTP/RedirectResponseTest.php @@ -13,7 +13,9 @@ class RedirectResponseTest extends \CIUnitTestCase { - /** @var RouteCollection */ + /** + * @var RouteCollection + */ protected $routes; protected $request; protected $config; @@ -24,7 +26,7 @@ public function setUp() $_SERVER['REQUEST_METHOD'] = 'GET'; - $this->config = new App(); + $this->config = new App(); $this->config->baseURL = 'http://example.com'; $this->routes = new RouteCollection(new MockFileLocator(new Autoload()), new \Config\Modules()); @@ -34,6 +36,8 @@ public function setUp() Services::injectMock('request', $this->request); } + //-------------------------------------------------------------------- + public function testRedirectToFullURI() { $response = new RedirectResponse(new App()); @@ -44,6 +48,8 @@ public function testRedirectToFullURI() $this->assertEquals('http://example.com/foo', $response->getHeaderLine('Location')); } + //-------------------------------------------------------------------- + public function testRedirectRoute() { $response = new RedirectResponse(new App()); @@ -74,6 +80,8 @@ public function testRedirectRouteBad() $response->route('differentRoute'); } + //-------------------------------------------------------------------- + public function testRedirectRelativeConvertsToFullURI() { $response = new RedirectResponse($this->config); @@ -84,15 +92,17 @@ public function testRedirectRelativeConvertsToFullURI() $this->assertEquals('http://example.com/foo', $response->getHeaderLine('Location')); } + //-------------------------------------------------------------------- + /** * @runInSeparateProcess - * @preserveGlobalState disabled + * @preserveGlobalState disabled */ public function testWithInput() { $_SESSION = []; - $_GET = ['foo' => 'bar']; - $_POST = ['bar' => 'baz']; + $_GET = ['foo' => 'bar']; + $_POST = ['bar' => 'baz']; $response = new RedirectResponse(new App()); @@ -104,9 +114,11 @@ public function testWithInput() $this->assertEquals('baz', $_SESSION['_ci_old_input']['post']['bar']); } + //-------------------------------------------------------------------- + /** * @runInSeparateProcess - * @preserveGlobalState disabled + * @preserveGlobalState disabled */ public function testWithValidationErrors() { @@ -125,9 +137,11 @@ public function testWithValidationErrors() $this->assertArrayHasKey('_ci_validation_errors', $_SESSION); } + //-------------------------------------------------------------------- + /** * @runInSeparateProcess - * @preserveGlobalState disabled + * @preserveGlobalState disabled */ public function testWith() { @@ -141,14 +155,16 @@ public function testWith() $this->assertArrayHasKey('foo', $_SESSION); } + //-------------------------------------------------------------------- + /** * @runInSeparateProcess - * @preserveGlobalState disabled + * @preserveGlobalState disabled */ public function testRedirectBack() { $_SERVER['HTTP_REFERER'] = 'http://somewhere.com'; - $this->request = new MockIncomingRequest($this->config, new URI('http://somewhere.com'), null, new UserAgent()); + $this->request = new MockIncomingRequest($this->config, new URI('http://somewhere.com'), null, new UserAgent()); Services::injectMock('request', $this->request); $response = new RedirectResponse(new App()); @@ -159,7 +175,7 @@ public function testRedirectBack() /** * @runInSeparateProcess - * @preserveGlobalState disabled + * @preserveGlobalState disabled */ public function testRedirectBackMissing() { diff --git a/tests/system/HTTP/ResponseTest.php b/tests/system/HTTP/ResponseTest.php index 272d31b75594..c865068998ef 100644 --- a/tests/system/HTTP/ResponseTest.php +++ b/tests/system/HTTP/ResponseTest.php @@ -1,13 +1,27 @@ -server = $_SERVER; + } + + public function tearDown() + { + $_SERVER = $this->server; + } + public function testCanSetStatusCode() { $response = new Response(new App()); @@ -29,6 +43,16 @@ public function testSetStatusCodeThrowsExceptionForBadCodes() //-------------------------------------------------------------------- + public function testConstructWithCSPEnabled() + { + $config = new App(); + $config->CSPEnabled = true; + $response = new Response($config); + + $this->assertTrue($response instanceof Response); + } + + //-------------------------------------------------------------------- public function testSetStatusCodeSetsReason() { @@ -127,7 +151,7 @@ public function testSetDateRemembersDateInUTC() $header = $response->getHeaderLine('Date'); - $this->assertEquals($date->format('D, d M Y H:i:s').' GMT', $header); + $this->assertEquals($date->format('D, d M Y H:i:s') . ' GMT', $header); } //-------------------------------------------------------------------- @@ -161,9 +185,9 @@ public function testSetCache() $date = date('r'); $options = [ - 'etag' => '12345678', + 'etag' => '12345678', 'last-modified' => $date, - 'max-age' => 300, + 'max-age' => 300, 'must-revalidate' ]; @@ -174,6 +198,19 @@ public function testSetCache() $this->assertEquals('max-age=300, must-revalidate', $response->getHeaderLine('Cache-Control')); } + public function testSetCacheNoOptions() + { + $response = new Response(new App()); + + $date = date('r'); + + $options = []; + + $response->setCache($options); + + $this->assertEquals('no-store, max-age=0, no-cache', $response->getHeaderLine('Cache-Control')); + } + //-------------------------------------------------------------------- public function testSetLastModifiedWithDateTimeObject() @@ -187,7 +224,7 @@ public function testSetLastModifiedWithDateTimeObject() $header = $response->getHeaderLine('Last-Modified'); - $this->assertEquals($date->format('D, d M Y H:i:s').' GMT', $header); + $this->assertEquals($date->format('D, d M Y H:i:s') . ' GMT', $header); } //-------------------------------------------------------------------- @@ -218,6 +255,16 @@ public function testRedirectSetsCode() //-------------------------------------------------------------------- + public function testRedirectWithIIS() + { + $_SERVER['SERVER_SOFTWARE'] = 'Microsoft-IIS'; + $response = new Response(new App()); + $response->redirect('example.com', 'auto', 307); + $this->assertEquals('0;url=example.com', $response->getHeaderLine('Refresh')); + } + + //-------------------------------------------------------------------- + public function testSetCookieFails() { $response = new Response(new App()); @@ -250,15 +297,21 @@ public function testSetCookieSuccessOnPrefix() $this->assertFalse($response->hasCookie('foo', null, 'ack')); } + //-------------------------------------------------------------------- + public function testJSONWithArray() { - $response = new Response(new App()); - $config = new Format(); + $response = new Response(new App()); + $config = new Format(); $formatter = $config->getFormatter('application/json'); - $body = [ + $body = [ 'foo' => 'bar', - 'bar' => [1, 2, 3] + 'bar' => [ + 1, + 2, + 3, + ], ]; $expected = $formatter->format($body); @@ -270,13 +323,17 @@ public function testJSONWithArray() public function testJSONGetFromNormalBody() { - $response = new Response(new App()); - $config = new Format(); + $response = new Response(new App()); + $config = new Format(); $formatter = $config->getFormatter('application/json'); - $body = [ + $body = [ 'foo' => 'bar', - 'bar' => [1, 2, 3] + 'bar' => [ + 1, + 2, + 3, + ], ]; $expected = $formatter->format($body); @@ -285,15 +342,21 @@ public function testJSONGetFromNormalBody() $this->assertEquals($expected, $response->getJSON()); } + //-------------------------------------------------------------------- + public function testXMLWithArray() { - $response = new Response(new App()); - $config = new Format(); + $response = new Response(new App()); + $config = new Format(); $formatter = $config->getFormatter('application/xml'); - $body = [ + $body = [ 'foo' => 'bar', - 'bar' => [1, 2, 3] + 'bar' => [ + 1, + 2, + 3, + ], ]; $expected = $formatter->format($body); @@ -305,13 +368,17 @@ public function testXMLWithArray() public function testXMLGetFromNormalBody() { - $response = new Response(new App()); - $config = new Format(); + $response = new Response(new App()); + $config = new Format(); $formatter = $config->getFormatter('application/xml'); - $body = [ + $body = [ 'foo' => 'bar', - 'bar' => [1, 2, 3] + 'bar' => [ + 1, + 2, + 3, + ], ]; $expected = $formatter->format($body); @@ -320,6 +387,8 @@ public function testXMLGetFromNormalBody() $this->assertEquals($expected, $response->getXML()); } + //-------------------------------------------------------------------- + public function testGetDownloadResponseByData() { $response = new Response(new App()); @@ -346,7 +415,7 @@ public function testGetDownloadResponseByFilePath() $this->assertInstanceOf(DownloadResponse::class, $actual); $actual->buildHeaders(); - $this->assertSame('attachment; filename="'.basename(__FILE__).'"; filename*=UTF-8\'\''.basename(__FILE__), $actual->getHeaderLine('Content-Disposition')); + $this->assertSame('attachment; filename="' . basename(__FILE__) . '"; filename*=UTF-8\'\'' . basename(__FILE__), $actual->getHeaderLine('Content-Disposition')); ob_start(); $actual->sendBody(); @@ -355,4 +424,60 @@ public function testGetDownloadResponseByFilePath() $this->assertSame(file_get_contents(__FILE__), $actual_output); } + + public function testVagueDownload() + { + $response = new Response(new App()); + + $actual = $response->download(); + + $this->assertNull($actual); + } + + //-------------------------------------------------------------------- + + public function testPretendMode() + { + $response = new MockResponse(new App()); + $response->pretend(true); + $this->assertTrue($response->getPretend()); + $response->pretend(false); + $this->assertFalse($response->getPretend()); + } + + public function testMisbehaving() + { + $response = new MockResponse(new App()); + $response->misbehave(); + + $this->expectException(HTTPException::class); + $response->getStatusCode(); + } + + //-------------------------------------------------------------------- + + public function testTemporaryRedirect11() + { + $_SERVER['SERVER_PROTOCOL'] = 'HTTP/1.1'; + $_SERVER['REQUEST_METHOD'] = 'POST'; + $response = new Response(new App()); + + $response->setProtocolVersion('HTTP/1.1'); + $response->redirect('/foo'); + + $this->assertEquals(303, $response->getStatusCode()); + } + + public function testTemporaryRedirectGet11() + { + $_SERVER['SERVER_PROTOCOL'] = 'HTTP/1.1'; + $_SERVER['REQUEST_METHOD'] = 'GET'; + $response = new Response(new App()); + + $response->setProtocolVersion('HTTP/1.1'); + $response->redirect('/foo'); + + $this->assertEquals(307, $response->getStatusCode()); + } + }