Skip to main content

This site requires you to update your browser. Your browsing experience maybe affected by not having the most up to date version.

We've moved the forum!

Please use forum.silverstripe.org for any new questions (announcement).
The forum archive will stick around, but will be read only.

You can also use our Slack channel or StackOverflow to ask for help.
Check out our community overview for more options to contribute.

Archive /

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

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

Form saving of Readonly-Fields


Go to End


3 Posts   2333 Views

Avatar
Ingo

Forum Moderator, 801 Posts

11 September 2007 at 10:53am

Edited: 11/09/2007 10:55am

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?

Avatar
Ingo

Forum Moderator, 801 Posts

11 September 2007 at 12:27pm

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.

Avatar
Ingo

Forum Moderator, 801 Posts

11 September 2007 at 12:28pm

there we go