fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

Should & in urls be &? #1124

Closed pachico closed 11 years ago

pachico commented 12 years ago

Php's dom class allows you to parse an entire DOM using the loadHTML method and check for errors. Interesting, since you could inject any (x)html response of your main controller (or controller templates) and append to the log any validation issues in development environment. This way you can always have control over the dom. So far, so good.

However, Uri:create(), when get params are added creates a url like this: script?foo=bar&foo1=bar1 This, in most validators, is wrong, including php DOM class, since it should be: script?foo=bar&foo1=bar1

Please, read http://htmlhelp.com/tools/validator/problems.html

Now, would it be interesting to transform & to & in Uri::create and avoid validation problems? Would that have collateral implications?

(Please, don't reply things already written in the url I posted above).

Cheers

dudeami0 commented 12 years ago

The behavior you noted is expected, and you'll notice you'll get similar results with Html::anchor(). The reason Uri::create() gives out a url that is not safe for HTML is two-fold. First off, you may not know what the URL is being used for, and certainly can't assume its in HTML. The next reason is because of the way Views work, filtering the variables before they get used. I wrote up an example of how this works, and how to get the results you expect.

public function action_anchors() {
    $params = array(
        'foo' => 'bar',
        'ampersand' => '&&&'
    );

    $url = Uri::create('http://fuelphp.com/', array(), $params);
    // Output: http://fuelphp.com/?q=Ampersand&ampersand=%26%26%26

    // This will render incorrectly in a view, or just be invalid HTML.
    $anchor = Html::anchor($url, 'FuelPHP.com');
    // Output: <a href="http://fuelphp.com/?q=Ampersand&ampersand=%26%26%26">FuelPHP.com</a>"

    // Setup data for view
    $data = array(
        'url' => $url,
        'anchor' => $anchor
    );

    return Response::forge(View::forge('test/anchor', $data));
}

Output of the view:

    <a href="http://fuelphp.com/?foo=bar&amp;ampersand=%26%26%26">FuelPHP.com</a>
    <a href="http://fuelphp.com/?foo=bar&amp;ampersand=%26%26%26">FuelPHP.com</a>
    &lt;a href=&quot;http://fuelphp.com/?foo=bar&amp;ampersand=%26%26%26&quot;&gt;FuelPHP.com&lt;/a&gt

Contents of APPPATH/views/test/anchor.php:

    <a href="<?php echo $url; ?>">FuelPHP.com</a>
    <?php echo Html::anchor($url, 'FuelPHP.com')."\n"; ?>
    <?php echo $anchor; ?>

Explaination of code:

The first function is Uri::create(). This works as expected, doing a urlencode() on the name-value pairs.

Now, you have a url in $url that is not ready for HTML. The next line of code creates an anchor, but this will either be invalid HTML, or will become totally escaped in a view (See line #3 of output).

The way you get a valid HTML anchor is by either echoing it in plain HTML (line #1 of output and view code) or using the Html::anchor() method and echoing the results of that inside the view (line #2 of output and view code).

WanWizard commented 11 years ago

I have nothing more to add to this response.

pachico commented 11 years ago

I agree, thanks!