Merge pull request #41 from icewind1991/invalid-handle

Improve error handling in NativeShare->get

install_libsmbclient.sh

@@ -5,4 +5,4 @@ wget -O /tmp/libsmbclient-php.zip https://github.com/eduardok/libsmbclient-php/a
 unzip /tmp/libsmbclient-php.zip -d /tmp
 cd /tmp/libsmbclient-php-master
 phpize && ./configure && make && sudo make install
-echo 'extension="libsmbclient.so"' >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini
+echo 'extension="smbclient.so"' >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini

src/Exception/InvalidResourceException.php

@@ -0,0 +1,11 @@
+<?php
+/**
+ * Copyright (c) 2014 Robin Appelman <icewind@owncloud.com>
+ * This file is licensed under the Licensed under the MIT license:
+ * http://opensource.org/licenses/MIT
+ */
+
+namespace Icewind\SMB\Exception;
+
+class InvalidResourceException extends Exception {
+}

src/NativeShare.php

@@ -7,6 +7,9 @@
 
 namespace Icewind\SMB;
 
+use Icewind\SMB\Exception\InvalidPathException;
+use Icewind\SMB\Exception\InvalidResourceException;
+
 class NativeShare extends AbstractShare {
 	/**
 	 * @var Server $server
@@ -193,11 +196,30 @@ class NativeShare extends AbstractShare {
 	 *
 	 * @throws \Icewind\SMB\Exception\NotFoundException
 	 * @throws \Icewind\SMB\Exception\InvalidTypeException
+	 * @throws \Icewind\SMB\Exception\InvalidPathException
+	 * @throws \Icewind\SMB\Exception\InvalidResourceException
 	 */
 	public function get($source, $target) {
+		if (!$target) {
+			throw new InvalidPathException('Invalid target path: Filename cannot be empty');
+		}
+		$targetHandle = @fopen($target, 'wb');
+		if (!$targetHandle) {
+			$error = error_get_last();
+			if (is_array($error)) {
+				$reason = $error['message'];
+			} else {
+				$reason = 'Unknown error';
+			}
+			throw new InvalidResourceException('Failed opening local file "' . $target . '" for writing: ' . $reason);
+		}
+
 		$this->connect();
 		$sourceHandle = $this->state->open($this->buildUrl($source), 'r');
-		$targetHandle = fopen($target, 'wb');
+		if (!$sourceHandle) {
+			fclose($targetHandle);
+			throw new InvalidResourceException('Failed opening remote file "' . $source . '" for reading');
+		}
 
 		while ($data = $this->state->read($sourceHandle, 4096)) {
 			fwrite($targetHandle, $data);

src/Parser.php

@@ -11,6 +11,7 @@ use Icewind\SMB\Exception\AccessDeniedException;
 use Icewind\SMB\Exception\AlreadyExistsException;
 use Icewind\SMB\Exception\Exception;
 use Icewind\SMB\Exception\FileInUseException;
+use Icewind\SMB\Exception\InvalidResourceException;
 use Icewind\SMB\Exception\InvalidTypeException;
 use Icewind\SMB\Exception\NotEmptyException;
 use Icewind\SMB\Exception\NotFoundException;
@@ -42,6 +43,13 @@ class Parser {
 					$error = $part;
 				}
 			}
+
+			$notFoundMsg = 'Error opening local file ';
+			if (substr($output[0], 0, strlen($notFoundMsg)) === $notFoundMsg) {
+				$localPath = substr($output[0], strlen($notFoundMsg));
+				throw new InvalidResourceException('Failed opening local file "' . $localPath . '" for writing');
+			}
+
 			switch ($error) {
 				case ErrorCodes::PathNotFound:
 				case ErrorCodes::ObjectNotFound:

tests/AbstractShare.php

@@ -230,6 +230,20 @@ abstract class AbstractShare extends TestCase {
 		unlink($targetFile);
 	}
 
+	/**
+	 * @expectedException \Icewind\SMB\Exception\InvalidResourceException
+	 */
+	public function testGetInvalidTarget() {
+		$name = 'test.txt';
+		$text = 'dummy';
+		$tmpFile = $this->getTextFile($text);
+
+		$this->share->put($tmpFile, $this->root . '/' . $name);
+		unlink($tmpFile);
+
+		$this->share->get($this->root . '/' . $name, '/non/existing/file');
+	}
+
 	/**
 	 * @dataProvider nameProvider
 	 */