2
\$\begingroup\$

I have written a small PHP script to get a variable from the URL and echo it in the body ID.

An example of the url:

http://www.example.com/?var=123

I have added this to the head of my HTML page:

<?php if (isset($_GET['var'])) {
    $var = $_GET['var'];
} else {
    $var = NULL;
} ?>

The body tag of the HTML page looks like this:

<body class="whatever" <?php if(isset($var)) {echo "id=\"$var\""; } ?>>

The result is a body tag like this when the ?var=123 is added to the URL:

<body class="whatever" id="123">

This is working but I am sure there is a better way to go about it.

\$\endgroup\$
  • \$\begingroup\$ For retrieving the value of var I would suggest using the ternary option which makes it a lot cleaner and easier to understand. $var = $GET['var'] ?? null; See Comparison Operators for more details. \$\endgroup\$ – Dave Aug 23 '18 at 18:34
  • \$\begingroup\$ Also curious why you are adding an ID to the body tag since there will only ever be one body tag on a page (or should be). \$\endgroup\$ – Dave Aug 23 '18 at 18:41
  • \$\begingroup\$ To make sure I am understanding, I can replace $var = $_GET['var']; } else { $var = NULL; in the head? As for why an ID on the body, long story, but adding a class to the body is not an option and I need to hide certain content depending on which app is loading the webpage. \$\endgroup\$ – ShanePK Aug 23 '18 at 18:47
  • \$\begingroup\$ Yes, that single statement replaces the entirety of the PHP that you posted. How are you hiding content? If it's dynamic content from PHP for example there would of course be no reason to add an ID to the body tag. \$\endgroup\$ – Dave Aug 23 '18 at 18:49
  • \$\begingroup\$ Content is being hidden using CSS. \$\endgroup\$ – ShanePK Aug 23 '18 at 19:18
3
\$\begingroup\$

One technique is to default the value to NULL:

$var = NULL;
if (isset($_GET['var'])) {
    $var = $_GET['var'];
} ?>

Then when setting the attribute on the tag, isset() doesn't need to be used.

<body class="whatever" <?php if($var) {echo "id=\"$var\""; } ?>>

For a demonstration of this, see this phpFiddle.

Note that it is recommended that PHP not be mixed in with HTML code. For a good explanation, refer to this answer to Should I Include PHP code in HTML or HTML in PHP? on the Software Engineering site.

Mixing languages is not a good idea. You don't put JavaScript in HTML, or HTML in JavaScript, or JavaScript in PHP, or HTML in Python or Ruby in SQL.1

As is recommended by that post, consider the use of templates:

What are you probably looking for is called templates. Depending on the framework you use, it may already be available, usually under a form of MVC, where the template is in the view, or you may have to use a third-party template engine, such as Smarty.

In both cases, the idea remains the same. You have PHP code strictly separated from the template which contains the HTML and a bit of very simplistic logic: simple loops over entities, conditions for conditional displaying of information, etc. When the PHP code is ready, it calls the template engine, passing to it some information. The engine uses a specific template to build the final output (often HTML, but other formats are possible as well) which is then sent to the user.1

1https://softwareengineering.stackexchange.com/a/291839/244085

\$\endgroup\$
  • \$\begingroup\$ Worked flawlessly and makes perfect sense. Thanks! \$\endgroup\$ – ShanePK Aug 23 '18 at 19:28
  • 1
    \$\begingroup\$ Specifically the creating a string to represent the id tag. Much cleaner and makes sense. I also implemented the technique you mentioned, setting default value for the var to null. \$\endgroup\$ – ShanePK Aug 23 '18 at 19:55
  • \$\begingroup\$ @ShanePK quite contrary, it is not a good idea to define an HTML element in the business logic section. All HTMl lrelated stuff should be defined in the HTML section. \$\endgroup\$ – Your Common Sense Aug 24 '18 at 10:15
  • \$\begingroup\$ @YourCommonSense bearing in mind that it might have been somebody else who downvoted - is there any change I can make to change that vote? Perhaps mentioning templating? \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Aug 24 '18 at 13:32
  • \$\begingroup\$ I would just keep all HTML elements inside the HTML block, no matter how complex the resulting code will be. Whether it would be a raw PHP file or a template just doesn't matter. \$\endgroup\$ – Your Common Sense Aug 29 '18 at 9:59
2
\$\begingroup\$

This is highly vulnerable for XSS attacks

Always sanitize user input, never output user input without escaping

Your code will execute arbitrary HTML, CSS and JavaScript that a user can pass. Try this simple example:

?var="><script>alert("How's that?")</script>

Newer versions of Chrome and Safari will block JavaScript insertions, but this will still work in the latest Firefox.

Escaping and sanitizing

You can use functions like htmlspecialchars or htmlentities to escape user input. Learn more: How to prevent XSS with HTML/PHP?.

There are also multiple ways of sanitizing your input using PHP's various filter functions.

Alternatively you can create a white list of possible ids and check the input against this list:

$ids = ['value-a', 'value-b'];
$id = in_array($_GET['val'], $ids) ? $_GET['val'] : null;
\$\endgroup\$
  • \$\begingroup\$ This answer, being good by intention, lacks a concrete suggestion. PHP filter functions are numerous and confusing, a learner would have a hard time figuring out which one is needed. Not to mention that any xss stuff should belong to output, not to input filtering. \$\endgroup\$ – Your Common Sense Aug 24 '18 at 10:20
  • \$\begingroup\$ The question does lack information about the the real values, as I don't suppose that 123 is a concrete value. So it's hard to give helpful information, what way would be the most helpful. However, you were right, that this answer wasn't clearly worded - I've updated that. @YourCommonSense \$\endgroup\$ – insertusernamehere Aug 24 '18 at 10:32
  • \$\begingroup\$ Thanks for the input. I have updated the code to use a white list of possible ids. \$\endgroup\$ – ShanePK Aug 24 '18 at 14:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.