1

I have the follow code:

onclick=" <?php echo 'postwith(\''.$_SERVER['PHP_SELF'].'\',{export:\'export\',date:\''.$_POST['date'].'\'})'; ?>"

while postwith is a function.

in ie i have an error: Expected identifier, string or number

in firefox it's ok and the link is:

postwith('/page/page.php',{export:'export',date:'Yesterday'})

so where is my mistake?

thank you!

Peter Ajtai
  • 56,972
  • 13
  • 121
  • 140
Ronny
  • 285
  • 3
  • 4
  • 10
  • 4
    Please show the finished, rendered HTML code. The PHP source code is meaningless in this context – Pekka Sep 14 '10 at 15:37

5 Answers5

4

export is a keyword, so it appears that the IE Javascript engine is getting confused with you using it in that context. You could put it in quotes to make it clear that it's a key.

warrenm
  • 31,094
  • 6
  • 92
  • 116
2

+1 warrenm, it's export that needs to be quoted.

But this sort of thing isn't good form. With all that nested quoting it's barely readable, and because you've not JavaScript-string-literal-escaped or HTML-escaped either date or PHP_SELF, you've got HTML-injection bugs which may lead to cross-site-scripting security holes.

Never output a text string to HTML text content or attribute values without htmlspecialchars(), and when you're building JS objects use json_encode() to create the output because it will cope with string escaping problems and quoting object literal names for you.

From PHP 5.3, the JSON_HEX options allow you to ensure all HTML-special characters are encoded as JavaScript string literal escapes, so you don't have to HTML-encode on top of JSON-encoding, which means you can use the same output function in both event handler attributes and <script> blocks (which, being CDATA, have no HTML-escaping).

<?php
    function j($o) {
        echo json_encode($o, JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_QUOT);
    };
    $pars= array("export"=>"export", "date"=>$_POST['date']);
?>

onclick="postwith(<?php j($_SERVER['PHP_SELF']); ?>, <?php j($pars); ?>);"

Also consider breaking out the onclick handler and assigning it from <script> instead of using inline event handler attributes. This tends to be more readable.

bobince
  • 528,062
  • 107
  • 651
  • 834
1

As warrenm pointed out export is a keyword and needs to be quoted.

That is, alter the PHP so the result output is:

postwith('/page/page.php',{'export':'export','date':'Yesterday'});

Your PHP would look like this:

onclick="<?php echo "postwith('{$_SERVER['PHP_SELF']}',
     {'export':'export','date':'{$_POST['date']}'})"; ?>"

(Thanks, Peter for the improved syntax).

Also, you may wish to remove the space after onclick:

onclick=" <?php 

will become:

onclick="<?php 
Sean Vieira
  • 155,703
  • 32
  • 311
  • 293
1

For future reference, you might find it easier to proof read if you use double quotes for your PHP string and curly bracket notation for array elements inside the string:

onclick="<?php echo "postwith('{$_SERVER['PHP_SELF']}',
         {'export':'export','date':'{$_POST['date']}'})"; ?>"

simplified example of using curly bracket notation inside double quotes
(note that you do not need to escape literally rendered curly brackets)

Additionally, you should make use of json_encode() to make sure your JSON is in the right format:
(note the single quotes after onclick to accommodate the double quote JSON)

onclick='<?php
    echo "postwith(\"{$_SERVER['PHP_SELF']}\"," .
    json_encode(array("export" => "export", "date" => $_POST['date']),
                JSON_FORCE_OBJECT|JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_QUOT) .
    ")";
?>'

example

See bobince, post about the JSON encoding options.

Community
  • 1
  • 1
Peter Ajtai
  • 56,972
  • 13
  • 121
  • 140
1

This is sloppy coding, IMO. Keep your template formatting separate from your processing.

    <?php
    // do processing of information

    $var = (((PSEUDOCODED DATA OUTPUT)));
    processtemplate($var);




    -------------
    //new file that is included by processtemplate()
?>
    ... blah ... blah ... blah ... blah 
    onclick="[[_KEYNAME_]]"
    ... blah ... blah ... blah ... blah ... blah 
Geekster
  • 336
  • 1
  • 10
  • For more complex things yes. But the ability to mix interpreted PHP with HTML is a strength of using PHP that is completely valid to take advantage of. – Peter Ajtai Sep 14 '10 at 16:14
  • @Peter Ajtai : people keep saying that but I think this is where we've lost our way. Separating your code from your css & html will help you to make changes and test security on, plus if you need to make a change it's always easier to do those kinds of changes without hunting everywhere for them. Of course if you really want to, you can print $vars out in your templates, but I prefer having a system that is like a layer cake. Nobody touches the icing. (if that makes sense to you) – Geekster Sep 17 '10 at 19:34
  • @geekster - I understand what you're saying, but you are describing a new language you wish to build out of PHP ;) – Peter Ajtai Sep 17 '10 at 20:01
  • @Peter Ajtai : Not at all. That's pseudocode. It's really messy to embed code in your templates and that's what you appear to be doing. Pre-process your variables and swap them out. You could do a search/replace on the string "[[_KEYNAME_]]" and that's all it is -- it's a string so that you can swap it out after the fact. You can further optimize it so that your output is preprocessed on change only and then output to files. – Geekster Sep 20 '10 at 13:25
  • @Geekster - What I'm seeing is that one of the design features of PHP is that you **can** embed code directly into HTML templates...... In many situations this is much more straight forward and less error prone than string replacement. Both have a time and place. – Peter Ajtai Sep 20 '10 at 15:52
  • @Peter Ajtai : sure you can, but that doesn't mean you should. You have your beliefs, but in the case of mixing formatting with code, we are in disagreement. Let me give you an example: let's say you have 100 templates and they have 457 different little vars in them that you need to parse together in different ways. What's faster: OPTION A: preparse them all in an included file using a function and swap the static vars in the templates --or-- OPTION B: scatter all the preparsing through the 100 templates in 500 different places so you have to hunt all over your project for them? KISS – Geekster Sep 20 '10 at 16:14
  • @Geekster - Whether it's good or bad, one of the main features and "selling points" of PHP is that you can mix code and formatting. That is why I say you want to create a new language out of PHP. ---- From the first sentence on PHP.net in the about section: `PHP is a widely-used general-purpose scripting language that is especially suited for Web development and can be embedded into HTML.` ---------- And my very first comment began with `For more complex things yes....`, and you're describing relatively complex situations. – Peter Ajtai Sep 20 '10 at 16:34
  • @Peter Ajtai : You lost the debate when you said "whether it's good or bad" meaning you don't care if it's a bad implementation. PHP is a tool; what you build with the tool depends on your level of craftsmanship. If you want to try and argue with an architect that the Burj Khalifa in Dubai could be made out of Popsicle sticks or granite or Kraft Dinner noodles -- technically you could be correct, but would it be the same? Would it be the Burj Khalifa in Dubai? Would the architect respond to your emails? – Geekster Sep 20 '10 at 18:06
  • @Geekster - [ **My architects use toothpicks.** ](http://www.youtube.com/watch?v=6R1BrMPMdNY) – Peter Ajtai Sep 20 '10 at 18:19
  • 1
    @Peter Ajtai : that sums it up. PHP can be a great model if you want to turn everything on and use every single function and premise they devised since they started developing PHP in 1994, and you would have fantastic models. But if you put every function and premise into use that has existed since inception, you would DESTROY any viability of the language. It's a tool -- use it wisely and you will prosper. Take the model > reality approach and watch what happens. (it won't be pretty) – Geekster Sep 20 '10 at 18:37