This PR was merged into the 2.3 branch.
Discussion
----------
[HttpFoundation] fix PDO session handler under high concurrency
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #8448 and http://trac.symfony-project.org/ticket/4777 (which was never really fixed as you can see here)
| License | MIT
- The first commit fixes PDO session handler under high concurrency.
- The second commit uses MERGE SQL for MS SQL Server. Tested with http://sqlfiddle.com/#!6/66b6d/14
- The third commit uses INSERT OR REPLACE for sqlite session handler http://sqlfiddle.com/#!7/e6707/3
What I find rather bad with the class design is that it depends on the table definition, but it's not part of the class. Also it doesn't make use of open() and close() which could be used to make the database connection lazy instead of having is open all the time when not needed. Doctrine also only lazy connects, but we use PDO directly here.
Furthermore, the session handlers should not throw exceptions, from what I read, but return false when an error occurs. This is not followed in this class. Maybe @drak knows how php session management behaves when the session handlers return false?
Commits
-------
5c08e29 [HttpFoundation] use insert or replace for sqlite session handler
05ea19a [HttpFoundation] use MERGE SQL for MS SQL Server session storage
e58d7cf [HttpFoundation] fix PDO session handler under high concurrency
This PR was merged into the 2.3 branch.
Discussion
----------
[Profiler] Prevent throwing fatal errors when searching timestamps or invalid dates
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
Commits
-------
eea9d24 [Profiler] Prevent throwing fatal errors when searching timestamps or invalid dates
This PR was merged into the 2.3 branch.
Discussion
----------
[Templating] PhpEngine should propagate charset to its helpers
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #10666
| License | MIT
Commits
-------
ed9e48b [Templating] PhpEngine should propagate charset to its helpers
This PR was merged into the 2.3 branch.
Discussion
----------
[BrowserKit] Fix#10641 : BrowserKit is broken when using ip as host
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #10641
| License | MIT
As documented in http://php.net/manual/en/function.preg-replace.php, we have to use the `$` notation for replacing the backreference
>When working with a replacement pattern where a backreference is immediately followed by another number (i.e.: placing a literal number immediately after a matched pattern), you cannot use the familiar \\1 notation for your backreference. \\11, for example, would confuse preg_replace() since it does not know whether you want the \\1 backreference followed by a literal 1, or the \\11 backreference followed by nothing. In this case the solution is to use \${1}1. This creates an isolated $1 backreference, leaving the 1 as a literal.
Commits
-------
e946da3 [BrowserKit] Fix#10641 : BrowserKit is broken when using ip as host
It is done for two reasons:
* consistency - we use real exception objects in most of the code
* latest phpunit does not like the way we were creating mocks for exceptions (it could be also fixed by letting phpunit to call the original constructor)
This PR was merged into the 2.3 branch.
Discussion
----------
Fixes URL validator to accept single part urls
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #3207#6817
| License | MIT
| Doc PR | N/A
This PR replaces #6817 and has been rebased for 2.3, making a change to the regex to be simpler for single dot matches. (changing `([\pL\pN\pS-\.])+([\.]{0,1}[\pL]+[\.]{0,1})` to `([\pL\pN\pS-\.])+(\.?[\pL]+\.?)`)
Commits
-------
91e226e Fixes URL validator to accept single part urls
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3] Use `Filesystem::chmod` instead of `chmod` when dumping file
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
This adds consistency as discussed in ca5eea5c19 (commitcomment-5804089)
Commits
-------
c2ffefd Use `Filesystem::chmod` instead of `chmod` when dumping file
YamlFileLoader used the resource name as the current directory during import, which can cause a failed import when using a relative path. Using the $path variable output from the locator is consistent with other loaders and fixes the bug.
If the response contents has been sent before an error occurs, PHP
triggers the warning "Cannot modify header information - headers already sent"
This change ensure that the error message is echoed, while it's impossible
to change the HTTP status code and headers.
This PR was squashed before being merged into the 2.3 branch (closes#10410).
Discussion
----------
[Form] Fix "Array was modified outside object" in ResizeFormListener.
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #10405
| License | MIT
| Doc PR |
This is the second pull request for this issue. The history of this is as follows:
Original Fix was added Feb 11th under Pull Request #10232.
Users began complaining of Doctrine ArrayCollection not being updated in forms.
Revert was added Feb 15th under Pull Request #10269.
Issue #10405 was opened on 7th Mar for the original bug.
Pull Request #10269 has a failing test that illustrates users concerns.
I have added failing tests to this pull request to illustrate the problems described in #10405.
All tests now pass, and all forms of $data are now supported, including arrays, DoctrineCollection, ArrayObject, and other IteratorAggregates.
__Details as follows:__
The onSubmit() method of the ResizeFormListener class is assuming the data is an array, and calling unset directly inside a foreach. This works fine in most scenarios, but if data is an instance of IteratorAggregate that is iterating over $data by reference, it breaks with the following error:
Symfony\Component\Form\Extension\Core\EventListener\ResizeFormListener::onSubmit(): ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in ./vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php line 142
This is because the foreach loop is using an Iterator in the background, but the ResizeFormListener has unset the underlying data directly, causing the value that the Iterators internal pointer is pointing to to change.
The Iterator provided by IteratorAggregate may or may not have the ability to modify it's underlying data. So it is not possible to rely on the Iterator to modify the data. Instead, it is simpler to avoid modifying $data at all while we are iterating over it. So instead, we simply iterate over $data once to determine the keys we need to delete, and store them. Then we iterate over the keys and delete them from $data.
Commits
-------
aa63fae [Form] Fix "Array was modified outside object" in ResizeFormListener.
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Validator] Minor fix in IBAN validator
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #10481, #10489
| License | MIT
Added more values to unit tests of IBAN validator to make clear it doesn't accept lower case letters.
> Permitted IBAN characters are the digits 0 to 9 and the 26 upper-case Latin alphabetic characters A to Z.
http://en.wikipedia.org/wiki/International_Bank_Account_Number
Also made little adjustment to code which meant to validate lowercase letters but actually was useless.
Commits
-------
3eeb306 Fix IBAN validator
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Process] Remove unreachable code + avoid skipping tests in sigchild environment
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
As mentioned by @Tobion in https://github.com/symfony/symfony/pull/10480#issuecomment-38002910, I removed the dead code. I also fixed/updated the test suite on PHP compiled with `--enable-sigchild`.
Commits
-------
d52dd32 [Process] Remove unreachable code + avoid skipping tests in sigchild environment
This PR was merged into the 2.3 branch.
Discussion
----------
[Process] fix some typos and refactor some code
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
Commits
-------
b422613 [Process] fix some typos and refactor some code
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Process] Fix escaping on Windows
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
Windows escaping is broken since the last merges.
After digging more on Windows escaping, I realised some things:
- We forbid environment variable expansion by escaping `%APPDATA%` to `^%"APPDATA"^%`
- We explicitly ask for variable expansion at runtime (running the command line with the [`/V:ON`](https://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/Process/Process.php#L235) flag). Running a command containing `!APPDATA!` will be escaped and expanded (our previous rule is easily overriden)
- On platform that are not windows, we use strong escaping that prevents any variable expansion (`$PATH` will be escaped to `'$PATH'` that is not interpreted as the current PATH)
We have three possibilities:
- Keep this behavior as this.
- Prefer a consistent API and use a strong escaping strategy everywhere, but it would result in a BC break (see #8975).
- Allow environment variable expansion and escape `%APPDATA%` to `"%APPDATA%"`
Any thoughts about this ?
Commits
-------
0f65f90 [Process] Fix escaping on Windows
This PR was merged into the 2.3 branch.
Discussion
----------
[Process] Fixed fatal errors in getOutput and getErrorOutput when process was not started
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #10022
| License | MIT
This PR replaces #9452 and address the latest changes.
Side note : I've not updated `getExitCode`, `getExitCodeText` and `isSuccessful` as they were explicitly tested to return null in case the process is not started or terminated. I think it would be a BC break to do that.
Commits
-------
449fe01 [Process] Trow exceptions in case a Process method is supposed to be called after termination
0ae6858 [Process] fixed fatal errors in getOutput and getErrorOutput when process was not started
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Process] Fix quoted arguments escaping
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
This PR replaces #8972
Commits
-------
de681cb [Process] Add tests on ProcessUtils::escapeArgument
85fb495 [Process] Fix: Arguments including space and quote are not correctly escaped (win)
This PR was merged into the 2.3 branch.
Discussion
----------
[DomCrawler] Fixed incorrect value name conversion in getPhpValues() and getPhpFiles()
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #6908
| License | MIT
This PR replaces #10193
Commits
-------
89c599e [DomCrawler] Add tests for recursive cases of getPhpValues() and getPhpFiles()
e961f57 [DomCrawler] Fixed incorrect value name conversion in getPhpValues() and getPhpFiles()
This PR was merged into the 2.3 branch.
Discussion
----------
[Process] Fixed data in pipe being truncated if not read before process termination
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #9409
| License | MIT
| Doc PR | N/A
This is a repeat of the botched pull request #9630.
Commits
-------
7e51913 Fixed data in pipe being truncated if not read before process termination
This PR was merged into the 2.3 branch.
Discussion
----------
[Process] Fix process status in TTY mode
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
When running a process with TTY mode, status in automatically set to `terminated`
once it's started.
It's wrong for two reasons :
- The status of the process is not yet terminated.
- The exitcode value is never caught
Commits
-------
51c70f8 [Process] Fix process status in TTY mode
When running a process with TTY mode, status in automatically set to `terminated`
once it's started.
It's wrong for two reasons :
- The status of the process is not yet terminated.
- The exitcode value is never caught