#7 Sanitizing sources

src/Grale/WebDav/Client.php

@@ -600,8 +600,9 @@ protected function resolveUrl($uri)
      *
      * @param HttpRequest $request The request
      *
+     * @throws Exception\NoSuchResourceException
+     * @throws Exception\HttpException
      * @return HttpResponse Returns the server response
-     * @throws HttpException If an HTTP error is returned
      */
     protected function doRequest(HttpRequest $request)
     {

src/Grale/WebDav/Exception/HttpException.php

@@ -89,23 +89,26 @@ public static function factory(BadResponseException $cause)
 
         if ($response->isClientError()) {
             $class = __NAMESPACE__ . '\\ClientFailureException';
-        } elseif ($response->isServerError()) {
+        } else {
             $class = __NAMESPACE__ . '\\ServerFailureException';
         }
 
-        $statusCode = $response->getStatusCode();
-        $httpMethod = $request->getMethod();
+        $exception = new $class($response->getReasonPhrase(), null, $cause);
 
-        $e = new $class($response->getReasonPhrase(), null, $cause);
-        $e->setStatusCode($statusCode);
-        $e->setResponse($response);
-        $e->setRequest($request);
+        if ($exception instanceof HttpException) {
+            $statusCode = $response->getStatusCode();
+            $httpMethod = $request->getMethod();
 
-        if (isset(self::$descriptionMapping[$httpMethod][$statusCode])) {
-            $e->setDescription(self::$descriptionMapping[$httpMethod][$statusCode]);
+            $exception->setStatusCode($statusCode);
+            $exception->setResponse($response);
+            $exception->setRequest($request);
+
+            if (isset(self::$descriptionMapping[$httpMethod][$statusCode])) {
+                $exception->setDescription(self::$descriptionMapping[$httpMethod][$statusCode]);
+            }
         }
 
-        return $e;
+        return $exception;
     }
 
     /**

src/Grale/WebDav/Lock.php

@@ -136,6 +136,8 @@ public function getDepth()
      * Accepted values are zero or {@link Header\DepthHeader::INFINITY}
      *
      * @param int $depth The depth of lock
+     *
+     * @throws \InvalidArgumentException
      */
     public function setDepth($depth)
     {
@@ -203,8 +205,11 @@ public function setTimeout($timeout)
     }
 
     /**
-     * @return self
+     * @param \DOMElement $element
+     *
      * @throws \InvalidArgumentException
+     * @return self
+     *
      * @todo define exception message
      */
     public static function fromXml(\DOMElement $element)
@@ -251,6 +256,7 @@ public static function fromXml(\DOMElement $element)
      * @param Client $client
      * @param string $xml
      *
+     * @throws \RuntimeException
      * @return self
      *
      * @todo

src/Grale/WebDav/MultiStatus.php

@@ -68,7 +68,7 @@ public function getDescription()
     }
 
     /**
-     * @return Iterator Returns an iterator to be used in loop functions
+     * @return \Iterator Returns an iterator to be used in loop functions
      */
     public function getIterator()
     {
@@ -85,8 +85,9 @@ public function count()
 
     /**
      * @param Client $client
-     * @param string $xml    The multi-status response as an XML string
+     * @param string $xml The multi-status response as an XML string
      *
+     * @throws \RuntimeException
      * @return self Returns the parsed multi-status response as an object
      *
      * @todo
@@ -158,7 +159,9 @@ public static function parse(Client $client, $xml)
 
     /**
      * @param string $statusLine
+     * @throws \Exception
      * @return int
+     * @todo define exception here
      */
     protected static function parseHttpStatus($statusLine)
     {

src/Grale/WebDav/Property/SupportedLock.php

@@ -55,8 +55,6 @@ public function getValue()
      */
     public function isLockable($type = null, $scope = null)
     {
-        $result = false;
-
         if ($type === null) {
             $result = count($this->capabilities) > 0;
         } elseif ($scope === null) {

src/Grale/WebDav/PropertySet.php

@@ -104,7 +104,7 @@ public function getNames()
     /**
      * Returns an iterator over all properties in this set.
      *
-     * @return Iterator An iterator over {@link PropertyInterface}
+     * @return \Iterator An iterator over {@link PropertyInterface}
      */
     public function getIterator()
     {

src/Grale/WebDav/Resource.php

@@ -35,6 +35,7 @@ class Resource implements LockableInterface
     /**
      * @param string $href
      * @param PropertySet|array
+     * @throws \InvalidArgumentException
      */
     public function __construct($href, $properties = null)
     {

src/Grale/WebDav/Response.php

@@ -48,7 +48,7 @@ class Response
     protected $status;
 
     /**
-     * @var array A list of resource properties, grouped by HTTP status code
+     * @var PropertySet[] A list of resource properties, grouped by HTTP status code
      */
     protected $properties = array();
 

src/Grale/WebDav/StreamWrapper.php

@@ -55,7 +55,7 @@ class StreamWrapper
     protected $stream;
 
     /**
-     * @var Iterator An iterator used to iterate the <tt>response</tt> elements of a WebDAV multi-status response
+     * @var \Iterator An iterator used to iterate the <tt>response</tt> elements of a WebDAV multi-status response
      */
     protected $iterator;
 
@@ -166,7 +166,7 @@ public static function getDefaultClient()
      */
     public function stream_open($path, $mode, $options, &$openedPath)
     {
-        $result = false;
+        $url = null;
 
         try {
             // We don't care about text-mode translation and binary-mode flags
@@ -334,8 +334,6 @@ public function stream_seek($offset, $whence = SEEK_SET)
      */
     public function stream_lock($operation)
     {
-        $result = false;
-
         try {
             // We don't care about LOCK_NB
             $operation = $operation & ~LOCK_NB;
@@ -410,8 +408,6 @@ public function stream_close()
      */
     public function rename($old, $new)
     {
-        $result = false;
-
         try {
             // Retrieve the context options
             $this->setClientConfig();
@@ -441,8 +437,6 @@ public function rename($old, $new)
      */
     public function unlink($path)
     {
-        $result = false;
-
         try {
             // Retrieve the context options
             $this->setClientConfig();
@@ -477,8 +471,6 @@ public function unlink($path)
      */
     public function mkdir($path, $mode, $options)
     {
-        $result = false;
-
         try {
             if ($options & STREAM_MKDIR_RECURSIVE) {
                 throw new \RuntimeException('WebDAV stream wrapper does not allow to create directories recursively');
@@ -516,8 +508,6 @@ public function mkdir($path, $mode, $options)
      */
     public function rmdir($path, $options)
     {
-        $result = false;
-
         try {
             // Retrieve the context options
             $this->setClientConfig();
@@ -591,7 +581,7 @@ public function dir_readdir()
             $result   = $resource->getFilename();
 
             // Cache the resource statistics for quick url_stat lookups
-            $url = (string)$this->openedPath->combine($resource->getHref());
+            $url = (string)Url::factory($this->openedPath)->combine($resource->getHref());
             self::$statCache[$url] = $resource->getStat();
 
             $this->iterator->next();
@@ -764,8 +754,6 @@ protected function lock($scope)
      */
     protected function releaseLock()
     {
-        $result = false;
-
         try {
             $result = self::$client->releaseLock($this->openedPath, $this->locktoken);
             $this->locktoken = null;
@@ -806,7 +794,7 @@ protected function openAppendMode($url)
 
         } catch (NoSuchResourceException $exception) {
             // The resource does not exist, so use a simple write stream
-            return $this->openWriteOnly($path);
+            return $this->openWriteOnly($url);
         }
 
         return true;
@@ -869,10 +857,10 @@ protected function clearStatCache($path = null)
      * Trigger an error
      *
      * @param \Exception|string $error Error to trigger
-     * @param int               $quiet If set to true, then no error or exception occurs
+     * @param bool|int          $quiet If set to true, then no error or exception occurs
      *
+     * @throws Exception\StreamException
      * @return bool Returns false
-     * @throws StreamException if throw_exceptions is true
      */
     protected function triggerError($error, $quiet = false)
     {

tests/Grale/WebDav/ClientTest.php

@@ -10,6 +10,7 @@
 
 namespace Grale\WebDav;
 
+use Guzzle\Http\Message\Request;
 use Guzzle\Http\Message\Response;
 use Guzzle\Http\Message\RequestFactory;
 use Guzzle\Http\Exception\BadResponseException;
@@ -257,7 +258,8 @@ public function testMoveLockedCollection()
         $this->assertContains('Destination: http://www.foo.bar/othercontainer/', $request);
         $this->assertContains(
             'If: (<opaquelocktoken:fe184f2e-6eec-41d0-c765-01adc56e6bb4>)' .
-               ' (<opaquelocktoken:e454f3f3-acdc-452a-56c7-00a5c91e4b77>)', $request
+            ' (<opaquelocktoken:e454f3f3-acdc-452a-56c7-00a5c91e4b77>)',
+            $request
         );
     }
 
@@ -468,7 +470,6 @@ public function testSimpleLockRequest()
             'timeout' => 4100000000
         ));
 
-        $status  = $client->getLastResponseStatus();
         $request = $client->getLastRequest();
 
         $this->assertInstanceOf('Grale\\WebDav\\Lock', $lock);
@@ -506,7 +507,7 @@ public function testRefreshingWriteLock()
             'opaquelocktoken:e71d4fae-5dec-22d6-fea5-00a0c91e6be4',
             4100000000
         );
-        $status  = $client->getLastResponseStatus();
+
         $request = $client->getLastRequest();
 
         $this->assertInstanceOf('Grale\\WebDav\\Lock', $result);
@@ -592,6 +593,7 @@ public function testUnlock()
      * @param  string $name
      * @param  bool   $asString
      *
+     * @throws \RuntimeException
      * @return Request|Response
      */
     protected function getFixture($name, $asString = false)

tests/Grale/WebDav/LockTest.php

@@ -18,6 +18,9 @@
  */
 class LockTest extends \PHPUnit_Framework_TestCase
 {
+    /**
+     * @var Lock
+     */
     protected $lock;
 
     public function setUp()
@@ -113,7 +116,7 @@ public function testInvalidLockScope()
     public function testFromInvalidXml()
     {
         $dom = new \DOMDocument();
-        $dom->loadXML('<?xml version="1.0" encoding="utf-8"?><element/>');
+        $dom->loadXML('<?xml version="1.0" encoding="utf-8"?><html></html>');
         Lock::fromXml($dom->documentElement);
     }
 

tests/Grale/WebDav/Property/LockDiscoveryTest.php

@@ -17,6 +17,9 @@
  */
 class LockDiscoveryTest extends \PHPUnit_Framework_TestCase
 {
+    /**
+     * @var LockDiscovery
+     */
     protected $property;
 
     public function setUp()

tests/Grale/WebDav/PropertySetTest.php

@@ -21,6 +21,9 @@ class PropertySetTest extends \PHPUnit_Framework_TestCase
         'D:getcontenttype'
     );
 
+    /**
+     * @var PropertySet
+     */
     protected $props;
 
     public function setUp()
@@ -58,7 +61,10 @@ public function testOffsetExists()
     public function testOffsetUnset()
     {
         unset($this->props['R:bigbox']);
-        $this->assertFalse(isset($this->props['R:bigbox']), 'Unable to assert that R:bigbox property has been deleted!');
+        $this->assertFalse(
+            isset($this->props['R:bigbox']),
+            'Unable to assert that R:bigbox property has been deleted!'
+        );
     }
 
     /** Testing Countable **/

tests/Grale/WebDav/ResourceTest.php

@@ -20,8 +20,19 @@
  */
 class ResourceTest extends \PHPUnit_Framework_TestCase
 {
+    /**
+     * @var \Grale\WebDav\Resource
+     */
     protected $collection;
+
+    /**
+     * @var \Grale\WebDav\Resource
+     */
     protected $resource;
+
+    /**
+     * @var Lock
+     */
     protected $lock;
 
     public function setUp()
@@ -160,7 +171,10 @@ public function testHasLockToken()
 
     public function testGetLock()
     {
-        $this->assertEquals($this->lock, $this->resource->getLock('opaquelocktoken:e71d4fae-5dec-22d6-fea5-00a0c91e6be4'));
+        $this->assertEquals(
+            $this->lock,
+            $this->resource->getLock('opaquelocktoken:e71d4fae-5dec-22d6-fea5-00a0c91e6be4')
+        );
     }
 
     public function testGetNoLocks()