set / enum definitions broken

Bug #557870 reported by Adam Łyskawa
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
chive
Fix Released
High
Matthias Burtscher

Bug Description

Using Opera for Windows you cannot properly define "enum" and "set" data types. You get: "enum('a\r','b\r','c')" instead of "enum('a','b','c')". This happens because line endings (EOLs) differ from OS to OS. Some browsers convert EOLs to LFs, some don't. It seems it's not defined clearly in any specification. The conversion should be done server side then, assuming EOL type is always unknown, because it really cannot be determined in any way by the server. It would be highly inefficient to include all cases (LF, CRLF and CR) in regexes and other code (like explode() operations), so my recomendation of fixing this bug is to filter ALL data from POST with simple str_replace(), like this:

define('CR', "\r");
define('LF', "\n");
define('CRLF', "\r\n");

foreach ($_POST as $k => $v) {
  $_POST[$k] = str_replace(CRLF, LF, $v); // Windows case
  $_POST[$k] = str_replace(CR, LF, $v); // Exotic OSes case
}

The following code executed before each operation on $_POST data will prevent ALL line ending related bugs possible with ANY OS / ANY BROWSER, ever. It's good to check line endings in all code which directly depends on them.

I posted this error for Chive 0.2.0 and it was fixed incorrectly, that's why I post detailed description. The error could be impossible to reproduce using OS other than Windows or browsers other than Opera, so please just apply described filtering by default, it's perfectly safe.

Revision history for this message
Adam Łyskawa (a-lyskawa) wrote :

I added described conversion to index.php as quick fix, but it didn't help - extra LFs were added somwhere else, which shows previously bug was fixed in very wrong way causing more damage than profit. Please note the conversion cannot be done any other way. First - it should be done ONLY ONCE in whole code, then the sequence of replacing must be exactly as described or it can produce unpredictable results.

Revision history for this message
Adam Łyskawa (a-lyskawa) wrote :

There was a bug in my fix, I don't know why it didn't work, here's the working fix, add in index.php on very top of it:

/**
 * Class used as namespace for quick fix to Chive project
 * This one applies quick conversion of all possible EOL types to LF character
 */
class quick_fix {
  /**
   * Normalizes all EOL types to LFs in all $data strings
   * @param mixed $data any type, is changed directly instead of returning a value
   */
  static function normalize_eol(&$data) {
    if (is_array($data) || is_object($data)) foreach ($data as &$var) self::normalize_eol($var);
    elseif (is_string($data)) $data = str_replace("\r", "\n", str_replace("\r\n", "\n", $data));
  }
  /**
   * Converts any string into its normalised hexadecimal ASCII representation, used for tests
   * @param string $s any string
   * @return string hexadecimal string like "41 0A 42 0A 43"
   */
  static function strhex($s) {
    foreach (str_split($s) as $c) $h[]= strtoupper(str_pad(dechex(ord($c)), 2, '0', STR_PAD_LEFT));
    return implode(' ', $h);
  }
  /**
   * Applies quick fix
   */
  static function apply() {
    self::normalize_eol($_POST);
  }
}

quick_fix::apply();

This one was tested and works fine. quick_fix::strhex() function is redundant, I left it only for testing. I used class wrapping to avoid possible name conflicts.

David Roth (davrot)
Changed in chive:
assignee: nobody → David Roth (davrot)
importance: Undecided → High
milestone: none → 0.4.0
status: New → Triaged
Revision history for this message
Matthias Burtscher (mburtscher) wrote :

Same issue with Opera for Linux!

Changed in chive:
assignee: David Roth (davrot) → Matthias Burtscher (mburtscher)
Revision history for this message
Matthias Burtscher (mburtscher) wrote :

Reported as issue for Yii Framework:
http://code.google.com/p/yii/issues/detail?id=1378

Waiting for an answer if this issue will be fixed in the framework itself or if we have to implement it on our own.

Changed in chive:
status: Triaged → In Progress
Changed in chive:
status: In Progress → Fix Committed
David Roth (davrot)
Changed in chive:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.