Will forcing validation on a node save result in security issues or data integrity loss?

Question

I have a content type containing lots of required and conditional fields. Because it can take a bit of time to properly create these nodes, and not all information will always be available to the end-user, I've implemented the ability for the user to "Save as Draft" while on this node's edit form.

Here's my form alter:

function mymodule_form_node_form_alter(&$form, &$form_state) {
  if ($form['#bundle'] == 'content_type') {
    // If node already exists and is not published, show "Draft" markup.
    if (isset($form['#node']->nid) && $form['#node']->status == 0) {
      $form['draft_markup']['#markup'] = '<div id="node-draft">Draft</div>';
    }

    // Add a new validate function to disable error checking on required fields.
    if (empty($form['#validate'])) {
      $form['#validate'] = array();
    }
    array_unshift($form['#validate'], 'mymodule_save_draft_validate');

    // Add 'Save as Draft' button
    $form['actions']['draft'] = array(
      '#type' => 'submit',
      '#class' => 'form-submit',
      '#value' => t('Save as Draft'),
      '#weight' => '0',
      '#submit' => array('mymodule_save_draft_submit'),
    );

    // Change submit button's text.
    $form['actions']['submit']['#value'] = t('Save & Publish');
  }
}

Custom validate function – the magic lies in &drupal_static('form_set_error', NULL, TRUE), where TRUE resets the static form_set_error variable:

function mymodule_save_draft_validate($form, &$form_state) {
  if ($form_state['triggering_element']['#value'] == $form['actions']['draft']['#value']) {
    // Set as unpublished.
    $form_state['values']['status'] = 0;

    // Force validation.
    $errors = &drupal_static('form_set_error', NULL, TRUE);
  } else {
    // Set as published.
    $form_state['values']['status'] = 1;
  }
}

Custom submit function – drupal_get_messages() stops "required" field errors from displaying when a user saves as draft, while a user doing the normal 'submit' will see the errors:

function mymodule_save_draft_submit($form, &$form_state) {
  drupal_get_messages();
  node_form_submit($form, $form_state);
}

Outside of obviously missing field data (which I'm not worried about since status = 0 and I can filter to never display unpublished nodes to the public) does anything stand out as dangerous here for a long-term, production site? Maybe the answer is simply "no", but forcing validation via &drupal_static('form_set_error', NULL, TRUE) makes me wonder if I'm opening the door to bigger security vulnerabilities. Because fields still run their validation, I am unable to get data to save that shouldn't be saved (i.e. if I try and put PHP code in a number input, the integer "0" is saved into the database - cool). Looking for a bit of peer review here...

Answer 1

Looking at Drupal code, I don't see any possible security flaw; there could be any, depending on the code executed from third-party modules.
For sure, I am expecting many errors raised for form fields that are added to the node edit form and whose content is not validated. None of the modules I know (and which alter the node edit form) validate the user input in the validation, and in the submission handlers they add.

Unconditionally removing any error message is not what I would do. I would rather let the users know when the node they are editing has been already changed from another user, which is the error message returned from node_validate().

  if (isset($node->nid) && (node_last_changed($node->nid) > $node->changed)) {
    form_set_error('changed', t('The content on this page has either been modified by another user, or you have already submitted modifications using this form. As a result, your changes cannot be saved.'));
  }

I would also let the users know there is already a translation for a node in a language, which is the error message given from translation_node_validate().

if (isset($translations[$langcode]) && $translations[$langcode]->nid != $node->nid) {
  form_set_error('language', t('There is already a translation in this language.'));
}

Using drupal_static('form_set_error', NULL, TRUE) (you could simply use drupal_static_reset('form_set_error')), you are resetting all the form errors for any form present in the page. See How do I make form_set_error() affect the submitted form only?

If I were to implement that code, I would write it as follows. I don't think that avoiding the validation errors are seen from the users causes anything good.

function mymodule_form_node_form_alter(&$form, &$form_state) {
  if ($form['#bundle'] == 'content_type') {
    // If node already exists and is not published, show "Draft" markup.
    if (!empty($form['#node']->nid) && $form['#node']->status == 0) {
      $form['draft_markup']['#markup'] = '<div id="node-draft">Draft</div>';
    }

    // Add 'Save as Draft' button
    $form['actions']['draft'] = array(
      '#type' => 'submit',
      '#class' => 'form-submit',
      '#value' => t('Save as Draft'),
      '#weight' => 0,
      '#validate' => array('mymodule_save_draft_validate'),
      // Change the following line to contain the name of the fields that 
      // needs to be validated. As it is, it avoids any form field is validated,
      // when this button is clicked.
      'limit_validation_errors' => array(),
      '#submit' => array('mymodule_save_draft_submit'),
    );

    // Change submit button's text.
    $form['actions']['submit']['#value'] = t('Save & Publish');
  }
}

function mymodule_save_draft_validate($form, &$form_state) {
  $form_state['values']['status'] = 0;
}

function mymodule_save_draft_submit($form, &$form_state) {
  // …
}