# HG changeset patch
# User Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
# Date 2013-04-05 00:49:03
# Node ID 1c451d4983166e368fc087955ddc70edcfc298cf
# Parent  84dacca3fe100be1a5dd90e3780ae68d001c4a38

Enable users who fail to meet all the feedback requirements tested in feedback-submit.php to return to feedback.php without losing their comments (when using the “Try again” button).

diff --git a/feedback-submit.php b/feedback-submit.php
--- a/feedback-submit.php
+++ b/feedback-submit.php
@@ -31,23 +31,30 @@ if ($use_captcha)
 
 $feedbackpage = page::page_create('Feedback');
 $feedbackpage->head();
-$subject = '[SlatePermutate] - Feedback';
-?>
 
-<h3>Thanks!</h3>
+if (isset($_GET['success']))
+  {
+    echo '<h3>Thanks</h3>' . PHP_EOL
+      . '<p>Thanks for helping make SlatePermutate better. Your feedback is greatly appreciated.</p>' . PHP_EOL
+      . '<p>We will attempt to respond via email if your feedback lends itself to a response.</p>' . PHP_EOL;
+    $feedbackpage->foot();
+    exit();
+  }
 
-<?php
+$subject = '[SlatePermutate] - Feedback';
 
-$ip = $_POST['ip'];
-$httpagent = $_POST['httpagent'];
-$fromdom = $_POST['fromdom'];
-$nameis = $_POST['nameis'];
-$visitormail = $_POST['visitormail'];
-$school = $_POST['school'];
+$ip = $_SERVER['REMOTE_ADDR'];
+$httpagent = $_SERVER['HTTP_USER_AGENT'];
+$user_supplied_params = array('fromdom', 'nameis', 'visitormail', 'school', 'feedback', 'rating', 'referrer');
+foreach ($user_supplied_params as $var)
+  {
+    if (isset($_POST[$var]))
+      ${$var} = $_POST[$var];
+    else
+      /* Obviously, the user has not actually  */
+      page::redirect('feedback.php');
+  }
 $school_id = isset($_SESSION['school']) ? $_SESSION['school'] : '';
-$feedback = $_POST['feedback'];
-$rating = $_POST['rating'];
-$referrer = $_POST['referrer'];
 
 $saved_schedules = array();
 if (!empty($_SESSION['saved']))
@@ -56,36 +63,41 @@ if (!empty($_SESSION['saved']))
 $saved_schedules = implode(', ', $saved_schedules);
 
 $reject = FALSE;
+$messages = '';
 
 if (preg_match('/https?:/i', $feedback)) { 
-  echo '<p>Please do not include URLs in your submission! Please click "back" and try again.</p>';
+  $messages .= '<p>Please do not include URLs in your submission!</p>' . PHP_EOL;
   $reject = TRUE;
 }
-if (empty($visitormail) || !preg_match('/^[^@]+@[^@]+\.[^@]+$/', $visitormail)) {
-  echo '<p>Please click "back" and enter valid e-mail address.</p>';
+if (empty($visitormail) || !preg_match('/^[^@]+@[^@]+\.[^@]+$/', $visitormail)
+    || !($visitormail = filter_var($visitormail, FILTER_VALIDATE_EMAIL)))
+  {
+  $messages .= '<p>Please enter a valid e-mail address.</p>' . PHP_EOL;
   $reject = TRUE;
 }
 if(empty($nameis) || empty($feedback) || empty($visitormail)) {
-  echo '<p>Please click "back" and fill in all fields.</p>';
+  $messages .= '<p>You must fill in in all of the fields.</p>' . PHP_EOL;
   $reject = TRUE;
 }
 
 /** Check the captcha */
 if ($use_captcha)
   {
-    if (!$securimage->check($_REQUEST['captcha_code']))
+    if (empty($_REQUEST['captcha_code'])
+	|| !$securimage->check($_REQUEST['captcha_code']))
       {
-	echo '<p>Your captcha response was incorrect or expired. Please try again.</p>';
+	$messages .= '<p>Your captcha response was incorrect or expired.</p>';
 	$reject = TRUE;
       }
   }
 
+$success = FALSE;
 if (!$reject)
   {
     $feedback = stripcslashes($feedback);
 
     $message = gmdate('l, F j, Y, g:i a') ."
-From: $nameis ($visitormail)
+From: $nameis <$visitormai>
 School: $school ($school_id)\n
 Rating: $rating 
 Feedback: $feedback 
@@ -102,7 +114,11 @@ saved_schedules = $saved_schedules
     /* $feedback_emails has its default set in inc/class.page.inc, can be set in config.inc */
     foreach($feedback_emails as $toaddr)
       {
-	mail($toaddr, $subject, $message, $from);
+	$success = mail($toaddr, $subject, $message, $from);
+	if (!$success)
+	  {
+	    $messages .= '<p>This Slate Permutate installation is misconfigured and unable to send email. Please contact the administrator of this website using a more direct means if possible.</p>' . PHP_EOL;
+	  }
       }
 
     if($feedback_disk_log) {
@@ -110,7 +126,16 @@ saved_schedules = $saved_schedules
       fwrite($file, $message . "----------------------------------------\n");
       fclose($file);
     }
-    echo '<p>Thanks for helping make SlatePermutate better. Your feedback is greatly appreciated.</p>';
-    echo '<p>We will attempt to respond via email if your feedback lends itself to a response.</p>';
   }
-    $feedbackpage->foot();
+if ($success)
+  page::redirect('feedback-submit.php?success');
+else
+  echo '<h3>Error</h3>' . PHP_EOL
+    . $messages;
+
+$repost = array();
+foreach ($user_supplied_params as $user_supplied_param)
+  $repost[$user_supplied_param] = $_POST[$user_supplied_param];
+echo $feedbackpage->query_formbutton('feedback.php', $repost, $feedbackpage->entities('try again'), '<p>Consider the error messages, then ', '.</p>');
+
+$feedbackpage->foot();
diff --git a/feedback.php b/feedback.php
--- a/feedback.php
+++ b/feedback.php
@@ -23,9 +23,8 @@ require_once 'inc/class.schedule.php';
 
 $feedbackpage = page::page_create('Feedback');
 $feedbackpage->head();
-$ipi = $_SERVER['REMOTE_ADDR'];
+
 $fromdom = $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];
-$httpagenti = $_SERVER['HTTP_USER_AGENT'];
 
 $referrer = '';
 if (!empty($_SERVER['HTTP_REFERER']))
@@ -51,13 +50,11 @@ if (isset($_GET['feedback']))
 
 <form action="feedback-submit.php" method="post">
 <div id="feedback-form-content">
-<input type="hidden" id="ip" name="ip" value="<?php echo $ipi ?>" />
-<input type="hidden" id="fromdom" name="fromdom" value="<?php echo $fromdom ?>" />
-<input type="hidden" id="httpagent" name="httpagent" value="<?php echo $httpagenti ?>" />
+<input type="hidden" id="fromdom" name="fromdom" value="<?php echo htmlentities($fromdom, ENT_QUOTES); ?>" />
 
 <table>
-<tr><td><label for="nameis">Name: </label></td><td><input type="text" id="nameis" name="nameis" size="20" /></td></tr>
-<tr><td><label for="visitormail">Email:</label></td><td><input type="text" id="visitormail" name="visitormail" size="20" /></td></tr>
+<tr><td><label for="nameis">Name: </label></td><td><input type="text" id="nameis" name="nameis" size="20" value="<?php echo empty($_REQUEST['nameis']) ? '' : page::entities($_REQUEST['nameis']);?>" /></td></tr>
+<tr><td><label for="visitormail">Email:</label></td><td><input type="text" id="visitormail" name="visitormail" size="20" value="<?php echo empty($_REQUEST['visitormail']) ? '' : page::entities($_REQUEST['visitormail']);?>" /></td></tr>
 <tr><td><label for="school">School: </label></td><td><input type="text" id="school" name="school" value="<?php echo htmlentities($school['id']); ?>" size="20" /> <span class="graytext">(if relevant to your feedback)</span></td></tr>
   <tr><td><label for="referrer">Relevant Page:</label></td><td><input type="text" id="referrer" name="referrer" value="<?php echo htmlentities($referrer); ?>" size="20" /> <span class="graytext">(if relevant to your feedback)</span></td></tr>
 </table>
diff --git a/inc/class.page.php b/inc/class.page.php
--- a/inc/class.page.php
+++ b/inc/class.page.php
@@ -823,6 +823,25 @@ class page
 
   /**
    * \brief
+   *   Return an array of name=value pairs that are urlencoded.
+   *
+   * Supports query_string() and query_formbutton().
+   */
+  private static function _uriencode_query_array(array $query)
+  {
+    $query_string_parts = array();
+    foreach ($query as $param => $values)
+      {
+	if (!is_array($values))
+	  $values = array($values);
+	foreach ($values as $value)
+	  $query_string_parts[] = rawurlencode($param) . '=' . rawurlencode($value);
+      }
+    return $query_string_parts;
+  }
+
+  /**
+   * \brief
    *   Form a query string from a map.
    *
    * \param $query
@@ -836,17 +855,136 @@ class page
    */
   public static function query_string(array $query, $question = TRUE)
   {
-    $query_string_parts = array();
-    foreach ($query as $param => $values)
+    $query_string_parts = self::_uriencode_query_array($query);
+    if (count($query_string_parts))
+      return ($question ? '?' : '') . implode('&', $query_string_parts);
+    return '';
+  }
+
+  /**
+   * \brief
+   *   Return an HTML form button which submits all keys, as many of
+   *   them with GET as possible.
+   *
+   * Allows one to automatically delegate fatter values to be POSTed
+   * to prevent the querystring from getting too long and making the
+   * URI itself become too long. Always returns a <form/> with a
+   * <button/>. The <form/> may be method="GET", unless there is too
+   * much data in which case it becomes method="POST".
+   *
+   * Currently, this function will mess up the order of parameters. If
+   * order matters, this function will not work for you.
+   *
+   * \sa query_string()
+   *   An alternative to calling query_string() when unbounded amounts
+   *   of data may need to be transmitted.
+   *
+   * \param $uri
+   *   The URI to submit the data to. Will be used as-is.
+   * \param $query
+   *   The map of parameters onto values.
+   * \param $button_html
+   *   A valid XHTML fragment to place inside of the button, such as
+   *   page::entities($text) telling the user what the button does.
+   * \param $button_pre_html
+   *   The HTML which wraps around the <button/>, such as the opening
+   *   of a <p/> within which a <button/> may be placed.
+   * \param $button_post_html
+   *   The close of the HTML wrapping around the <button/>, such as
+   *   the closing of a <p/>.
+   */
+  public function query_formbutton($uri, array $query, $button_html, $button_pre_html, $button_post_html)
+  {
+    /*
+     * Recommended, but low, upper URI limit. Modern browsers can
+     * handle around 2000+ chars, so could be upped to 2000 without
+     * harm probably.
+     */
+    $uri_len_limit=255;
+
+    /*
+     * Calculate urlencoded lengths of param/values so as to greedily
+     * take the smallest params into GET…
+     */
+    $uriencoded_parts = self::_uriencode_query_array($query);
+
+    /*
+     * Join parameters of the same name together…
+     */
+    $flirting_uriencoded_parts = array();
+    foreach ($uriencoded_parts as $uriencoded_part)
+      {
+	list($key) = explode('=', $uriencoded_part);
+	if (empty($flirting_uriencoded_parts[$key]))
+	  $flirting_uriencoded_parts[$key] = $uriencoded_part;
+	else
+	  $flirting_uriencoded_parts[$key] .= '&' . $uriencoded_part;
+      }
+
+    usort($flirting_uriencoded_parts, function($a, $b) {
+	$a_strlen = strlen($a);
+	$b_strlen = strlen($b);
+	/*
+	 * There is no “ursort()”, so reverse the sort so that
+	 * shortest is first.
+	 */
+	return $a_strlen > $b_strlen ? 1 : $a_strlen == $b_strlen ? 0 : -1;
+      });
+
+    $uri_orig = $uri;
+    $query_orig = $query;
+    if (strpos($uri, '?') === FALSE
+	&& !empty($uriencoded_parts))
+      $uri .= '?';
+    $uri_len = strlen($uri);
+
+    $first = TRUE;
+    foreach ($flirting_uriencoded_parts as $last => $flirting_uriencoded_part)
+      {
+	if (($new_uri_len = ($first ? 0 : 1) + $uri_len + strlen($flirting_uriencoded_part)) > $uri_len_limit)
+	  break;
+	if ($first)
+	  $first = FALSE;
+	else
+	  $uri .= '&';
+
+	$uri .= $flirting_uriencoded_part;
+	$uri_len = $new_uri_len;
+
+	/*
+	 * Drop this param from the $query array as we have taken care
+	 * of it and don’t need to have it be in POST.
+	 */
+	list($key) = explode('=', $flirting_uriencoded_part);
+	$key = rawurldecode($key);
+	unset($query[$key]);
+      }
+
+    if (empty($query))
+      {
+	$method = 'get';
+	/*
+	 * When making a <form method="get"/>, the browser will clear
+	 * out the entire querystring portion of the URI. Thus, we
+	 * need to reformat everything as <input/>… We can only have
+	 * some things in action="" as GET params if our form is POST.
+	 */
+	$query = $query_orig;
+	$uri = $uri_orig;
+      }
+    else
+      $method = 'post';
+    $form = '<form method="' . self::entities($method) . '" action="' . self::entities($uri) . '">' . PHP_EOL;
+    foreach ($query as $key => $values)
       {
 	if (!is_array($values))
 	  $values = array($values);
 	foreach ($values as $value)
-	  $query_string_parts[] = rawurlencode($param) . '=' . rawurlencode($value);
+	  $form .= '  <input type="hidden" name="' . self::entities($key) . '" value="' . self::entities($value) . '" ' . $this->element_self_close() . '>' . PHP_EOL;
       }
-    if (count($query_string_parts))
-      return ($question ? '?' : '') . implode('&', $query_string_parts);
-    return '';
+    return ''
+      . $form . '  ' . $button_pre_html . '<button type="submit">' . $button_html . '</button>' . $button_post_html . PHP_EOL
+      . '</form>';
   }
 
   /**
@@ -958,4 +1096,16 @@ class page
       return ' /';
     return '';
   }
+
+  /**
+   * \brief
+   *   Encode things using htmlentities() with proper precautions.
+   */
+  public static function entities($text)
+  {
+    $opts = ENT_QUOTES;
+    if (defined('ENT_XML1'))
+      $opts |= ENT_XML1;
+    return htmlentities($text, $opts, 'utf-8');
+  }
 }