formers / former

A powerful form builder, for Laravel and other frameworks (stand-alone too)
https://formers.github.io/former/
1.34k stars 204 forks source link

Escape HTML value of 'plaintext' field by default #605

Closed tortuetorche closed 3 years ago

tortuetorche commented 3 years ago

Basic usage with Laravel and Bootstrap 4

{!! Former::open()->method('GET') !!}
    {!! Former::plaintext('test_static_escaped')->value(
          '<b>Text</b>'
        )
    !!}
    {!! Former::plaintext('test_static')->value(
          new Illuminate\Support\HtmlString('<b>Text</b>')
        )
    !!}
{!! Former::close() !!}

Will produce: former_basic_plaintext_field_escaped

<form accept-charset="utf-8" class="form-horizontal" method="GET">
  <div class="form-group row">
    <label for="" class="col-form-label col-lg-2 col-sm-4">
      Test static escaped
    </label>
    <div class="col-lg-10 col-sm-8">
      <div class="form-control-plaintext" id="test_static_escaped">
        &lt;b&gt;Text&lt;/b&gt;
      </div>
    </div>
  </div>
  <div class="form-group row">
    <label for="" class="col-form-label col-lg-2 col-sm-4">
      Test static
    </label>
    <div class="col-lg-10 col-sm-8">
      <div class="form-control-plaintext" id="test_static">
        <b>Text</b>
      </div>
    </div>
  </div>
</form>

Advanced usage with Laravel and Bootstrap 4

If you populate your form, you should use the forceValue() method instead of the value() method, like this:

@php
    $data = ['text' => '<b>rich text</b>'];
    Former::populate($data);
@endphp
{!! Former::open()->method('GET') !!}
    {!! Former::plaintext('text') !!}
    {!! Former::plaintext('text')
            ->forceValue(
                new Illuminate\Support\HtmlString($data['text'])
        )
    !!}
{!! Former::close() !!}

Will produce: former_populated_plaintext_field_escaped

<form accept-charset="utf-8" class="form-horizontal" method="GET">
  <div class="form-group row">
    <label for="" class="col-form-label col-lg-2 col-sm-4">Text</label>
    <div class="col-lg-10 col-sm-8">
      <div class="form-control-plaintext" id="text">&lt;b&gt;rich text&lt;/b&gt;</div>
    </div>
  </div>
  <div class="form-group row">
    <label for="" class="col-form-label col-lg-2 col-sm-4">Text</label>
    <div class="col-lg-10 col-sm-8">
      <div class="form-control-plaintext" id="text-2"><b>rich text</b></div>
    </div>
  </div>
</form>

But the best way is to directly populate data which are already marked as safe HTML, like this:

$data = [
    'safe_text' => new Illuminate\Support\HtmlString('<b>rich text</b>'),
];
Former::populate($data);
//...

So you don't have to use value() or forceValue() methods in your views 👍

HtmlString class and safe HTML tags

But what is Illuminate\Support\HtmlString class used in the code above? An instance of the HtmlString class can potentially check whether a string it's been given should be treated as HTML or not.

So when you do:

$value = new Illuminate\Support\HtmlString('<b>rich text</b>');

You tell to your Laravel views that the string given, which can contains HTML tags, is safe and shouldn't be escaped. In addition, you should use an HTML purifier to remove automagically unsafe HTML tags (e.g. https://github.com/mewebstudio/Purifier).

Enable or disable this feature with Laravel

In your former config file config/former.php, you can enable or disable this feature:

<?php

return [
    //...

    // Whether Former should escape HTML tags of 'plaintext' fields
    // Enabled by default
    //
    // Instead of disabled this option,
    // you should use the 'HtmlString' class:
    //  Former::plaintext('text')
    //      ->forceValue(
    //          new Illuminate\Support\HtmlString('<b>HTML data</b>')
    //      )
    'escape_plaintext_value' => true,

    //...
];

Breaking change

⚠️ Important ⚠️: This pull request could be considered as a breaking change, but the behavior proposed here is much safer than the previous behavior. Maybe we can add a config option to disable this new behavior? You can disable this new behavior with the escape_plaintext_value former config option set to false, see previous paragraph. But IMHO it should be enabled by default 🔒

tortuetorche commented 3 years ago

Hi @claar and @stayallive,

As you know or not, I'm a member of this repository, so I can merge myself this pull request. But I wasn't a very active contributor until now, so if you're OK I can merge this one, otherwise let me know...

I've also send other pull requests to fix pending issues... Then, my goal is to support Bootstrap 5, if I find the time to do it 🤞

Cheers, Tortue Torche

tortuetorche commented 3 years ago

Hi folks,

I'm going to merge my pending pull requests and then, after some testing, tag a 5.0.0 release 👍

Have a good day, Tortue Torche