Reverted the script source getting addition, and in fact completely removed the concept altogether. File contents is fetched from the output of executing ":error" (which produces a !trap reply with the message in it), and this same mechanism can be used for getting output of scripts in general;

Also added tests for all previously untested features, and added two special case groups for testing encrypted connections' initialization.

Author: boenrobot

RELEASE-1.0.0b6

@@ -2,7 +2,9 @@ Util stuff, mostly.
 
 * __BREAKING CHANGES:__
     - Removed all $mode arguments from all Countable implementations (turns out no stable 5.6+ releases uses it...), and the COUNT_RECURSIVE implementations (which were mostly useless anyway). Util::count() has the $query argument as its first and only one.
+    - All Util CRUD methods throw RouterErrorException when the router returns an error. Previously, you'd need to inspect the returned value to check if there's an error.
     - Util::edit() is no longer an alias of Util::set(). It's now its own method that can modify or unset a single specified property.
+    - Util::fileGetContents() now throws RouterErrorException if the file doesn't exist or if there are other problems with getting its contents. Previously, it returned FALSE for such cases.
     - Util's escapeValue(), escapeString(), parseValue(), prepareScript() and appendScript() methods are now in their own new class called "Script", with the last two now being called just prepare() and append().
     - Script::escapeValue() now converts DateTime objects into a string having the "M/d/Y H:i:s" format (used across RouterOS), or just "M/d/Y" if the time is exactly midnight, and the timezone is UTC. The old behaviour can be achieved by "manually" adding the DateTime to "new DateTime('@0')".
 * New Util methods:
@@ -11,7 +13,6 @@ Util stuff, mostly.
     - newRequest()
 * Script::parseValue() now supports letter notation for time (1h2m3s), not just double colon notation (01:02:03), modeled after RouterOS. Related to that is also that leading zeroes, and zero minutes and seconds are now optional (e.g. "1:" is a valid way of saying 1 hour). Sub-second information is rounded up to the nearest second on current PHP versions (future versions are expected to support sub-second information in DateInterval by allowing seconds to be a double; The code currently attempts to give DateInterval a double, falling back to rounding to a second).
 * Script::parseValue() now recognizes dates in the "M/d/Y H:i:s" format (used across RouterOS), and turns that into a DateTime object for that time (or midnight in UTC if the time part is omitted).
-* Util::exec() now returns the script source after execution. Previously, this was a private option, but is now the only way, and it's public. Intended to be used for easier output extraction.
 * Util::getAll() now throws a NotSupportedException if the arguments "follow", "follow-only" or "count-only" are used. The first two, because PHP would hang (since Client::sendSync() is used under the hood), and the last one because it's unredable in the returned output (use Util::count() instead).
 * Util::setMenu() can now go back to the root menu.
 * Util::get() can now accept a query as an argument for $number

src/PEAR2/Net/RouterOS/RouterErrorException.php

@@ -59,10 +59,10 @@ class RouterErrorException extends RuntimeException implements Exception
     const CODE_COMMENT_ERROR        = 0x120042;
     const CODE_UNSET_ERROR          = 0x120082;
     const CODE_MOVE_ERROR           = 0x120107;
-    const CODE_SCRIPT_GET_ERROR     = 0x210001;
     const CODE_SCRIPT_ADD_ERROR     = 0x220001;
     const CODE_SCRIPT_REMOVE_ERROR  = 0x220004;
     const CODE_SCRIPT_RUN_ERROR     = 0x240001;
+    const CODE_SCRIPT_FILE_ERROR    = 0x240003;
 
     /**
      * @var ResponseCollection|null The complete response returned by the router.

src/PEAR2/Net/RouterOS/Util.php

@@ -126,7 +126,7 @@ public function setMenu($newMenu)
             } elseif ('/' === $newMenu[0]) {
                 $this->menu = $menuRequest->setCommand($newMenu)->getCommand();
             } else {
-                $newMenu = substr(
+                $newMenu = (string)substr(
                     $menuRequest->setCommand(
                         '/' .
                         str_replace('/', ' ', (string)substr($this->menu, 1)) .
@@ -235,16 +235,13 @@ public function newRequest(
      *     To eliminate any possibility of name clashes,
      *     you can specify your own name instead.
      *
-     * @return string|resource The source of the script, as it appears after
-     *     the run, right before it is removed.
-     *     This can be used for easily retrieving basic output,
-     *     by modifying the script from inside the script
-     *     (use the $"_" variable to refer to the script's name within the
-     *     "/system script" menu).
+     * @return ResponseCollection The responses of all requests involved, i.e.
+     *     the add, the run and the remove.
      * 
      * @throws RouterErrorException When there is an error in any step of the
      *     way. The reponses include all successful commands prior to the error
-     *     as well.
+     *     as well. If the error occurs during the run, there will also be a
+     *     remove attempt, and the results will include its results as well.
      */
     public function exec(
         $source,
@@ -286,56 +283,46 @@ public function exec(
         $request = new Request('/system/script/run');
         $request->setArgument('number', $name);
         $runResult = $this->client->sendSync($request);
+        $request = new Request('/system/script/remove');
+        $request->setArgument('numbers', $name);
+        $removeResult = $this->client->sendSync($request);
+
         if (count($runResult->getAllOfType(Response::TYPE_ERROR)) > 0) {
             throw new RouterErrorException(
                 'Error when running script',
                 RouterErrorException::CODE_SCRIPT_RUN_ERROR,
                 null,
-                new ResponseCollection(
-                    array_merge($addResult->toArray(), $runResult->toArray())
-                )
-            );
-        }
-
-        $request = new Request('/system/script/get');
-        $request->setArgument('number', $name);
-        $request->setArgument('value-name', 'source');
-        $getResult = $this->client->sendSync($request);
-        if (count($getResult->getAllOfType(Response::TYPE_ERROR)) > 0) {
-            throw new RouterErrorException(
-                'Error when getting script source',
-                RouterErrorException::CODE_SCRIPT_GET_ERROR,
-                null,
                 new ResponseCollection(
                     array_merge(
                         $addResult->toArray(),
                         $runResult->toArray(),
-                        $getResult->toArray()
+                        $removeResult->toArray()
                     )
                 )
             );
         }
-        $postSource = $getResult->end()->getProperty('ret');
-        
-        $request = new Request('/system/script/remove');
-        $request->setArgument('numbers', $name);
-        $removeResult = $this->client->sendSync($request);
         if (count($removeResult->getAllOfType(Response::TYPE_ERROR)) > 0) {
             throw new RouterErrorException(
-                'Error when getting script source',
-                RouterErrorException::CODE_SCRIPT_GET_ERROR,
+                'Error when removing script',
+                RouterErrorException::CODE_SCRIPT_REMOVE_ERROR,
                 null,
                 new ResponseCollection(
                     array_merge(
                         $addResult->toArray(),
                         $runResult->toArray(),
-                        $getResult->toArray(),
                         $removeResult->toArray()
                     )
                 )
             );
         }
-        return $postSource;
+
+        return new ResponseCollection(
+            array_merge(
+                $addResult->toArray(),
+                $runResult->toArray(),
+                $removeResult->toArray()
+            )
+        );
     }
 
     /**
@@ -535,9 +522,6 @@ public function find()
      *     considered the target item.
      * @param string|null           $valueName The name of the value to get.
      *     If omitted, or set to NULL, gets all properties of the target item.
-     *     Note that for versions that don't support omitting $valueName
-     *     natively, a "print" with "detail" argument is used as a fallback,
-     *     which may not contain certain properties. 
      *
      * @return string|resource|null|array The value of the specified
      *     property as a string or as new PHP temp stream if the underlying
@@ -551,7 +535,10 @@ public function find()
      */
     public function get($number, $valueName = null)
     {
-        if (is_int($number) || ((string)$number === (string)(int)$number)) {
+        if ($number instanceof Query) {
+            $number = explode(',', $this->find($number));
+            $number = $number[0];
+        } elseif (is_int($number) || ((string)$number === (string)(int)$number)) {
             $this->find();
             if (isset($this->idCache[(int)$number])) {
                 $number = $this->idCache[(int)$number];
@@ -561,9 +548,6 @@ public function get($number, $valueName = null)
                     RouterErrorException::CODE_CACHE_ERROR
                 );
             }
-        } elseif ($number instanceof Query) {
-            $number = explode(',', $this->find($number));
-            $number = $number[0];
         }
 
         $request = new Request($this->menu . '/get');
@@ -584,12 +568,15 @@ public function get($number, $valueName = null)
             $result = stream_get_contents($result);
         }
         if (null === $valueName) {
+            // @codeCoverageIgnoreStart
             //Some earlier RouterOS versions use "," instead of ";" as separator
+            //Newer versions can't possibly enter this condition
             if (false === strpos($result, ';')
-                && 1 === preg_match('/^([^=,]+\=[^=,]*)(?:\,(?1))+$/', $result)
+                && preg_match('/^([^=,]+\=[^=,]*)(?:\,(?1))+$/', $result)
             ) {
                 $result = str_replace(',', ';', $result);
             }
+            // @codeCoverageIgnoreEnd
             return Script::parseValueToArray('{' . $result . '}');
         }
         return $result;
@@ -1120,29 +1107,45 @@ public function filePutContents($filename, $data, $overwrite = false)
      *     To eliminate any possibility of name clashes,
      *     you can specify your own name instead.
      *
-     * @return string|resource|false The contents of the file as a string or as
+     * @return string|resource The contents of the file as a string or as
      *     new PHP temp stream if the underlying
      *     {@link Client::isStreamingResponses()} is set to TRUE.
-     *     FALSE is returned if there is no such file.
      *
      * @throws RouterErrorException When there's an error with the temporary
-     *     script used to get the file.
+     *     script used to get the file, or if the file doesn't exist.
      */
     public function fileGetContents($filename, $tmpScriptName = null)
     {
-        $checkRequest = new Request(
-            '/file/print .proplist=""',
-            Query::where('name', $filename)
-        );
-        if (1 === count($this->client->sendSync($checkRequest))) {
-            return false;
+        try {
+            $responses = $this->exec(
+                ':error ("&" . [/file get $filename contents]);',
+                array('filename' => $filename),
+                null,
+                $tmpScriptName
+            );
+            throw new RouterErrorException(
+                'Unable to read file through script (no error returned)',
+                RouterErrorException::CODE_SCRIPT_FILE_ERROR,
+                null,
+                $responses
+            );
+        } catch (RouterErrorException $e) {
+            if ($e->getCode() !== RouterErrorException::CODE_SCRIPT_RUN_ERROR) {
+                throw $e;
+            }
+            $message = $e->getResponses()->getAllOfType(Response::TYPE_ERROR)
+                ->getProperty('message');
+            if (Stream::isStream($message)) {
+                $successToken = fread($message, 1/*strlen('&')*/);
+                if ('&' === $successToken) {
+                    return $message;
+                }
+                rewind($message);
+            } elseif (strpos($message, '&') === 0) {
+                return substr($message, 1/*strlen('&')*/);
+            }
+            throw $e;
         }
-        return $this->exec(
-            '/system script set $"_" source=[/file get $filename contents]',
-            array('filename' => $filename),
-            null,
-            $tmpScriptName
-        );
     }
 
     /**

tests/Client/Safe.php

@@ -825,7 +825,7 @@ public function testSendSyncWithQueryEquals()
         $this->assertEquals(
             2,
             count($list),
-            'The list should have only one item and a "done" reply.'
+            'The list should have only one item and a "done" reply. target=' . HOSTNAME_INVALID . ';list=' . print_r($list->toArray(), true)
         );
 
         $request->setQuery(