Hidden "secure value" component losing its token (%get, %post) value on webform submission

Thanks for the report! I'll see if I can reproduce it. Do you have any modules that modify the behavior of Webform installed (mollom, captcha, webform_rules, webform_validation, etc)?

Thanks, looks like this problem is reproducible without any additional modules. Happens in the 4.x release too (which uses the new-style tokens).

Same here, though perhaps a different situation:
- no upload field
- but posting some values from another page to the webform
- using %post[...] to populate some hidden fields (entity types and id's)

Some debugging led me to function function _webform_render_hidden() that does the replacement of %get, %post. However this replacement is done twice: when the webform is rendered the first time AND when the webform itself is posted (submitted). The second time however the values for %get and %post are not there anymore.

So, to prevent this rebuilding when the values are not there anymore, webform should set form caching to true. This can be done by this line of code:

$form_state['cache'] = TRUE;

Specific for this problem it needs only to be done when there are secure hidden fields that get their value from a token that won't be there a 2nd time (at least %get, %post). But $form_state is not passed on that deep, so I added the above line to function webform_client_form(). It did work for me, but I am not sure if and how this may influence e.g. multi-page webforms with auto-saving by anonymous users.

Making title more general.

I'll be tuned to this as well.

subscribe

I had the same issue when passing the values programmatically. Fixed this by adding a form state..

function HOOK_form_alter(&$form,&$form_state,&$form_id){

  $job_applications_form = variable_get('job_application_form',16);

  if($form['#form_id'] == "webform_client_form_".$job_applications_form){

    // Keeps the job id on the form build, used fo when user uploads file
    if(isset($_GET['job']) && !isset($form_state['jid']))
    {
      $form_state['jid'] = $_GET['job'];
      $form_state['job_node'] = node_load(intval($form_state['jid']));
    }

    $job_node = $form_state['job_node']; // sets the job id.
    // Sets the default value being passed from the job node.

    $job_type = $job_node->field_job_type['und'][0]['value'];
    $job_location = $job_node->field_job_location['und'][0]['safe_value'];
    $job_state = $job_node->field_state['und'][0]['value'];

    // Adding the Job details into the form. 
    $form['submitted']['job_details']['field_job_type']['#default_value'] = $job_type;
    $form['submitted']['job_details']['field_job_location']['#default_value'] = $job_location;
    $form['submitted']['job_details']['field_state']['#default_value'] = $job_state;
  }
}

I've been working with Availability Calendars, and I've found that if I make, say, entity_id a hidden field with %post[entity_id], it doesn't show up in my Webform results; but if I make it a regular ol' textfield, it shows up just fine.

In other words, it's not just the fields specific to the Availability Calendars module, but basic core fields. I think. I'll try good ol' nid and report back if it's any different, but I doubt it.

I'm using Webform 7.x-3.19, if that helps.

@firstlut: Are you visiting the page that contains the Webform by sending a POST request to it? How are you expecting %post[entity_id] to be populated? It's much more common to use %get tokens when populating a field, because it's easy to link to the form with a query string appended to the URL.

I've been using POST because the README for AC said to.

I'm not a coder, and I only have a vague understanding of the difference between the two. GET sounds grabby, which is what I need.

I'll continue my experiments with your suggestion.

Thank you so much!

I think that it is not so much a difference between POST and GET

1) we have multiple pages that all link to the same webform (reservation request). (in our case the pages represent a resource for rent and the webform is the reservation request).
2) To distinguish what page has sent the visitor to the webform, some id's are passed, either via
2.1) GET: path/to/weborm?id1=value1&id2=value2
2.2) POST path/to/webform (values in request body)
(in our case: resource id and start and end dates as the user selects these via an interactive availability calendar on the resource page).
3) Hidden fields are populated with these values
4) Webform is rendered and presented to visitor
5) Webform is completed by visitor
6) Webform is submitted to webserver by a POST: path/to/webform (filled in values in request body)

In step 6 the hidden values as were passed to the webform in step 2 are not available anymore in the request data as they were not sent to the client, this true independent if step 2 was a GET or a POST. Because even with a GET the action attribute of the form will be the "clean url" of the webform, i.e. without the original parameters, thus just path/to/webform and not path/to/weborm?id1=value1&id2=value2.

So the hidden values should be reconstructed on the server. They are not computable, so this is only possible when they are cached in the (server side) form cache.

OK, now I get it. Kind of.

So the reason the "Hidden element" workaround works is that it's just hiding the field, like if I made CID a text field and hid it with CSS. Is that about right?

P.S. I corrected the spelling/grammar in the post title. Hope that's OK.

Thanks @fietserwin for your extensive analysis of the situation. Put into that context it explains a lot of the reason why %get or %post variables don't work as expected when dealing with a "secure" hidden field (one that isn't rendered client-side).

So, to prevent this rebuilding when the values are not there anymore, webform should set form caching to true. This can be done by this line of code:

$form_state['cache'] = TRUE;

I'm a little worried about taking this approach because it would result in one cache entry every time the form was viewed. It seems like it could easily cause thousands or millions of cache entries to be created if just by viewing the form it created a cache entry.

In the 4.x branch, we don't have %post any more because it's not (natively) provided by Token module, so you're left with %get, which we can "fix" by including the query string in the form's "action" property.

Because even with a GET the action attribute of the form will be the "clean url" of the webform, i.e. without the original parameters, thus just path/to/webform and not path/to/weborm?id1=value1&id2=value2.

In the latest releases (3.19 and higher), we included a fix to the GET query strings in #250767: #action breaks webform inside another node. It's only if you have a form that is either displayed in a teaser or in a block that the "action" property is modified at all, and in those situations values in the $_GET array are now set manually. So I *think* in the 3.19 version, all problems with the %get token should already be addressed.

#13: That would make it an unsecure hidden field. Don't do that! Keep it server side.

#14':

it would result in one cache entry every time the form was viewed.

We could set $form_state['cache'] to true only if there are secure hidden values populated with "external" tokens, but I am not sure if we can find out so. (it should be easy though to spot webforms with secure hidden default values populated with tokens.

- Caching a form prevents rebuilding it on submission, so can be faster in the end.
- cache_form table entries expire in 6 hours (by default) and cron removes them.
- In D8 it won't be a table anymore: #512026: Move $form_state storage from cache to new state/key-value system.
- Multi step forms are cached by default (to retain info from previous steps).
- Forms with ajax handling should be cached as ajax requests cannot rebuild forms from scratch

So I am not so worried about caching, though the #include's should be OK on all the form components defined by this module (I remember posting another issue about that).

It's only if you have a form that is either displayed in a teaser or in a block that the "action" property is modified at all,

So that issue does not solve this issue for %get for all situations? (let alone %post).

Perhaps offer it as a choice so people can disable it on webforms that are viewed extremely often but rarely submitted (a poll in a sidebar)? I am willing to look into solving it the "enable it on webforms with secure hidden default values populated with tokens" way.

Aside: to solve it on your own side without hacking the module (as I did in #3):

/**
 * Implements hook_form_FORM_ID_alter().
 * @see http://api.drupal.org/api/drupal/modules--system--system.api.php/function/hook_form_FORM_ID_alter/7
 *
 * Alters webforms, so they are cached.
 */
function mymodule_form_webform_client_form_alter(&$form, &$form_state, $form_id) {
  // #1580700: Hidden "secure value" component loosing it's token (%get, %post)
  // value on webform submission.
  $form_state['cache'] = TRUE;
}

So that issue does not solve this issue for %get for all situations? (let alone %post).

The problem with %get should be solved in all situations (though I haven't tested this scenario myself). Even though we modify the #action property on the form, we merge in all the existing variables from $_GET so the destination page still has all the same query string values.

- Caching a form prevents rebuilding it on submission, so can be faster in the end.

That's an interesting perspective. I hadn't thought about using the cached form as providing a savings in processing (though that's clearly what "caching" is supposed to do!) In the 4.x branch (or maybe it will wait until the next version yet, I'm not sure), I'm considering using CTools temporary data store instead of $form_state['storage'] for holding form values. That would make features like #335480: Set separate URL for each page of Multi-Page Forms (Funnel Tracking) possible. So I'm definitely open to using the database (or equivalent) to hold form values. I'll have another look at just enabling the $form_state['cache'] property and see what all it's side-effects are. Thanks for your insight on this issue!

Hi,

This is my first post on this forum, so any advice is welcome.

I have read through the items above and updated my webform module to 7.x-3.19. I am still reproducing the bug with secure value using %get.

Has anyone had any success in trying to resolve it?

Hi @cruser, welcome to drupal.org.

This problem hasn't been solved yet in an official release, so new downloads of the current version will still have it. There is a solution to solve this problem however. For now, the best approach is to make a small custom module (with just a module.info and module.module file) and put the code in the .module file that @fietserwin posted in comment #14.

Ultimately, I think we'll just include the line he's suggesting in the module directly. I think as a solution, it's probably the most accessible and direct way of solving the problem, even though it has some minor drawbacks.

It would be great if the suggested line would end up in the module.

Would it otherwise be an option that you'll create a module that can be downloaded from the Drupal site so it will also be included in the updates.

Since these token were removed in 7.x-4.x, this issue affects only the -3.x branch. Version 7.x-3.x is receiving critical bug fixes only.