Skip to content

Commit 552361e

Browse files
committed
Made Script::escapeString() not encode bytes above 0x7F, in turn allowing for script's parameter string values to be influenced by charset settings;
Added a $full parameter to Script::escapeString() that encodes all bytes (binary mode, basically); Made Script::append() escape stream values in full, meaning that script stream values are NOT influenced by charset settings; Util::fileGetContents() properly strips out the success token from the result when in streaming mode (by copying the stream to a new stream); Util::count() now has a second argument passed as the request's "from" argument; Streamlined Util::exec() result collection creation; Doc and CS fixes.
1 parent 8887b51 commit 552361e

File tree

5 files changed

+218
-87
lines changed

5 files changed

+218
-87
lines changed

RELEASE-1.0.0b6

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Util stuff, mostly.
22

33
* __BREAKING CHANGES:__
4-
- 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.
4+
- 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 one, and a new $from argument as a second one.
55
- 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.
66
- 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.
77
- 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.
@@ -11,6 +11,8 @@ Util stuff, mostly.
1111
- comment()
1212
- getCurrentTime()
1313
- newRequest()
14+
* Script::escapeString() no longer escapes bytes above 0x7F. This means that parameter string values are now influenced by charset settings, and thus no additional conversion is necessary. A new second argument can be set to TRUE to escape all bytes, including the previously untouched ASCII alphanumeric characters and underscores.
15+
* Stream parameters in Script::append() (and inherently, Script::prepare() and Util::exec()) now have ALL bytes escaped.
1416
* 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).
1517
* 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).
1618
* 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).

src/PEAR2/Net/RouterOS/Script.php

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class Script
8080
*
8181
* @return mixed Depending on RouterOS type detected:
8282
* - "nil" (the string "[]") or "nothing" (empty string) - NULL.
83-
* - "number" - int or double for large values.
83+
* - "num" - int or double for large values.
8484
* - "bool" - a boolean.
8585
* - "array" - an array, with the keys and values processed recursively.
8686
* - "time" - a {@link DateInterval} object.
@@ -147,7 +147,7 @@ public static function parseValueToString($value)
147147
*
148148
* @return string|bool|int|double|null Depending on RouterOS type detected:
149149
* - "nil" (the string "[]") or "nothing" (empty string) - NULL.
150-
* - "number" - int or double for large values.
150+
* - "num" - int or double for large values.
151151
* - "bool" - a boolean.
152152
* - Unrecognized type - casted to a string, unmodified.
153153
*/
@@ -192,6 +192,7 @@ public static function parseValueToDateTime(
192192
$value,
193193
DateTimeZone $timezone = null
194194
) {
195+
$previous = null;
195196
$value = (string)$value;
196197
if ('' !== $value && preg_match(
197198
'#^
@@ -221,12 +222,13 @@ public static function parseValueToDateTime(
221222
$timezone
222223
);
223224
} catch (E $e) {
224-
return $value;
225+
$previous = $e;
225226
}
226227
}
227228
throw new ParserException(
228229
'The supplied value can not be converted to a DateTime',
229-
ParserException::CODE_DATETIME
230+
ParserException::CODE_DATETIME,
231+
$previous
230232
);
231233
}
232234

@@ -406,7 +408,7 @@ public static function parseValueToArray(
406408
* variables to it.
407409
*
408410
* This is particularly useful when you're creating scripts that you don't
409-
* want to execute right now (as with {@link static::exec()}, but instead
411+
* want to execute right now (as with {@link Util::exec()}, but instead
410412
* you want to store it for later execution, perhaps by supplying it to
411413
* "/system scheduler".
412414
*
@@ -420,9 +422,9 @@ public static function parseValueToArray(
420422
* Array values are automatically processed with
421423
* {@link static::escapeValue()}. Streams are also supported, and are
422424
* processed in chunks, each with
423-
* {@link static::escapeString()}. Processing starts from the current
424-
* position to the end of the stream, and the stream's pointer stays at
425-
* the end after reading is done.
425+
* {@link static::escapeString()} with all bytes being escaped.
426+
* Processing starts from the current position to the end of the stream,
427+
* and the stream's pointer is left untouched after the reading is done.
426428
*
427429
* @return resource A new PHP temporary stream with the script as contents,
428430
* with the pointer back at the start.
@@ -456,9 +458,9 @@ public static function prepare(
456458
* Array values are automatically processed with
457459
* {@link static::escapeValue()}. Streams are also supported, and are
458460
* processed in chunks, each with
459-
* {@link static::escapeString()}. Processing starts from the current
460-
* position to the end of the stream, and the stream's pointer stays at
461-
* the end after reading is done.
461+
* {@link static::escapeString()} with all bytes being escaped.
462+
* Processing starts from the current position to the end of the stream,
463+
* and the stream's pointer is left untouched after the reading is done.
462464
*
463465
* @return int The number of bytes written to $stream is returned,
464466
* and the pointer remains where it was after the write
@@ -481,7 +483,7 @@ public static function append(
481483
$bytes += $writer->send('"');
482484
while ($reader->isAvailable() && $reader->isDataAwaiting()) {
483485
$bytes += $writer->send(
484-
static::escapeString(fread($pvalue, $chunkSize))
486+
static::escapeString(fread($pvalue, $chunkSize), true)
485487
);
486488
}
487489
$bytes += $writer->send("\";\n");
@@ -518,7 +520,7 @@ public static function escapeValue($value)
518520
{
519521
switch(gettype($value)) {
520522
case 'NULL':
521-
$value = '';
523+
$value = '[]';
522524
break;
523525
case 'integer':
524526
$value = (string)$value;
@@ -573,22 +575,28 @@ public static function escapeValue($value)
573575
* larger string first), and you can be sure there won't be any code
574576
* injections coming from it.
575577
*
576-
* For the sake of brevity of the output, alphanumeric characters and
577-
* underscores are left untouched
578+
* By default, for the sake of brevity of the output, ASCII alphanumeric
579+
* characters and underscores are left untouched. And for the sake of
580+
* character conversion, bytes above 0x7F are also left untouched.
578581
*
579582
* @param string $value Value to be escaped.
583+
* @param bool $full Whether to escape all bytes in the string, including
584+
* ASCII alphanumeric characters, underscores and bytes above 0x7F.
580585
*
581586
* @return string The escaped value.
582587
*
583-
* @internal Why leave ONLY those characters and not also others?
588+
* @internal Why leave ONLY those ASCII characters and not also others?
584589
* Because those can't in any way be mistaken for language constructs,
585590
* unlike many other "safe inside strings, but not outside" ASCII
586591
* characters, like ",", ".", "+", "-", "~", etc.
587592
*/
588-
public static function escapeString($value)
593+
public static function escapeString($value, $full = false)
589594
{
595+
if ($full) {
596+
return self::_escapeCharacters(array($value));
597+
}
590598
return preg_replace_callback(
591-
'/[^\\_A-Za-z0-9]+/S',
599+
'/[^\\_A-Za-z0-9\\x80-\\xFF]+/S',
592600
array(__CLASS__, '_escapeCharacters'),
593601
$value
594602
);
@@ -597,8 +605,9 @@ public static function escapeString($value)
597605
/**
598606
* Escapes a character for a RouterOS scripting context.
599607
*
600-
* Escapes a character for a RouterOS scripting context. Intended to only be
601-
* called for non-alphanumeric characters.
608+
* Escapes a character for a RouterOS scripting context.
609+
* Intended to only be called by {@link self::escapeString()} for the
610+
* matching strings.
602611
*
603612
* @param array $chars The matches array, expected to contain exactly one
604613
* member, in which is the whole string to be escaped.

src/PEAR2/Net/RouterOS/Util.php

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,9 @@ public function newRequest(
226226
* Array values are automatically processed with
227227
* {@link static::escapeValue()}. Streams are also supported, and are
228228
* processed in chunks, each processed with
229-
* {@link static::escapeString()}. Processing starts from the current
230-
* position to the end of the stream, and the stream's pointer is left
231-
* untouched after the reading is done.
229+
* {@link static::escapeString()} with all bytes being escaped.
230+
* Processing starts from the current position to the end of the stream,
231+
* and the stream's pointer is left untouched after the reading is done.
232232
* Note that the script's (generated) name is always added as the
233233
* variable "_", which will be inadvertently lost if you overwrite it
234234
* from here.
@@ -296,42 +296,32 @@ public function exec(
296296
$request->setArgument('numbers', $name);
297297
$removeResult = $this->client->sendSync($request);
298298

299+
$results = new ResponseCollection(
300+
array_merge(
301+
$addResult->toArray(),
302+
$runResult->toArray(),
303+
$removeResult->toArray()
304+
)
305+
);
306+
299307
if (count($runResult->getAllOfType(Response::TYPE_ERROR)) > 0) {
300308
throw new RouterErrorException(
301309
'Error when running script',
302310
RouterErrorException::CODE_SCRIPT_RUN_ERROR,
303311
null,
304-
new ResponseCollection(
305-
array_merge(
306-
$addResult->toArray(),
307-
$runResult->toArray(),
308-
$removeResult->toArray()
309-
)
310-
)
312+
$results
311313
);
312314
}
313315
if (count($removeResult->getAllOfType(Response::TYPE_ERROR)) > 0) {
314316
throw new RouterErrorException(
315317
'Error when removing script',
316318
RouterErrorException::CODE_SCRIPT_REMOVE_ERROR,
317319
null,
318-
new ResponseCollection(
319-
array_merge(
320-
$addResult->toArray(),
321-
$runResult->toArray(),
322-
$removeResult->toArray()
323-
)
324-
)
320+
$results
325321
);
326322
}
327323

328-
return new ResponseCollection(
329-
array_merge(
330-
$addResult->toArray(),
331-
$runResult->toArray(),
332-
$removeResult->toArray()
333-
)
334-
);
324+
return $results;
335325
}
336326

337327
/**
@@ -928,17 +918,27 @@ public function move($numbers, $destination = null)
928918
* queries are allowed as a criteria, in contrast with
929919
* {@link static::find()}, where numbers and callbacks are allowed also.
930920
*
931-
* @param Query|null $query A query to filter items by. Without it, all items
932-
* are included in the count.
921+
* @param Query|null $query A query to filter items by.
922+
* Without it, all items are included in the count.
923+
* @param string|resource|null $from A comma separated list of item IDs.
924+
* Any items in the set that still exist at the time of couting
925+
* are included in the final tally. Note that the $query filters this
926+
* set further (i.e. the item must be in the list AND match the $query).
927+
* Leaving the value to NULL means all matching items at the current
928+
* menu are included in the count.
933929
*
934930
* @return int The number of items, or -1 on failure (e.g. if the
935931
* current menu does not have a "print" command or items to be counted).
936932
*/
937-
public function count(Query $query = null)
933+
public function count(Query $query = null, $from = null)
938934
{
939-
$result = $this->client->sendSync(
940-
new Request($this->menu . '/print count-only=""', $query)
941-
)->end()->getProperty('ret');
935+
$countRequest = new Request(
936+
$this->menu . '/print count-only=""',
937+
$query
938+
);
939+
$countRequest->setArgument('from', $from);
940+
$result = $this->client->sendSync($countRequest)->end()
941+
->getProperty('ret');
942942

943943
if (null === $result) {
944944
return -1;
@@ -1147,7 +1147,11 @@ public function fileGetContents($filename, $tmpScriptName = null)
11471147
if (Stream::isStream($message)) {
11481148
$successToken = fread($message, 1/*strlen('&')*/);
11491149
if ('&' === $successToken) {
1150-
return $message;
1150+
$messageCopy = fopen('php://temp', 'r+b');
1151+
stream_copy_to_stream($message, $messageCopy);
1152+
rewind($messageCopy);
1153+
fclose($message);
1154+
return $messageCopy;
11511155
}
11521156
rewind($message);
11531157
} elseif (strpos($message, '&') === 0) {

0 commit comments

Comments
 (0)