Level-2 / Transphporm

Transformation Style Sheets - Revolutionising PHP templating
276 stars 27 forks source link

Odd issue results in Undefined index: OPEN_BRACE notice and fatal error #143

Closed Mr-Chimp closed 7 years ago

Mr-Chimp commented 7 years ago

Hi, so I have found something weird when using Transphporm. Below is the tss and data I am using: array(1) { [0]=> object(Objects\ShipOptions_obj)#22 (4) { ["id"]=> object(MongoDB\BSON\ObjectID)#24 (1) { ["oid"]=> string(24) "58c5aedc615d403cbf019f4e" } ["type"]=> string(5) "manga" ["name"]=> string(3) "sdf" ["max_qty"]=> string(1) "3" } }

tbody tr {repeat: data(options);}
thead th:nth-child(1) {content: "Name"; } 
thead th:nth-child(2) {content: "Type"; } 
thead th:nth-child(3) {content: "Max Qty"; } 
thead th:nth-child(4) {content: ""; } 
tbody td:nth-child(3) {content: iteration(max_qty); } 
tbody td:nth-child(1) {content: iteration(name);        
tbody td:nth-child(2) {content: iteration(type); }
tbody td:nth-child(4) {content: ""; } 

tbody td a.edit:attr(href) {content: "/admin/ship_options/edit/",iteration(id); }       
tbody td button.delete:attr(data-id) {content: iteration(id); }

tbody td button.delete:attr(data-delurl) {content: "/admin/ship_options/delete/"; }   
#checkDelete .delMsg { content: "This will permanently delete the Option, are you sure?"; }
#checkDelete .delete.btn { content: "Delete Option"; }

Now with the above I get the following error: Notice: Undefined index: OPEN_BRACE in /var/www/html/cms/lib/transphporm/Parser/Value.php on line 60 Fatal error: Uncaught Error: Method name must be a string in /var/www/html/cms/lib/transphporm/Parser/Value.php:60

What makes this really odd is through elimination I have discovered the data is the cause of the problem. But if I remove the last to lines of code (the #CheckDelete bits) then it works without issue. OR if I leave those last two lines in there and remove the two lines on there own (the lines that use iteration(id) ) then it also works. Without any data it works as well. I believe the issue is to do with the id parameter but am not sure why and I might be grasping at straws there.

What is really odd is that I have used the same idea on about 6 other admin pages (working in a custom cms) and this is the only time I have ran into this issue and the others work beautifully, as I said they use the same technique just different data (though all of the data for the other pages still have the MongoDB BSON Object ID parameter). I grabbed the latest version of Transphporm just now trying to debug this but it didn't make any difference.

Any help greatly appreciated.

solleer commented 7 years ago

Can you give the data in json format and some example xml so I can reproduce this in a test?

TRPB commented 7 years ago

Yes please provide some sample XML and ideally some data in JSON format so we can replicate the issue.

What confuses me is that if it's an issue with the data, I'm not sure how the last two lines could be affected, as they don't use the data() function.

Mr-Chimp commented 7 years ago

Here is all of the data in JSON format, the bit above was only most of the data:

{"text":"

This page lists all of the shipping options in the system Add Options<\/a><\/p>","title":"Shipping Options","options":[{"id":{"$oid":"58c5aedc615d403cbf019f4e"},"type":"manga","name":"sdf","max_qty":"3"}]}

I am not sure why there is the big bit of whitespace in the text field but that is what the output of json_encode was so thought I would paste as is in case that was the cause.

Below is the xml that this specifically applies to:

    <div class="col-sm-9 col-sm-offset-3 col-md-10 col-md-offset-2 main">
      <h1 class="page-header">Home</h1>
      <div class="content">
        <p class="main">Nothing much to see here yet</p>
        </div>

        <table class="table table-striped ">
            <thead>
                <tr>
                    <th>First Name</th>
                    <th>Last Name</th>
                    <th>E-mail Address</th>
                    <th>Level</th>
                    <th></th>
                </tr>
                <tbody>
                     <tr>
                        <td>First Name</td>
                        <td>Last Name</td>
                        <td>E-mail Address</th>
                        <td>Level</td>
                        <td><a class="edit btn btn-primary" href="">Edit</a> | <button class="delete btn btn-danger" data-toggle="modal" data-target="#checkDelete" data-id="" data-delurl="">Delete</button></td>
                    </tr>
                </tbody>
            </thead>
        </table>
    </div>

Also for good measure the full tss, where I have built Transphporm into my framework some bits of tss were missing from above:

h1 { content: data(title) }
.content { content: data(text); format: html; }
tbody tr {repeat: data(options);}
thead th:nth-child(1) {content: "Name"; } 
thead th:nth-child(2) {content: "Type"; } 
thead th:nth-child(3) {content: "Max Qty"; } 
thead th:nth-child(4) {content: ""; } 

tbody td:nth-child(3) {content: iteration(max_qty); } 
tbody td:nth-child(1) {content: iteration(name);        
tbody td:nth-child(2) {content: iteration(type); }
     tbody td:nth-child(4) {content: ""; } 

     tbody td a.edit:attr(href) {content: "/admin/ship_options/edit/",iteration(id); }       
     tbody td button.delete:attr(data-id) {content: iteration(id); }
     tbody td button.delete:attr(data-delurl) {content: "/admin/ship_options/delete/"; }

     #checkDelete .delMsg { content: "This will permanently delete the Option, are you sure?"; }
     #checkDelete .delete.btn { content: "Delete Option"; }

Thanks very much

TRPB commented 7 years ago

For reference: PHP code that triggers the issue:

$data = json_decode('{"text":"This page lists all of the shipping options in the system Add Options</a></p>","title":"Shipping Options","options":[{"id":{"$oid":"58c5aedc615d403cbf019f4e"},"type":"manga","name":"sdf","max_qty":"3"}]}');

$xml =    '<div class="col-sm-9 col-sm-offset-3 col-md-10 col-md-offset-2 main">
      <h1 class="page-header">Home</h1>
      <div class="content">
        <p class="main">Nothing much to see here yet</p>
        </div>

        <table class="table table-striped ">
            <thead>
                <tr>
                    <th>First Name</th>
                    <th>Last Name</th>
                    <th>E-mail Address</th>
                    <th>Level</th>
                    <th></th>
                </tr>
                <tbody>
                     <tr>
                        <td>First Name</td>
                        <td>Last Name</td>
                        <td>E-mail Address</th>
                        <td>Level</td>
                        <td><a class="edit btn btn-primary" href="">Edit</a> | <button class="delete btn btn-danger" data-toggle="modal" data-target="#checkDelete" data-id="" data-delurl="">Delete</button></td>
                    </tr>
                </tbody>
            </thead>
        </table>
    </div>';

  $tss = 'h1 { content: data(title) }
.content { content: data(text); format: html; }
tbody tr {repeat: data(options);}
thead th:nth-child(1) {content: "Name"; } 
thead th:nth-child(2) {content: "Type"; } 
thead th:nth-child(3) {content: "Max Qty"; } 
thead th:nth-child(4) {content: ""; } 

tbody td:nth-child(3) {content: iteration(max_qty); } 
tbody td:nth-child(1) {content: iteration(name);        
tbody td:nth-child(2) {content: iteration(type); }
     tbody td:nth-child(4) {content: ""; } 

     tbody td a.edit:attr(href) {content: "/admin/ship_options/edit/",iteration(id); }       
     tbody td button.delete:attr(data-id) {content: iteration(id); }
     tbody td button.delete:attr(data-delurl) {content: "/admin/ship_options/delete/"; }

     #checkDelete .delMsg { content: "This will permanently delete the Option, are you sure?"; }
     #checkDelete .delete.btn { content: "Delete Option"; }';

    $template = new \Transphporm\Builder($xml, $tss);

    var_dump($template->output($data));
TRPB commented 7 years ago

The following line:

     tbody td a.edit:attr(href) {content: "/admin/ship_options/edit/",iteration(id); }  

causes the errors:

Transphporm\Exception: TSS Error: Problem carrying out property 'content' on Line 14 of tss

Object of class stdClass could not be converted to string Content.php:84
TRPB commented 7 years ago

Bizarrely it seems to be the trailing slash in the string

     tbody td a.edit:attr(href) {content: "/admin/ship_options"; }  

works but

     tbody td a.edit:attr(href) {content: "/admin/ship_options/"; }  

causes an error

TRPB commented 7 years ago

aha I found the problem. It's really simple but does show that we need a better method of validating TSS files.

Take a look at the lines:

tbody td:nth-child(3) {content: iteration(max_qty); } 
tbody td:nth-child(1) {content: iteration(name);        
tbody td:nth-child(2) {content: iteration(type); }
     tbody td:nth-child(4) {content: ""; } 

The name line is missing the closing brace.

Mr-Chimp commented 7 years ago

Thank you for spotting that, can't believe it was a simple issue like that, that is what I get for coding at night after a day of even more coding.

Thanks again for your time and apologies it was just a missing brace.

As a point of interest is there anything out there for validating TSS at all? If I get a moment I might put something simple together to catch simple issues as I doubt this will be the last time I run into something like this.

TRPB commented 7 years ago

A CSS validator should catch missing braces, but a proper validator would be useful, checking for valid property names and number of arguments. If you do try to throw something together you could use the existing tokenizer from the project

solleer commented 7 years ago

I started working on this and one issue is that you need a fully configured Config object to test what properties exist and to avoid duplicate code I was thinking of creating a ConfigBuilder class. What do you think?

TRPB commented 7 years ago

We're probably better off differentiating between parse errors and runtime errors. Runtime errors can be exceptions as they are now, while the validator here could just check the document parses correctly.

e.g. does every opening brace have a closing brace.

solleer commented 7 years ago

What other errors should it check for?

TRPB commented 7 years ago

Off the top of my head:

  1. Scan the tokens and check that every opening brace has a corresponding closing brace. This should be simple, just iterate over the tokens after reaching OPEN_BRACE if you find another OPEN_BRACE before CLOSING_BRACE then error.

  2. Do the same thing with SEMICOLON and COLON, after you find a COLON then ensure there is a SEMICOLON (or OPEN_BRACE) before the next COLON

  3. Break apart the individual rules so you'll see content: data(attr(foo) and count opening/closing brackets. No real need to match them up, just check there are an equal number of opening and closing brackets (and square brackets).

Getting line number might be tricky, it would probably be worth having the tokenizer store the line number of each token but I'm not sure how easy that will be.

TRPB commented 7 years ago

Actually, getting the tokenizer to give each token a line number is really easy:

diff --git a/src/Parser/Tokenizer.php b/src/Parser/Tokenizer.php
index 59cfe28..a84d5db 100644
--- a/src/Parser/Tokenizer.php
+++ b/src/Parser/Tokenizer.php
@@ -33,6 +33,8 @@ class Tokenizer {
    const MULTIPLY = 'MULTIPLY';
    const DIVIDE = 'DIVIDE';

+   private $lineNo = 1;
+
    private $chars = [
        '"' => self::STRING,
        '\'' => self::STRING,
@@ -71,6 +73,7 @@ class Tokenizer {
        for ($i = 0; $i < strlen($this->str); $i++) {
            $char = $this->identifyChar($this->str[$i]);

+           $this->doNewLine($tokens, $char);
            $this->doSimpleTokens($tokens, $char);
            $this->doLiterals($tokens, $char, $i);
            $i += $this->doStrings($tokens, $char, $i);
@@ -82,12 +85,19 @@ class Tokenizer {

    private function doSimpleTokens(&$tokens, $char) {
        if (in_array($char, [Tokenizer::ARG, Tokenizer::CONCAT, Tokenizer::DOT, Tokenizer::NOT, Tokenizer::EQUALS,
-           Tokenizer::COLON, Tokenizer::SEMI_COLON, Tokenizer::WHITESPACE, Tokenizer::NEW_LINE, Tokenizer::NUM_SIGN,
+           Tokenizer::COLON, Tokenizer::SEMI_COLON, Tokenizer::WHITESPACE, Tokenizer::NUM_SIGN,
            Tokenizer::GREATER_THAN, Tokenizer::AT_SIGN, Tokenizer::SUBTRACT, Tokenizer::MULTIPLY, Tokenizer::DIVIDE])) {
-           $tokens[] = ['type' => $char];
+           $tokens[] = ['type' => $char, 'line' => $this->lineNo];
        }
    }

+   private function doNewLine(&$tokens, $char) {
+       if ($char == Tokenizer::NEW_LINE) {
+           $this->lineNo++;
+           $tokens[] = ['type' => $char, 'line' => $this->lineNo];
+       } 
+   }
+
    private function isLiteral($n) {
        //Is it a normal literal character
        return isset($this->str[$n]) && ($this->identifyChar($this->str[$n]) == self::NAME
@@ -110,7 +120,7 @@ class Tokenizer {
        if (is_numeric($name)) $tokens[] = ['type' => self::NUMERIC, 'value' => $name];
        else if ($name == 'true') $tokens[] = ['type' => self::BOOL, 'value' => true];
        else if ($name == 'false') $tokens[] = ['type' => self::BOOL, 'value' => false];
-       else $tokens[] = ['type' => self::NAME, 'value' => $name];
+       else $tokens[] = ['type' => self::NAME, 'value' => $name, 'line' => $this->lineNo];
    }

    private function doBrackets(&$tokens, $char, $i) {
@@ -124,7 +134,7 @@ class Tokenizer {
            if ($char === $type) {
                $contents = $this->extractBrackets($i, $brackets[0], $brackets[1]);
                $tokenizer = new Tokenizer($contents);
-               $tokens[] = ['type' => $type, 'value' => $tokenizer->getTokens(), 'string' => $contents];
+               $tokens[] = ['type' => $type, 'value' => $tokenizer->getTokens(), 'string' => $contents, 'line' => $this->lineNo];
                return strlen($contents);
            }
        }
@@ -136,7 +146,7 @@ class Tokenizer {
            $length = strlen($string)+1;
            $char = $this->getChar($char);
            $string = str_replace('\\' . $char, $char, $string);
-           $tokens[] = ['type' => self::STRING, 'value' => $string];
+           $tokens[] = ['type' => self::STRING, 'value' => $string, 'line' => $this->lineNo];
            return $length;
        }
    }
<?php
/* @description     Transformation Style Sheets - Revolutionising PHP templating    *
 * @author          Tom Butler tom@r.je                                             *
 * @copyright       2015 Tom Butler <tom@r.je> | https://r.je/                      *
 * @license         http://www.opensource.org/licenses/bsd-license.php  BSD License *
 * @version         1.0                                                             */
namespace Transphporm\Parser;
class Tokenizer {
    private $str;
    const NAME = 'LITERAL';
    const STRING = 'STRING';
    const OPEN_BRACKET = 'OPEN_BRACKET';
    const CLOSE_BRACKET = 'CLOSE_BRACKET';
    const OPEN_SQUARE_BRACKET = 'SQUARE_BRACKET';
    const CLOSE_SQUARE_BRACKET = 'CLOSE_SQUARE_BRACKET';
    const CONCAT = 'CONCAT';
    const ARG = 'ARG';
    const WHITESPACE = 'WHITESPACE';
    const NEW_LINE = 'NEW_LINE';
    const DOT = 'DOT';
    const NUMERIC = 'NUMERIC';
    const EQUALS = 'EQUALS';
    const NOT = 'NOT';
    const OPEN_BRACE = 'OPEN_BRACE';
    const CLOSE_BRACE = 'CLOSE_BRACE';
    const BOOL = 'BOOL';
    const COLON = 'COLON';
    const SEMI_COLON = 'SEMI_COLON';
    const NUM_SIGN = 'NUM_SIGN';
    const GREATER_THAN = 'GREATER_THAN';
    const AT_SIGN = 'AT_SIGN';
    const SUBTRACT = 'SUBTRACT';
    const MULTIPLY = 'MULTIPLY';
    const DIVIDE = 'DIVIDE';

    private $lineNo = 1;

    private $chars = [
        '"' => self::STRING,
        '\'' => self::STRING,
        '(' => self::OPEN_BRACKET,
        ')' => self::CLOSE_BRACKET,
        '[' => self::OPEN_SQUARE_BRACKET,
        ']' => self::CLOSE_SQUARE_BRACKET,
        '+' => self::CONCAT,
        ',' => self::ARG,
        '.' => self::DOT,
        '!' => self::NOT,
        '=' => self::EQUALS,
        '{' => self::OPEN_BRACE,
        '}' => self::CLOSE_BRACE,
        ':' => self::COLON,
        ';' => self::SEMI_COLON,
        '#' => self::NUM_SIGN,
        '>' => self::GREATER_THAN,
        '@' => self::AT_SIGN,
        '-' => self::SUBTRACT,
        '*' => self::MULTIPLY,
        '/' => self::DIVIDE,
        ' ' => self::WHITESPACE,
        "\n" => self::NEW_LINE,
        "\r" => self::WHITESPACE,
        "\t" => self::WHITESPACE
    ];

    public function __construct($str) {
        $this->str = $str;
    }

    public function getTokens($returnObj = true) {
        $tokens = [];

        for ($i = 0; $i < strlen($this->str); $i++) {
            $char = $this->identifyChar($this->str[$i]);

            $this->doNewLine($tokens, $char);
            $this->doSimpleTokens($tokens, $char);
            $this->doLiterals($tokens, $char, $i);
            $i += $this->doStrings($tokens, $char, $i);
            $i += $this->doBrackets($tokens, $char, $i);
        }
        if ($returnObj) return new Tokens($tokens);
        else return $tokens;
    }

    private function doSimpleTokens(&$tokens, $char) {
        if (in_array($char, [Tokenizer::ARG, Tokenizer::CONCAT, Tokenizer::DOT, Tokenizer::NOT, Tokenizer::EQUALS,
            Tokenizer::COLON, Tokenizer::SEMI_COLON, Tokenizer::WHITESPACE, Tokenizer::NUM_SIGN,
            Tokenizer::GREATER_THAN, Tokenizer::AT_SIGN, Tokenizer::SUBTRACT, Tokenizer::MULTIPLY, Tokenizer::DIVIDE])) {
            $tokens[] = ['type' => $char, 'line' => $this->lineNo];
        }
    }

    private function doNewLine(&$tokens, $char) {
        if ($char == Tokenizer::NEW_LINE) {
            $this->lineNo++;
            $tokens[] = ['type' => $char, 'line' => $this->lineNo];
        } 
    }

    private function isLiteral($n) {
        //Is it a normal literal character
        return isset($this->str[$n]) && ($this->identifyChar($this->str[$n]) == self::NAME
        //but a subtract can be part of a class name or a mathematical operation
                || ($this->identifyChar($this->str[$n]) == self::SUBTRACT && !is_numeric($this->str[$n-1])));
    }

    private function doLiterals(&$tokens, $char, &$i) {
        if ($char === self::NAME) {
            $name = $this->str[$i];
            while ($this->isLiteral($i+1)) {
                $name .= $this->str[$i+1];
                $i++;
            }
            $this->processLiterals($tokens, $name);
        }
    }

    private function processLiterals(&$tokens, $name) {
        if (is_numeric($name)) $tokens[] = ['type' => self::NUMERIC, 'value' => $name];
        else if ($name == 'true') $tokens[] = ['type' => self::BOOL, 'value' => true];
        else if ($name == 'false') $tokens[] = ['type' => self::BOOL, 'value' => false];
        else $tokens[] = ['type' => self::NAME, 'value' => $name, 'line' => $this->lineNo];
    }

    private function doBrackets(&$tokens, $char, $i) {
        $types = [
            self::OPEN_BRACKET => ['(', ')'],
            self::OPEN_BRACE => ['{', '}'],
            self::OPEN_SQUARE_BRACKET => ['[', ']']
        ];

        foreach ($types as $type => $brackets) {
            if ($char === $type) {
                $contents = $this->extractBrackets($i, $brackets[0], $brackets[1]);
                $tokenizer = new Tokenizer($contents);
                $tokens[] = ['type' => $type, 'value' => $tokenizer->getTokens(), 'string' => $contents, 'line' => $this->lineNo];
                return strlen($contents);
            }
        }
    }

    private function doStrings(&$tokens, $char, $i) {
        if ($char === self::STRING) {
            $string = $this->extractString($i);
            $length = strlen($string)+1;
            $char = $this->getChar($char);
            $string = str_replace('\\' . $char, $char, $string);
            $tokens[] = ['type' => self::STRING, 'value' => $string, 'line' => $this->lineNo];
            return $length;
        }
    }

    private function extractString($pos) {
        $char = $this->str[$pos];
        $end = strpos($this->str, $char, $pos+1);
        while ($end !== false && $this->str[$end-1] == '\\') $end = strpos($this->str, $char, $end+1);

        return substr($this->str, $pos+1, $end-$pos-1);
    }

    private function extractBrackets($open, $startBracket = '(', $closeBracket = ')') {
        $close = strpos($this->str, $closeBracket, $open);

        $cPos = $open+1;
        while (($cPos = strpos($this->str, $startBracket, $cPos+1)) !== false && $cPos < $close) $close = strpos($this->str, $closeBracket, $close+1);
        return substr($this->str, $open+1, $close-$open-1);
    }

    private function identifyChar($chr) {
        if (isset($this->chars[$chr])) return $this->chars[$chr];
        else return self::NAME;
    }

    private function getChar($num) {
        $chars = array_reverse($this->chars);
        if (isset($chars[$num])) return $chars[$num];
        else return false;
    }
}
solleer commented 7 years ago

The only issue with that is that it causes the override for before and after to not work

TRPB commented 7 years ago

Which override? How does adding an array key to $tokens break :before?

solleer commented 7 years ago

It causes the serialize to be different because the line number is different on all the tokens being serialized.

solleer commented 7 years ago

@TomBZombie I have added a TSS Validator. There is some copied code from the Sheet Parser though so it probably needs to be refactored.