Skip to content

Commit

Permalink
Reverted the script source getting addition, and in fact completely r…
Browse files Browse the repository at this point in the history 
…emoved 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.
  • Loading branch information
@boenrobot
boenrobot committed Aug 10, 2016
1 parent a7dd5ea commit 9cdb61e
Show file tree
Hide file tree
Showing 11 changed files with 410 additions and 94 deletions.
3 changes: 2 additions & 1 deletion RELEASE-1.0.0b6
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/PEAR2/Net/RouterOS/RouterErrorException.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
115 changes: 59 additions & 56 deletions src/PEAR2/Net/RouterOS/Util.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) .
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
)
);
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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];
Expand All @@ -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');
Expand All @@ -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;
Expand Down Expand Up @@ -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
);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/Client/Safe.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading

0 comments on commit 9cdb61e

Please sign in to comment.