Jump to:

17452 Posts in 4473 Topics by 1971 members

Archive

SilverStripe Forums » Archive » Form saving of Readonly-Fields

Our old forums are still available as a read-only archive.

Moderators: martimiz, Sean, biapar, Willr, Ingo, simon_w

Page: 1
Go to End
Author Topic: 1778 Views
  • Ingo
    Avatar
    Forum Moderator
    801 Posts

    Form saving of Readonly-Fields Link to this post

    Hayden and me came across some weird behaviour of Silverstripe Form handling: even if you make fields in a form readonly, they show up with the following markup:
    [html]
    <div id="EventName" class="field readonly">
    <label class="left" for="Form_EditForm_EventName">
    </label>
    <span class="middleColumn">
    <span id="Form_EditForm_EventName" class="readonly">myval</span>
    <input type="hidden" value="myval" name="EventName"/>
    </span>
    </div>
    [/html]
    apparently the <input type="hidden"> is used for easier javascript-access to the form-element values. problem is that these values save back with the form, which is not desireable from a security-perspective and causes strange side-effects in our case.

    i've looked into the form-saving code:

    // Form.php
    function saveInto(DataObject $dataObject) {   
    $dataFields = $this->fields->dataFields();
    ...
    }

    // FieldSet.php
    public function dataFields() {
       if(!$this->sequentialSet) $this->collateDataFields($this->sequentialSet);
       return $this->sequentialSet;
    }

    protected function collateDataFields(&$list) {
       foreach($this as $field) {
          if($field->isComposite()) $field->collateDataFields($list);
          if($field->hasData()) {
             $name = $field->Name();
             if(isset($list[$name])) {
                if($this->form) $errSuffix = " in your '{$this->form->class}' form called '" . $this->form->Name() . "'";
                user_error("collateDataFields() I noticed that a field called '$name' appears twice$errSuffix.", E_USER_ERROR);
             }
             $list[$name] = $field;
          }
       }
    }

    1. dataFields() seems a valid name for getting all fields including readonly (in the end, they store data as well, unlike e.g. CompositeFields). how about replacing this call with writeableFields(), which exludes all fields with readonly/disabled transformations?
    we'd gain a bit of data-security by this restriction as well, as you can't just fake POST-data to overcome readonly-fields...
    2. (optional) remove <input hidden> in readonly-implementations of fields - rather add a class "data" to the element holding the data in the div/span-structure noted above.

    as this is a pretty low-level change, can you think of any side-effects, or custom implementations that this might blow up?

  • Ingo
    Avatar
    Forum Moderator
    801 Posts

    Re: Form saving of Readonly-Fields Link to this post

    i gave solution 1 a shot at http://trac.silverstripe.com/silverstripe/changeset/41553 - a bit of code review on this would be greatly appreciated its still in a project-branch of sapphire, but a good candidate for merging back soon.
    diff is attached for those of you without trac-access.

  • Ingo
    Avatar
    Forum Moderator
    801 Posts

    Re: Form saving of Readonly-Fields Link to this post

    there we go

    1778 Views
Page: 1
Go to Top

Want to know more about the company that brought you SilverStripe? Then check out SilverStripe.com

Comments on this website? Please give feedback.