Issue #3052757 by Pancho, Liam Morland: Make parsing of Boolean query parameters consistent

fillpdf.info

@@ -8,6 +8,7 @@ test_dependencies[] = webform
 test_dependencies[] = features
 test_dependencies[] = webform_features
 ; Note: Tests require PHP 5.4.
+files[] = tests/FillPdfLinkContextTestCase.test
 files[] = tests/FillPdfMergeTestCase.test
 files[] = tests/FillPdfTestCase.test
 files[] = tests/FillPdfTestHelper.test

fillpdf.module

@@ -365,9 +365,20 @@ function fillpdf_link_to_stub_context($uri) {
     'flatten' => NULL,
   );
 
-  $context['sample'] = $query_string['sample'];
   $context['fid'] = $query_string['fid'];
 
+  if (isset($query_string['download']) && filter_var($query_string['download'], FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) === TRUE) {
+    $context['force_download'] = TRUE;
+  }
+
+  if (isset($query_string['flatten']) && $query_string['flatten'] !== '' && filter_var($query_string['flatten'], FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) === FALSE) {
+    $context['flatten'] = FALSE;
+  }
+
+  if (isset($query_string['sample']) && filter_var($query_string['sample'], FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) === TRUE) {
+    $context['sample'] = TRUE;
+  }
+
   if ($query_string['nid'] || $query_string['nids']) {
     $context['nids'] = ($query_string['nid'] ? array($query_string['nid']) : $query_string['nids']);
   }
@@ -414,14 +425,6 @@ function fillpdf_link_to_stub_context($uri) {
     $context['entity_ids'] = isset($query_string['entity_id']) ? array($query_string['entity_id']) : $query_string['entity_ids'];
   }
 
-  if (isset($query_string['download']) && (int) $query_string['download'] === 1) {
-    $context['force_download'] = TRUE;
-  }
-
-  if (isset($query_string['flatten']) && (int) $query_string['flatten'] === 0) {
-    $context['flatten'] = FALSE;
-    return $context;
-  }
   return $context;
 }
 
@@ -564,7 +567,7 @@ function fillpdf_merge_pdf($fid, $nids = NULL, $webform_array = NULL, $sample =
     $transform_string = FALSE;
 
     // Fill a sample PDF & return.
-    if ($sample == 'true') {
+    if ($sample) {
       $fields[$obj->pdf_key] = $obj->pdf_key;
       // If sampling, return to the form edit page.
       $_GET['destination'] = "admin/structure/fillpdf/$fid";

tests/FillPdfLinkContextTestCase.test

+<?php
+
+/**
+ * Tests some unit test cases.
+ */
+class FillPdfLinkContextTestCase extends DrupalWebTestCase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getInfo() {
+    return array(
+      'name' => 'FillPDF query parameter unit test',
+      'description' => 'Unit tests evaluation of query parameters.',
+      'group' => 'FillPDF',
+    );
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setUp() {
+    // Enable any modules required for the test. This should be an array of
+    // module names.
+    parent::setUp(array('fillpdf'));
+  }
+
+  /**
+   * Tests boolean query parameters.
+   */
+  public function testBooleans() {
+    foreach ($this->dataProvider() as $case) {
+      foreach ($case as $input => $expected) {
+        $request_context = fillpdf_link_to_stub_context($this->link($input));
+        $this->assertEqual(is_null($expected) ? FALSE : $expected, $request_context['sample']);
+        $this->assertEqual(is_null($expected) ? FALSE : $expected, $request_context['force_download']);
+        $this->assertEqual(is_null($expected) ? TRUE : $expected, $request_context['flatten']);
+      }
+    }
+  }
+
+  /**
+   * Input helper for testBooleans().
+   *
+   * @param string $input
+   *   The string to set as the query parameter value.
+   *
+   * @return string
+   *   The full URL.
+   */
+  public function link($input) {
+    return fillpdf_pdf_link(1, 1) . '&sample=' . $input . '&download=' . $input . '&flatten=' . $input;
+  }
+
+  /**
+   * Data provider for testBooleans().
+   *
+   * @return array
+   *   Array of test cases.
+   */
+  public function dataProvider() {
+    return array(
+      array('1' => TRUE),
+      array('true' => TRUE),
+      array('True' => TRUE),
+      array('TRUE' => TRUE),
+      array('on' => TRUE),
+      array('On' => TRUE),
+      array('ON' => TRUE),
+      array('yes' => TRUE),
+      array('Yes' => TRUE),
+      array('YES' => TRUE),
+
+      array('0' => FALSE),
+      array('false' => FALSE),
+      array('False' => FALSE),
+      array('FALSE' => FALSE),
+      array('off' => FALSE),
+      array('Off' => FALSE),
+      array('OFF' => FALSE),
+      array('no' => FALSE),
+      array('No' => FALSE),
+      array('NO' => FALSE),
+
+      // These three are important, so should always be obeyed:
+      array('' => NULL),
+      array('foo' => NULL),
+      array('bar' => NULL),
+
+      // The following ones are less fortunate, so may be refactored:
+      array('-1' => NULL),
+      array('2' => NULL),
+      array('y' => NULL),
+      array('Y' => NULL),
+      array('n' => NULL),
+      array('N' => NULL),
+    );
+  }
+
+}