diff --git a/src/Symfony/Component/DomCrawler/Field/ChoiceFormField.php b/src/Symfony/Component/DomCrawler/Field/ChoiceFormField.php index 8575e7216f..9720ea41cc 100644 --- a/src/Symfony/Component/DomCrawler/Field/ChoiceFormField.php +++ b/src/Symfony/Component/DomCrawler/Field/ChoiceFormField.php @@ -123,10 +123,10 @@ class ChoiceFormField extends FormField */ public function setValue($value) { - if ('checkbox' == $this->type && false === $value) { + if ('checkbox' === $this->type && false === $value) { // uncheck $this->value = null; - } elseif ('checkbox' == $this->type && true === $value) { + } elseif ('checkbox' === $this->type && true === $value) { // check $this->value = $this->options[0]['value']; } else { @@ -167,14 +167,14 @@ class ChoiceFormField extends FormField */ public function addChoice(\DOMElement $node) { - if (!$this->multiple && 'radio' != $this->type) { + if (!$this->multiple && 'radio' !== $this->type) { throw new \LogicException(sprintf('Unable to add a choice for "%s" as it is not multiple or is not a radio button.', $this->name)); } $option = $this->buildOptionValue($node); $this->options[] = $option; - if ($node->getAttribute('checked')) { + if ($node->hasAttribute('checked')) { $this->value = $option['value']; } } @@ -206,11 +206,11 @@ class ChoiceFormField extends FormField */ protected function initialize() { - if ('input' != $this->node->nodeName && 'select' != $this->node->nodeName) { + if ('input' !== $this->node->nodeName && 'select' !== $this->node->nodeName) { throw new \LogicException(sprintf('A ChoiceFormField can only be created from an input or select tag (%s given).', $this->node->nodeName)); } - if ('input' == $this->node->nodeName && 'checkbox' != strtolower($this->node->getAttribute('type')) && 'radio' != strtolower($this->node->getAttribute('type'))) { + if ('input' === $this->node->nodeName && 'checkbox' !== strtolower($this->node->getAttribute('type')) && 'radio' !== strtolower($this->node->getAttribute('type'))) { throw new \LogicException(sprintf('A ChoiceFormField can only be created from an input tag with a type of checkbox or radio (given type is %s).', $this->node->getAttribute('type'))); } @@ -223,7 +223,7 @@ class ChoiceFormField extends FormField $optionValue = $this->buildOptionValue($this->node); $this->options[] = $optionValue; - if ($this->node->getAttribute('checked')) { + if ($this->node->hasAttribute('checked')) { $this->value = $optionValue['value']; } } else { @@ -236,22 +236,22 @@ class ChoiceFormField extends FormField $found = false; foreach ($this->xpath->query('descendant::option', $this->node) as $option) { - $this->options[] = $this->buildOptionValue($option); + $optionValue = $this->buildOptionValue($option); + $this->options[] = $optionValue; - if ($option->getAttribute('selected')) { + if ($option->hasAttribute('selected')) { $found = true; if ($this->multiple) { - $this->value[] = $option->getAttribute('value'); + $this->value[] = $optionValue['value']; } else { - $this->value = $option->getAttribute('value'); + $this->value = $optionValue['value']; } } } // if no option is selected and if it is a simple select box, take the first option as the value - $option = $this->xpath->query('descendant::option', $this->node)->item(0); - if (!$found && !$this->multiple && $option instanceof \DOMElement) { - $this->value = $option->getAttribute('value'); + if (!$found && !$this->multiple && !empty($this->options)) { + $this->value = $this->options[0]['value']; } } } @@ -269,13 +269,13 @@ class ChoiceFormField extends FormField $defaultValue = (isset($node->nodeValue) && !empty($node->nodeValue)) ? $node->nodeValue : '1'; $option['value'] = $node->hasAttribute('value') ? $node->getAttribute('value') : $defaultValue; - $option['disabled'] = ($node->hasAttribute('disabled') && $node->getAttribute('disabled') == 'disabled'); + $option['disabled'] = $node->hasAttribute('disabled'); return $option; } /** - * Checks whether given vale is in the existing options + * Checks whether given value is in the existing options * * @param string $optionValue * @param array $options diff --git a/src/Symfony/Component/DomCrawler/Field/FileFormField.php b/src/Symfony/Component/DomCrawler/Field/FileFormField.php index 160ddc6e0d..bbdf467994 100644 --- a/src/Symfony/Component/DomCrawler/Field/FileFormField.php +++ b/src/Symfony/Component/DomCrawler/Field/FileFormField.php @@ -99,11 +99,11 @@ class FileFormField extends FormField */ protected function initialize() { - if ('input' != $this->node->nodeName) { + if ('input' !== $this->node->nodeName) { throw new \LogicException(sprintf('A FileFormField can only be created from an input tag (%s given).', $this->node->nodeName)); } - if ('file' != strtolower($this->node->getAttribute('type'))) { + if ('file' !== strtolower($this->node->getAttribute('type'))) { throw new \LogicException(sprintf('A FileFormField can only be created from an input tag with a type of file (given type is %s).', $this->node->getAttribute('type'))); } diff --git a/src/Symfony/Component/DomCrawler/Field/InputFormField.php b/src/Symfony/Component/DomCrawler/Field/InputFormField.php index 00141d84c6..b9bd0a4829 100644 --- a/src/Symfony/Component/DomCrawler/Field/InputFormField.php +++ b/src/Symfony/Component/DomCrawler/Field/InputFormField.php @@ -30,15 +30,15 @@ class InputFormField extends FormField */ protected function initialize() { - if ('input' != $this->node->nodeName && 'button' != $this->node->nodeName) { + if ('input' !== $this->node->nodeName && 'button' !== $this->node->nodeName) { throw new \LogicException(sprintf('An InputFormField can only be created from an input or button tag (%s given).', $this->node->nodeName)); } - if ('checkbox' == strtolower($this->node->getAttribute('type'))) { + if ('checkbox' === strtolower($this->node->getAttribute('type'))) { throw new \LogicException('Checkboxes should be instances of ChoiceFormField.'); } - if ('file' == strtolower($this->node->getAttribute('type'))) { + if ('file' === strtolower($this->node->getAttribute('type'))) { throw new \LogicException('File inputs should be instances of FileFormField.'); } diff --git a/src/Symfony/Component/DomCrawler/Field/TextareaFormField.php b/src/Symfony/Component/DomCrawler/Field/TextareaFormField.php index 242a9d3724..a14e70783b 100644 --- a/src/Symfony/Component/DomCrawler/Field/TextareaFormField.php +++ b/src/Symfony/Component/DomCrawler/Field/TextareaFormField.php @@ -27,7 +27,7 @@ class TextareaFormField extends FormField */ protected function initialize() { - if ('textarea' != $this->node->nodeName) { + if ('textarea' !== $this->node->nodeName) { throw new \LogicException(sprintf('A TextareaFormField can only be created from a textarea tag (%s given).', $this->node->nodeName)); } diff --git a/src/Symfony/Component/DomCrawler/Form.php b/src/Symfony/Component/DomCrawler/Form.php index adce403f07..afee8d9e55 100644 --- a/src/Symfony/Component/DomCrawler/Form.php +++ b/src/Symfony/Component/DomCrawler/Form.php @@ -28,7 +28,7 @@ class Form extends Link implements \ArrayAccess private $button; /** - * @var Field\FormField[] + * @var FormFieldRegistry */ private $fields; @@ -368,7 +368,7 @@ class Form extends Link implements \ArrayAccess protected function setNode(\DOMElement $node) { $this->button = $node; - if ('button' == $node->nodeName || ('input' == $node->nodeName && in_array(strtolower($node->getAttribute('type')), array('submit', 'button', 'image')))) { + if ('button' === $node->nodeName || ('input' === $node->nodeName && in_array(strtolower($node->getAttribute('type')), array('submit', 'button', 'image')))) { if ($node->hasAttribute('form')) { // if the node has the HTML5-compliant 'form' attribute, use it $formId = $node->getAttribute('form'); @@ -385,8 +385,8 @@ class Form extends Link implements \ArrayAccess if (null === $node = $node->parentNode) { throw new \LogicException('The selected node does not have a form ancestor.'); } - } while ('form' != $node->nodeName); - } elseif ('form' != $node->nodeName) { + } while ('form' !== $node->nodeName); + } elseif ('form' !== $node->nodeName) { throw new \LogicException(sprintf('Unable to submit on a "%s" tag.', $node->nodeName)); } diff --git a/src/Symfony/Component/DomCrawler/FormFieldRegistry.php b/src/Symfony/Component/DomCrawler/FormFieldRegistry.php index f37e05025c..150f94dfc6 100644 --- a/src/Symfony/Component/DomCrawler/FormFieldRegistry.php +++ b/src/Symfony/Component/DomCrawler/FormFieldRegistry.php @@ -137,7 +137,7 @@ class FormFieldRegistry /** * Returns the list of field with their value. * - * @return array The list of fields as array((string) Fully qualified name => (mixed) value) + * @return FormField[] The list of fields as array((string) Fully qualified name => (mixed) value) */ public function all() { @@ -196,7 +196,7 @@ class FormFieldRegistry * * @param string $name The name of the field * - * @return array The list of segments + * @return string[] The list of segments * * @throws \InvalidArgumentException when the name is malformed */ diff --git a/src/Symfony/Component/DomCrawler/Link.php b/src/Symfony/Component/DomCrawler/Link.php index 96e9ec3134..9ddf2b2208 100644 --- a/src/Symfony/Component/DomCrawler/Link.php +++ b/src/Symfony/Component/DomCrawler/Link.php @@ -188,7 +188,7 @@ class Link */ protected function setNode(\DOMElement $node) { - if ('a' != $node->nodeName && 'area' != $node->nodeName) { + if ('a' !== $node->nodeName && 'area' !== $node->nodeName) { throw new \LogicException(sprintf('Unable to click on a "%s" tag.', $node->nodeName)); } diff --git a/src/Symfony/Component/DomCrawler/Tests/Field/ChoiceFormFieldTest.php b/src/Symfony/Component/DomCrawler/Tests/Field/ChoiceFormFieldTest.php index f0b086816e..c13c026a7f 100644 --- a/src/Symfony/Component/DomCrawler/Tests/Field/ChoiceFormFieldTest.php +++ b/src/Symfony/Component/DomCrawler/Tests/Field/ChoiceFormFieldTest.php @@ -73,6 +73,11 @@ class ChoiceFormFieldTest extends FormFieldTestCase $field = new ChoiceFormField($node); $this->assertTrue($field->isMultiple(), '->isMultiple() returns true for selects with the multiple attribute'); + + $node = $this->createNode('select', '', array('multiple' => '')); + $field = new ChoiceFormField($node); + + $this->assertTrue($field->isMultiple(), '->isMultiple() returns true for selects with an empty multiple attribute'); } public function testSelects() @@ -107,6 +112,14 @@ class ChoiceFormFieldTest extends FormFieldTestCase } } + public function testSelectWithEmptyBooleanAttribute() + { + $node = $this->createSelectNode(array('foo' => false, 'bar' => true), array(), ''); + $field = new ChoiceFormField($node); + + $this->assertEquals('bar', $field->getValue()); + } + public function testMultipleSelects() { $node = $this->createSelectNode(array('foo' => false, 'bar' => false), array('multiple' => 'multiple')); @@ -166,12 +179,25 @@ class ChoiceFormFieldTest extends FormFieldTestCase } } + public function testRadioButtonsWithEmptyBooleanAttribute() + { + $node = $this->createNode('input', '', array('type' => 'radio', 'name' => 'name', 'value' => 'foo')); + $field = new ChoiceFormField($node); + $node = $this->createNode('input', '', array('type' => 'radio', 'name' => 'name', 'value' => 'bar', 'checked' => '')); + $field->addChoice($node); + + $this->assertTrue($field->hasValue(), '->hasValue() returns true when a radio button is selected'); + $this->assertEquals('bar', $field->getValue(), '->getValue() returns the value attribute of the selected radio button'); + } + public function testRadioButtonIsDisabled() { $node = $this->createNode('input', '', array('type' => 'radio', 'name' => 'name', 'value' => 'foo', 'disabled' => 'disabled')); $field = new ChoiceFormField($node); $node = $this->createNode('input', '', array('type' => 'radio', 'name' => 'name', 'value' => 'bar')); $field->addChoice($node); + $node = $this->createNode('input', '', array('type' => 'radio', 'name' => 'name', 'value' => 'baz', 'disabled' => '')); + $field->addChoice($node); $field->select('foo'); $this->assertEquals('foo', $field->getValue(), '->getValue() returns the value attribute of the selected radio button'); @@ -180,6 +206,10 @@ class ChoiceFormFieldTest extends FormFieldTestCase $field->select('bar'); $this->assertEquals('bar', $field->getValue(), '->getValue() returns the value attribute of the selected radio button'); $this->assertFalse($field->isDisabled()); + + $field->select('baz'); + $this->assertEquals('baz', $field->getValue(), '->getValue() returns the value attribute of the selected radio button'); + $this->assertTrue($field->isDisabled()); } public function testCheckboxes() @@ -225,6 +255,15 @@ class ChoiceFormFieldTest extends FormFieldTestCase } } + public function testCheckboxWithEmptyBooleanAttribute() + { + $node = $this->createNode('input', '', array('type' => 'checkbox', 'name' => 'name', 'value' => 'foo', 'checked' => '')); + $field = new ChoiceFormField($node); + + $this->assertTrue($field->hasValue(), '->hasValue() returns true when the checkbox is checked'); + $this->assertEquals('foo', $field->getValue()); + } + public function testTick() { $node = $this->createSelectNode(array('foo' => false, 'bar' => false)); @@ -280,6 +319,11 @@ class ChoiceFormFieldTest extends FormFieldTestCase { $node = $this->createSelectNodeWithEmptyOption(array('foo' => false, 'bar' => false)); $field = new ChoiceFormField($node); + $this->assertEquals('foo', $field->getValue()); + + $node = $this->createSelectNodeWithEmptyOption(array('foo' => false, 'bar' => true)); + $field = new ChoiceFormField($node); + $this->assertEquals('bar', $field->getValue()); $field->select('foo'); $this->assertEquals('foo', $field->getValue(), '->select() changes the selected option'); } @@ -299,7 +343,7 @@ class ChoiceFormFieldTest extends FormFieldTestCase $this->assertEquals(array('foobar'), $field->getValue(), '->disableValidation() allows to set a value which is not in the selected options.'); } - protected function createSelectNode($options, $attributes = array()) + protected function createSelectNode($options, $attributes = array(), $selectedAttrText = 'selected') { $document = new \DOMDocument(); $node = $document->createElement('select'); @@ -313,7 +357,7 @@ class ChoiceFormFieldTest extends FormFieldTestCase $option = $document->createElement('option', $value); $option->setAttribute('value', $value); if ($selected) { - $option->setAttribute('selected', 'selected'); + $option->setAttribute('selected', $selectedAttrText); } $node->appendChild($option); } diff --git a/src/Symfony/Component/DomCrawler/Tests/FormTest.php b/src/Symfony/Component/DomCrawler/Tests/FormTest.php index df3020cd95..f4a0fb107f 100644 --- a/src/Symfony/Component/DomCrawler/Tests/FormTest.php +++ b/src/Symfony/Component/DomCrawler/Tests/FormTest.php @@ -813,7 +813,6 @@ class FormTest extends \PHPUnit_Framework_TestCase $dom = new \DOMDocument(); $dom->loadHTML(''.$form.''); - $nodes = $dom->getElementsByTagName('input'); $xPath = new \DOMXPath($dom); $nodes = $xPath->query('//input | //button'); @@ -876,5 +875,4 @@ class FormTest extends \PHPUnit_Framework_TestCase $form = new Form($nodes->item(0), 'http://example.com'); $this->assertEquals($form->getPhpValues(), array('example' => '')); } - } diff --git a/src/Symfony/Component/Form/composer.json b/src/Symfony/Component/Form/composer.json index 3de847e97f..56176c234f 100644 --- a/src/Symfony/Component/Form/composer.json +++ b/src/Symfony/Component/Form/composer.json @@ -25,7 +25,9 @@ "require-dev": { "symfony/validator": "~2.2", "symfony/http-foundation": "~2.2", - "symfony/security-csrf": "~2.4" + "symfony/http-kernel": "~2.4", + "symfony/security-csrf": "~2.4", + "doctrine/collections": "~1.0" }, "suggest": { "symfony/validator": "For form validation.", diff --git a/src/Symfony/Component/HttpFoundation/Resources/stubs/SessionHandlerInterface.php b/src/Symfony/Component/HttpFoundation/Resources/stubs/SessionHandlerInterface.php index 24280e38fc..9557135bcf 100644 --- a/src/Symfony/Component/HttpFoundation/Resources/stubs/SessionHandlerInterface.php +++ b/src/Symfony/Component/HttpFoundation/Resources/stubs/SessionHandlerInterface.php @@ -12,6 +12,14 @@ /** * SessionHandlerInterface for PHP < 5.4 * + * The order in which these methods are invoked by PHP are: + * 1. open [session_start] + * 2. read + * 3. gc [optional depending on probability settings: gc_probability / gc_divisor] + * 4. destroy [optional when session_regenerate_id(true) is used] + * 5. write [session_write_close] or destroy [session_destroy] + * 6. close + * * Extensive documentation can be found at php.net, see links: * * @see http://php.net/sessionhandlerinterface @@ -19,6 +27,7 @@ * @see http://php.net/session-set-save-handler * * @author Drak + * @author Tobias Schultze */ interface SessionHandlerInterface { @@ -57,6 +66,9 @@ interface SessionHandlerInterface /** * Writes the session data to the storage. * + * Care, the session ID passed to write() can be different from the one previously + * received in read() when the session ID changed due to session_regenerate_id(). + * * @see http://php.net/sessionhandlerinterface.write * * @param string $sessionId Session ID , see http://php.net/function.session-id diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php index b831383a24..4cdf3a8985 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php @@ -12,7 +12,19 @@ namespace Symfony\Component\HttpFoundation\Session\Storage\Handler; /** - * PdoSessionHandler. + * Session handler using a PDO connection to read and write data. + * + * It works with MySQL, PostgreSQL, Oracle, SQL Server and SQLite and implements + * locking of sessions to prevent loss of data by concurrent access to the same session. + * This means requests for the same session will wait until the other one finished. + * PHPs internal files session handler also works this way. + * + * Attention: Since SQLite does not support row level locks but locks the whole database, + * it means only one session can be accessed at a time. Even different sessions would wait + * for another to finish. So saving session in SQLite should only be considered for + * development or prototypes. + * + * @see http://php.net/sessionhandlerinterface * * @author Fabien Potencier * @author Michael Williams @@ -25,6 +37,11 @@ class PdoSessionHandler implements \SessionHandlerInterface */ private $pdo; + /** + * @var string Database driver + */ + private $driver; + /** * @var string Table name */ @@ -45,39 +62,50 @@ class PdoSessionHandler implements \SessionHandlerInterface */ private $timeCol; + /** + * @var bool Whether a transaction is active + */ + private $inTransaction = false; + + /** + * @var bool Whether gc() has been called + */ + private $gcCalled = false; + /** * Constructor. * * List of available options: - * * db_table: The name of the table [required] + * * db_table: The name of the table [default: sessions] * * db_id_col: The column where to store the session id [default: sess_id] * * db_data_col: The column where to store the session data [default: sess_data] * * db_time_col: The column where to store the timestamp [default: sess_time] * - * @param \PDO $pdo A \PDO instance - * @param array $dbOptions An associative array of DB options + * @param \PDO $pdo A \PDO instance + * @param array $options An associative array of DB options * - * @throws \InvalidArgumentException When "db_table" option is not provided + * @throws \InvalidArgumentException When PDO error mode is not PDO::ERRMODE_EXCEPTION */ - public function __construct(\PDO $pdo, array $dbOptions = array()) + public function __construct(\PDO $pdo, array $options = array()) { - if (!array_key_exists('db_table', $dbOptions)) { - throw new \InvalidArgumentException('You must provide the "db_table" option for a PdoSessionStorage.'); - } if (\PDO::ERRMODE_EXCEPTION !== $pdo->getAttribute(\PDO::ATTR_ERRMODE)) { throw new \InvalidArgumentException(sprintf('"%s" requires PDO error mode attribute be set to throw Exceptions (i.e. $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION))', __CLASS__)); } + $this->pdo = $pdo; - $dbOptions = array_merge(array( + $this->driver = $this->pdo->getAttribute(\PDO::ATTR_DRIVER_NAME); + + $options = array_replace(array( + 'db_table' => 'sessions', 'db_id_col' => 'sess_id', 'db_data_col' => 'sess_data', 'db_time_col' => 'sess_time', - ), $dbOptions); + ), $options); - $this->table = $dbOptions['db_table']; - $this->idCol = $dbOptions['db_id_col']; - $this->dataCol = $dbOptions['db_data_col']; - $this->timeCol = $dbOptions['db_time_col']; + $this->table = $options['db_table']; + $this->idCol = $options['db_id_col']; + $this->dataCol = $options['db_data_col']; + $this->timeCol = $options['db_time_col']; } /** @@ -85,14 +113,56 @@ class PdoSessionHandler implements \SessionHandlerInterface */ public function open($savePath, $sessionName) { + $this->gcCalled = false; + return true; } /** * {@inheritdoc} */ - public function close() + public function read($sessionId) { + $this->beginTransaction(); + + try { + $this->lockSession($sessionId); + + // We need to make sure we do not return session data that is already considered garbage according + // to the session.gc_maxlifetime setting because gc() is called after read() and only sometimes. + $maxlifetime = (int) ini_get('session.gc_maxlifetime'); + + $sql = "SELECT $this->dataCol FROM $this->table WHERE $this->idCol = :id AND $this->timeCol > :time"; + + $stmt = $this->pdo->prepare($sql); + $stmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); + $stmt->bindValue(':time', time() - $maxlifetime, \PDO::PARAM_INT); + $stmt->execute(); + + // We use fetchAll instead of fetchColumn to make sure the DB cursor gets closed + $sessionRows = $stmt->fetchAll(\PDO::FETCH_NUM); + + if ($sessionRows) { + return base64_decode($sessionRows[0][0]); + } + + return ''; + } catch (\PDOException $e) { + $this->rollback(); + + throw $e; + } + } + + /** + * {@inheritdoc} + */ + public function gc($maxlifetime) + { + // We delay gc() to close() so that it is executed outside the transactional and blocking read-write process. + // This way, pruning expired sessions does not block them from being started while the current session is used. + $this->gcCalled = true; + return true; } @@ -109,56 +179,14 @@ class PdoSessionHandler implements \SessionHandlerInterface $stmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); $stmt->execute(); } catch (\PDOException $e) { - throw new \RuntimeException(sprintf('PDOException was thrown when trying to delete a session: %s', $e->getMessage()), 0, $e); + $this->rollback(); + + throw $e; } return true; } - /** - * {@inheritdoc} - */ - public function gc($maxlifetime) - { - // delete the session records that have expired - $sql = "DELETE FROM $this->table WHERE $this->timeCol < :time"; - - try { - $stmt = $this->pdo->prepare($sql); - $stmt->bindValue(':time', time() - $maxlifetime, \PDO::PARAM_INT); - $stmt->execute(); - } catch (\PDOException $e) { - throw new \RuntimeException(sprintf('PDOException was thrown when trying to delete expired sessions: %s', $e->getMessage()), 0, $e); - } - - return true; - } - - /** - * {@inheritdoc} - */ - public function read($sessionId) - { - $sql = "SELECT $this->dataCol FROM $this->table WHERE $this->idCol = :id"; - - try { - $stmt = $this->pdo->prepare($sql); - $stmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); - $stmt->execute(); - - // We use fetchAll instead of fetchColumn to make sure the DB cursor gets closed - $sessionRows = $stmt->fetchAll(\PDO::FETCH_NUM); - - if ($sessionRows) { - return base64_decode($sessionRows[0][0]); - } - - return ''; - } catch (\PDOException $e) { - throw new \RuntimeException(sprintf('PDOException was thrown when trying to read the session data: %s', $e->getMessage()), 0, $e); - } - } - /** * {@inheritdoc} */ @@ -167,8 +195,10 @@ class PdoSessionHandler implements \SessionHandlerInterface // Session data can contain non binary safe characters so we need to encode it. $encoded = base64_encode($data); + // The session ID can be different from the one previously received in read() + // when the session ID changed due to session_regenerate_id(). So we have to + // do an insert or update even if we created a row in read() for locking. // We use a MERGE SQL query when supported by the database. - // Otherwise we have to use a transactional DELETE followed by INSERT to prevent duplicate entries under high concurrency. try { $mergeSql = $this->getMergeSql(); @@ -183,15 +213,18 @@ class PdoSessionHandler implements \SessionHandlerInterface return true; } - $this->pdo->beginTransaction(); - - try { - $deleteStmt = $this->pdo->prepare( - "DELETE FROM $this->table WHERE $this->idCol = :id" - ); - $deleteStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); - $deleteStmt->execute(); + $updateStmt = $this->pdo->prepare( + "UPDATE $this->table SET $this->dataCol = :data, $this->timeCol = :time WHERE $this->idCol = :id" + ); + $updateStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); + $updateStmt->bindParam(':data', $encoded, \PDO::PARAM_STR); + $updateStmt->bindValue(':time', time(), \PDO::PARAM_INT); + $updateStmt->execute(); + // Since we have a lock on the session, this is safe to do. Otherwise it would be prone to + // race conditions in high concurrency. And if it's a regenerated session ID it should be + // unique anyway. + if (!$updateStmt->rowCount()) { $insertStmt = $this->pdo->prepare( "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)" ); @@ -199,20 +232,155 @@ class PdoSessionHandler implements \SessionHandlerInterface $insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR); $insertStmt->bindValue(':time', time(), \PDO::PARAM_INT); $insertStmt->execute(); - - $this->pdo->commit(); - } catch (\PDOException $e) { - $this->pdo->rollback(); - - throw $e; } } catch (\PDOException $e) { - throw new \RuntimeException(sprintf('PDOException was thrown when trying to write the session data: %s', $e->getMessage()), 0, $e); + $this->rollback(); + + throw $e; } return true; } + /** + * {@inheritdoc} + */ + public function close() + { + $this->commit(); + + if ($this->gcCalled) { + $maxlifetime = (int) ini_get('session.gc_maxlifetime'); + + // delete the session records that have expired + $sql = "DELETE FROM $this->table WHERE $this->timeCol <= :time"; + + $stmt = $this->pdo->prepare($sql); + $stmt->bindValue(':time', time() - $maxlifetime, \PDO::PARAM_INT); + $stmt->execute(); + } + + return true; + } + + /** + * Helper method to begin a transaction. + * + * Since SQLite does not support row level locks, we have to acquire a reserved lock + * on the database immediately. Because of https://bugs.php.net/42766 we have to create + * such a transaction manually which also means we cannot use PDO::commit or + * PDO::rollback or PDO::inTransaction for SQLite. + */ + private function beginTransaction() + { + if ($this->inTransaction) { + $this->rollback(); + + throw new \BadMethodCallException( + 'Session handler methods have been invoked in wrong sequence. ' . + 'Expected sequence: open() -> read() -> destroy() / write() -> close()'); + } + + if ('sqlite' === $this->driver) { + $this->pdo->exec('BEGIN IMMEDIATE TRANSACTION'); + } else { + $this->pdo->beginTransaction(); + } + $this->inTransaction = true; + } + + /** + * Helper method to commit a transaction. + */ + private function commit() + { + if ($this->inTransaction) { + try { + // commit read-write transaction which also releases the lock + if ('sqlite' === $this->driver) { + $this->pdo->exec('COMMIT'); + } else { + $this->pdo->commit(); + } + $this->inTransaction = false; + } catch (\PDOException $e) { + $this->rollback(); + + throw $e; + } + } + } + + /** + * Helper method to rollback a transaction. + */ + private function rollback() + { + // We only need to rollback if we are in a transaction. Otherwise the resulting + // error would hide the real problem why rollback was called. We might not be + // in a transaction when two callbacks (e.g. destroy and write) are invoked that + // both fail. + if ($this->inTransaction) { + if ('sqlite' === $this->driver) { + $this->pdo->exec('ROLLBACK'); + } else { + $this->pdo->rollback(); + } + $this->inTransaction = false; + } + } + + /** + * Exclusively locks the row so other concurrent requests on the same session will block. + * + * This prevents loss of data by keeping the data consistent between read() and write(). + * We do not use SELECT FOR UPDATE because it does not lock non-existent rows. And a following + * INSERT when not found can result in a deadlock for one connection. + * + * @param string $sessionId Session ID + */ + private function lockSession($sessionId) + { + switch ($this->driver) { + case 'mysql': + // will also lock the row when actually nothing got updated (id = id) + $sql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . + "ON DUPLICATE KEY UPDATE $this->idCol = $this->idCol"; + break; + case 'oci': + // DUAL is Oracle specific dummy table + $sql = "MERGE INTO $this->table USING DUAL ON ($this->idCol = :id) " . + "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . + "WHEN MATCHED THEN UPDATE SET $this->idCol = $this->idCol"; + break; + case 'sqlsrv': + // MS SQL Server requires MERGE be terminated by semicolon + $sql = "MERGE INTO $this->table USING (SELECT 'x' AS dummy) AS src ON ($this->idCol = :id) " . + "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . + "WHEN MATCHED THEN UPDATE SET $this->idCol = $this->idCol;"; + break; + case 'pgsql': + // obtain an exclusive transaction level advisory lock + $sql = 'SELECT pg_advisory_xact_lock(:lock_id)'; + $stmt = $this->pdo->prepare($sql); + $stmt->bindValue(':lock_id', hexdec(substr($sessionId, 0, 15)), \PDO::PARAM_INT); + $stmt->execute(); + + return; + default: + return; + } + + // We create a DML lock for the session by inserting empty data or updating the row. + // This is safer than an application level advisory lock because it also prevents concurrent modification + // of the session from other parts of the application. + $stmt = $this->pdo->prepare($sql); + $stmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); + $stmt->bindValue(':data', '', \PDO::PARAM_STR); + $stmt->bindValue(':time', time(), \PDO::PARAM_INT); + $stmt->execute(); + } + /** * Returns a merge/upsert (i.e. insert or update) SQL query when supported by the database. * @@ -220,9 +388,7 @@ class PdoSessionHandler implements \SessionHandlerInterface */ private function getMergeSql() { - $driver = $this->pdo->getAttribute(\PDO::ATTR_DRIVER_NAME); - - switch ($driver) { + switch ($this->driver) { case 'mysql': return "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . "ON DUPLICATE KEY UPDATE $this->dataCol = VALUES($this->dataCol), $this->timeCol = VALUES($this->timeCol)"; @@ -230,12 +396,12 @@ class PdoSessionHandler implements \SessionHandlerInterface // DUAL is Oracle specific dummy table return "MERGE INTO $this->table USING DUAL ON ($this->idCol = :id) " . "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . - "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data"; + "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time"; case 'sqlsrv': // MS SQL Server requires MERGE be terminated by semicolon return "MERGE INTO $this->table USING (SELECT 'x' AS dummy) AS src ON ($this->idCol = :id) " . "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . - "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data;"; + "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time;"; case 'sqlite': return "INSERT OR REPLACE INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"; } diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php index e465f39898..109addbcbd 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php @@ -29,74 +29,108 @@ class PdoSessionHandlerTest extends \PHPUnit_Framework_TestCase $this->pdo->exec($sql); } - public function testIncompleteOptions() - { - $this->setExpectedException('InvalidArgumentException'); - $storage = new PdoSessionHandler($this->pdo, array()); - } - + /** + * @expectedException \InvalidArgumentException + */ public function testWrongPdoErrMode() { - $pdo = new \PDO("sqlite::memory:"); - $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_SILENT); - $pdo->exec("CREATE TABLE sessions (sess_id VARCHAR(255) PRIMARY KEY, sess_data TEXT, sess_time INTEGER)"); + $this->pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_SILENT); - $this->setExpectedException('InvalidArgumentException'); - $storage = new PdoSessionHandler($pdo, array('db_table' => 'sessions')); + $storage = new PdoSessionHandler($this->pdo); } - public function testWrongTableOptionsWrite() + /** + * @expectedException \RuntimeException + */ + public function testInexistentTable() { - $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'bad_name')); - $this->setExpectedException('RuntimeException'); - $storage->write('foo', 'bar'); + $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'inexistent_table')); + $storage->open('', 'sid'); + $storage->read('id'); + $storage->write('id', 'data'); + $storage->close(); } - public function testWrongTableOptionsRead() + public function testReadWriteRead() { - $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'bad_name')); - $this->setExpectedException('RuntimeException'); - $storage->read('foo', 'bar'); + $storage = new PdoSessionHandler($this->pdo); + $storage->open('', 'sid'); + $this->assertSame('', $storage->read('id'), 'New session returns empty string data'); + $storage->write('id', 'data'); + $storage->close(); + + $storage->open('', 'sid'); + $this->assertSame('data', $storage->read('id'), 'Written value can be read back correctly'); + $storage->close(); } - public function testWriteRead() + /** + * Simulates session_regenerate_id(true) which will require an INSERT or UPDATE (replace) + */ + public function testWriteDifferentSessionIdThanRead() { - $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'sessions')); - $storage->write('foo', 'bar'); - $this->assertEquals('bar', $storage->read('foo'), 'written value can be read back correctly'); + $storage = new PdoSessionHandler($this->pdo); + $storage->open('', 'sid'); + $storage->read('id'); + $storage->destroy('id'); + $storage->write('new_id', 'data_of_new_session_id'); + $storage->close(); + + $storage->open('', 'sid'); + $this->assertSame('data_of_new_session_id', $storage->read('new_id'), 'Data of regenerated session id is available'); + $storage->close(); } - public function testMultipleInstances() + /** + * @expectedException \BadMethodCallException + */ + public function testWrongUsage() { - $storage1 = new PdoSessionHandler($this->pdo, array('db_table' => 'sessions')); - $storage1->write('foo', 'bar'); - - $storage2 = new PdoSessionHandler($this->pdo, array('db_table' => 'sessions')); - $this->assertEquals('bar', $storage2->read('foo'), 'values persist between instances'); + $storage = new PdoSessionHandler($this->pdo); + $storage->open('', 'sid'); + $storage->read('id'); + $storage->read('id'); } public function testSessionDestroy() { - $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'sessions')); - $storage->write('foo', 'bar'); - $this->assertCount(1, $this->pdo->query('SELECT * FROM sessions')->fetchAll()); + $storage = new PdoSessionHandler($this->pdo); - $storage->destroy('foo'); + $storage->open('', 'sid'); + $storage->read('id'); + $storage->write('id', 'data'); + $storage->close(); + $this->assertEquals(1, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn()); - $this->assertCount(0, $this->pdo->query('SELECT * FROM sessions')->fetchAll()); + $storage->open('', 'sid'); + $storage->read('id'); + $storage->destroy('id'); + $storage->close(); + $this->assertEquals(0, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn()); + + $storage->open('', 'sid'); + $this->assertSame('', $storage->read('id'), 'Destroyed session returns empty string'); + $storage->close(); } public function testSessionGC() { - $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'sessions')); + $previousLifeTime = ini_set('session.gc_maxlifetime', 0); + $storage = new PdoSessionHandler($this->pdo); - $storage->write('foo', 'bar'); - $storage->write('baz', 'bar'); + $storage->open('', 'sid'); + $storage->read('id'); + $storage->write('id', 'data'); + $storage->close(); + $this->assertEquals(1, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn()); - $this->assertCount(2, $this->pdo->query('SELECT * FROM sessions')->fetchAll()); + $storage->open('', 'sid'); + $this->assertSame('', $storage->read('id'), 'Session already considered garbage, so not returning data even if it is not pruned yet'); + $storage->gc(0); + $storage->close(); + $this->assertEquals(0, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn()); - $storage->gc(-1); - $this->assertCount(0, $this->pdo->query('SELECT * FROM sessions')->fetchAll()); + ini_set('session.gc_maxlifetime', $previousLifeTime); } public function testGetConnection() diff --git a/src/Symfony/Component/HttpKernel/Kernel.php b/src/Symfony/Component/HttpKernel/Kernel.php index 7685d08dfc..15552a7c6b 100644 --- a/src/Symfony/Component/HttpKernel/Kernel.php +++ b/src/Symfony/Component/HttpKernel/Kernel.php @@ -32,6 +32,7 @@ use Symfony\Component\Config\Loader\LoaderResolver; use Symfony\Component\Config\Loader\DelegatingLoader; use Symfony\Component\Config\ConfigCache; use Symfony\Component\ClassLoader\ClassCollectionLoader; +use Symfony\Component\Filesystem\Filesystem; /** * The Kernel is the heart of the Symfony system. @@ -714,9 +715,43 @@ abstract class Kernel implements KernelInterface, TerminableInterface $content = static::stripComments($content); } + $content = $this->removeAbsolutePathsFromContainer($content); + $cache->write($content, $container->getResources()); } + /** + * Converts absolute paths to relative ones in the dumped container. + */ + private function removeAbsolutePathsFromContainer($content) + { + if (!class_exists('Symfony\Component\Filesystem\Filesystem')) { + return $content; + } + + // find the "real" root dir (by finding the composer.json file) + $rootDir = $this->getRootDir(); + $previous = $rootDir; + while (!file_exists($rootDir.'/composer.json')) { + if ($previous === $rootDir = realpath($rootDir.'/..')) { + // unable to detect the project root, give up + return $content; + } + + $previous = $rootDir; + } + + $rootDir = rtrim($rootDir, '/'); + $cacheDir = $this->getCacheDir(); + $filesystem = new Filesystem(); + + return preg_replace_callback("{'([^']*)(".preg_quote($rootDir)."[^']*)'}", function ($match) use ($filesystem, $cacheDir) { + $prefix = isset($match[1]) && $match[1] ? "'$match[1]'.__DIR__.'/" : "__DIR__.'/"; + + return $prefix.rtrim($filesystem->makePathRelative($match[2], $cacheDir), '/')."'"; + }, $content); + } + /** * Returns a loader for the container. * diff --git a/src/Symfony/Component/HttpKernel/Tests/Fixtures/DumpedContainers/app/cache/dev/withAbsolutePaths.php b/src/Symfony/Component/HttpKernel/Tests/Fixtures/DumpedContainers/app/cache/dev/withAbsolutePaths.php new file mode 100644 index 0000000000..54e0dedc4b --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/Fixtures/DumpedContainers/app/cache/dev/withAbsolutePaths.php @@ -0,0 +1,7 @@ +'ROOT_DIR/app/cache/dev/foo' +'ROOT_DIR/app/cache/foo' +'ROOT_DIR/foo/bar.php' + +'/some/where/else/foo' + +'file:ROOT_DIR/app/cache/dev/profiler' diff --git a/src/Symfony/Component/HttpKernel/Tests/Fixtures/DumpedContainers/app/cache/dev/withoutAbsolutePaths.php b/src/Symfony/Component/HttpKernel/Tests/Fixtures/DumpedContainers/app/cache/dev/withoutAbsolutePaths.php new file mode 100644 index 0000000000..09ce8b5683 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/Fixtures/DumpedContainers/app/cache/dev/withoutAbsolutePaths.php @@ -0,0 +1,7 @@ +__DIR__.'/foo' +__DIR__.'/../foo' +__DIR__.'/../../../foo/bar.php' + +'/some/where/else/foo' + +'file:'.__DIR__.'/profiler' diff --git a/src/Symfony/Component/HttpKernel/Tests/Fixtures/DumpedContainers/composer.json b/src/Symfony/Component/HttpKernel/Tests/Fixtures/DumpedContainers/composer.json new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/Fixtures/DumpedContainers/composer.json @@ -0,0 +1 @@ +{} diff --git a/src/Symfony/Component/HttpKernel/Tests/Fixtures/KernelForTest.php b/src/Symfony/Component/HttpKernel/Tests/Fixtures/KernelForTest.php index 5fd61bbc7e..c7396dd8a7 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fixtures/KernelForTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fixtures/KernelForTest.php @@ -16,6 +16,11 @@ use Symfony\Component\Config\Loader\LoaderInterface; class KernelForTest extends Kernel { + public function setRootDir($dir) + { + $this->rootDir = $dir; + } + public function getBundleMap() { return $this->bundleMap; diff --git a/src/Symfony/Component/HttpKernel/Tests/KernelTest.php b/src/Symfony/Component/HttpKernel/Tests/KernelTest.php index bc9876df3b..30e201fee5 100644 --- a/src/Symfony/Component/HttpKernel/Tests/KernelTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/KernelTest.php @@ -722,6 +722,34 @@ EOF; $kernel->terminate(Request::create('/'), new Response()); } + public function testRemoveAbsolutePathsFromContainer() + { + $kernel = new KernelForTest('dev', true); + $kernel->setRootDir($symfonyRootDir = __DIR__.'/Fixtures/DumpedContainers/app'); + + $content = file_get_contents($symfonyRootDir.'/cache/dev/withAbsolutePaths.php'); + $content = str_replace('ROOT_DIR', __DIR__.'/Fixtures/DumpedContainers', $content); + + $m = new \ReflectionMethod($kernel, 'removeAbsolutePathsFromContainer'); + $m->setAccessible(true); + $content = $m->invoke($kernel, $content); + $this->assertEquals(file_get_contents($symfonyRootDir.'/cache/dev/withoutAbsolutePaths.php'), $content); + } + + public function testRemoveAbsolutePathsFromContainerGiveUpWhenComposerJsonPathNotGuessable() + { + $kernel = new KernelForTest('dev', true); + $kernel->setRootDir($symfonyRootDir = sys_get_temp_dir()); + + $content = file_get_contents(__DIR__.'/Fixtures/DumpedContainers/app/cache/dev/withAbsolutePaths.php'); + $content = str_replace('ROOT_DIR', __DIR__.'/Fixtures/DumpedContainers', $content); + + $m = new \ReflectionMethod($kernel, 'removeAbsolutePathsFromContainer'); + $m->setAccessible(true); + $newContent = $m->invoke($kernel, $content); + $this->assertEquals($newContent, $content); + } + /** * Returns a mock for the BundleInterface * diff --git a/src/Symfony/Component/HttpKernel/composer.json b/src/Symfony/Component/HttpKernel/composer.json index dd01364985..5ddee871a3 100644 --- a/src/Symfony/Component/HttpKernel/composer.json +++ b/src/Symfony/Component/HttpKernel/composer.json @@ -29,6 +29,7 @@ "symfony/console": "~2.2", "symfony/dependency-injection": "~2.0", "symfony/finder": "~2.0", + "symfony/filesystem": "~2.4", "symfony/process": "~2.0", "symfony/routing": "~2.2", "symfony/stopwatch": "~2.2", @@ -40,7 +41,8 @@ "symfony/config": "", "symfony/console": "", "symfony/dependency-injection": "", - "symfony/finder": "" + "symfony/finder": "", + "symfony/filesystem": "" }, "autoload": { "psr-0": { "Symfony\\Component\\HttpKernel\\": "" }