Commit Graph

37253 Commits

Author SHA1 Message Date
Nicolas Grekas
df0dba6cdf bug #27668 [Lock] use 'r+' for fopen (fixes issue on Solaris) (fritzmg)
This PR was squashed before being merged into the 3.4 branch (closes #27668).

Discussion
----------

[Lock] use 'r+' for fopen (fixes issue on Solaris)

| Q             | A
| ------------- | ---
| Branch?       | 3.4 (also applicable to _LockHandler_ in 2.8 and 3.3)
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes [1]
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

We discovered a curious case on a specific hosting environment: the `FlockStorage` (or `LockHandler` in previous Symfony versions) could never get a file lock on **existing files**. So if you run a script for the first time and the lock file did not exist yet, the `FlockStorage` could get a lock on that file just fine. However on the second and subsequent runs, `FlockStorage` could _never_ get a file lock anymore. You can follow the discussion [here](https://github.com/contao/core-bundle/issues/1551) (if you speak German).

We have been using this little script to confirm the issue on the hosting environment:
```php
$fileName = __DIR__ . '/file.lock';

if (!$handle = @fopen($fileName, 'r')) {
    $handle = fopen($fileName, 'x');
}

if (!$handle) {
    echo "Could not open $fileName\n";
    exit;
}

if (flock($handle, LOCK_EX | LOCK_NB)) {
    echo "Got a lock on $fileName\n";
    flock($handle, LOCK_UN | LOCK_NB);
} else {
    echo "Could not get a lock on $fileName\n";
}
```
Whenever `file.lock` already existed prior to running the script, a lock could not be made.

After contacting the hosting provider's support on this they confirmed the issue and told us they are using **Solaris** instead of a Linux environment. And this is supposedly why it does not work. Instead you have to use `'r+'` instead of `'r'` for `fopen`.

I was able to confirm that changing from `'r'` to `'r+'` fixes the issue. However I am wondering who's actually at fault here. Is it Solaris? PHP? The compiled PHP version under Solaris? The hosting provider's operating system configuration?

### System information
```
uname -a
SunOS vlek 5.11 11.3 i86pc i386 i86pc Solaris
```
```
phpinfo:
SunOS localhost 5.10 Generic_150401-49 i86pc
```
```
php -v
PHP 7.2.5 (cli) (built: May  4 2018 12:57:43) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
```

### Footnotes

[1] Previously one failed test on the first run and two failing tests on the second run with the proposed changes:
```
1) Symfony\Component\Lock\Tests\Store\FlockStoreTest::testSaveSanitizeName
Symfony\Component\Lock\Exception\LockStorageException: fopen(C:\Users\[…]\AppData\Local\Temp/sf.-php-echo-hello-word-.Sz2dDSf.lock)
: failed to open stream: Permission denied
```
```
2) Symfony\Component\Lock\Tests\Store\FlockStoreTest::testSaveWithDifferentKeysOnSameResources
Symfony\Component\Lock\Exception\LockStorageException: fopen(C:\Users\[…]\AppData\Local\Temp/sf.Symfony-Component-Lock-Tests-Store-
AbstractStoreTest-testSaveWithDifferentKeysOnSameResources5b2b5f00872538.64807920.2u9bH+a.lock): failed to open stream: Permission denied
```
The latter failed both on the first run and on the second run.

After the [proposed changes](https://github.com/symfony/symfony/pull/27668#discussion_r197054198) from @nicolas-grekas everything works fine 👍

Commits
-------

9c9ae7d9c9 [Lock] use 'r+' for fopen (fixes issue on Solaris)
2018-06-21 13:12:36 +02:00
Fritz Michael Gschwantner
9c9ae7d9c9 [Lock] use 'r+' for fopen (fixes issue on Solaris) 2018-06-21 13:12:28 +02:00
Nicolas Grekas
7090495250 Merge branch '2.8' into 3.4
* 2.8:
  [HttpKernel] fix test compat with PHP 5.3
  fix file lock on SunOS
  change `evaluate()` docblock return type from string to mixed
  Set serialize_precision explicitly to avoid fancy float rounding
2018-06-21 13:10:19 +02:00
Nicolas Grekas
f63579d360 bug #27669 [Filesystem] fix file lock on SunOS (fritzmg)
This PR was merged into the 2.8 branch.

Discussion
----------

[Filesystem] fix file lock on SunOS

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

See https://github.com/symfony/symfony/pull/27668

Commits
-------

7adb641d7c fix file lock on SunOS
2018-06-21 13:08:28 +02:00
Nicolas Grekas
749410a224 [HttpKernel] fix test compat with PHP 5.3 2018-06-21 13:07:36 +02:00
Andrew Clark
dca9ff529a [Serializer] Updates DocBlock to a mixed param type 2018-06-21 12:57:13 +02:00
Nicolas Grekas
d652222ef6 bug #27662 [HttpKernel] fix handling of nested Error instances (xabbuh)
This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] fix handling of nested Error instances

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27613
| License       | MIT
| Doc PR        |

Commits
-------

a6696d03b1 fix handling of nested Error instances
2018-06-21 12:51:43 +02:00
Christian Flothmann
a6696d03b1 fix handling of nested Error instances 2018-06-21 12:30:28 +02:00
fritzmg
7adb641d7c fix file lock on SunOS 2018-06-21 11:24:14 +02:00
Valentin
1e10475d88 Ignore keepQueryParams attribute when generating route redirect. 2018-06-21 00:41:56 +03:00
Nicolas Grekas
ff0de67519 [Cache] more granular handling of exceptions in AbstractTrait::clear() 2018-06-20 22:30:04 +02:00
Fabien Potencier
00b6f53a55 minor #27657 [ExpressionLanguage] change evaluate() docblock return type from string to mixed (jspee)
This PR was submitted for the 4.1 branch but it was merged into the 2.8 branch instead (closes #27657).

Discussion
----------

[ExpressionLanguage] change `evaluate()` docblock return type from string to mixed

Issue: https://github.com/symfony/symfony/issues/27652

Commits
-------

2d26a556fd change `evaluate()` docblock return type from string to mixed
2018-06-20 19:39:13 +02:00
jspee
2d26a556fd change evaluate() docblock return type from string to mixed 2018-06-20 19:39:05 +02:00
Nicolas Grekas
e5059a0d7e minor #27637 Set serialize_precision explicitly to avoid fancy float rounding (Majkl578)
This PR was merged into the 2.8 branch.

Discussion
----------

Set serialize_precision explicitly to avoid fancy float rounding

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | none
| License       | MIT
| Doc PR        | -

This is fixing some of the test failures I was seeing locally due to increased `serialize_precision` INI setting:
```
2) Symfony\Component\HttpFoundation\Tests\JsonResponseTest::testConstructorWithSimpleTypes
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'0.1'
+'0.10000000000000001'

/www/symfony/symfony/src/Symfony/Component/HttpFoundation/Tests/JsonResponseTest.php:46

3) Symfony\Component\HttpFoundation\Tests\JsonResponseTest::testStaticCreateWithSimpleTypes
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'0.1'
+'0.10000000000000001'
```

The test assertions should not depend on externally configured PHP configuration.

Also default value for this option was changed multiple times: http://php.net/manual/en/ini.core.php#ini.serialize-precision
For compatibility reasons (with PHP <7.x) `-1` can't be used.

https://3v4l.org/HBNsT

HHVM doesn't seem to support this though, how to handle this?

Commits
-------

b5ee7c3ccd Set serialize_precision explicitly to avoid fancy float rounding
2018-06-20 14:43:58 +02:00
Nicolas Grekas
389fa4dc23 bug #27651 [Messenger] Fixed MessengerPass::guessHandledClasses return type (massimilianobraglia)
This PR was merged into the 4.1 branch.

Discussion
----------

[Messenger] Fixed MessengerPass::guessHandledClasses return type

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   |  no
| Fixed tickets | #27633
| License       | MIT
| Doc PR        | ø

#27633 should have been merged to 4.1.

Commits
-------

d6b6e9658c [Messenger] Fixed MessengerPass::guessHandledClasses return type
2018-06-20 13:15:41 +02:00
Nicolas Grekas
20147fce57 Merge branch '4.0' into 4.1
* 4.0:
  [Cache] Improve resiliency when calling doFetch() in AbstractTrait
  Fixing GlobResource when inside phar archive
2018-06-20 13:15:17 +02:00
Nicolas Grekas
641ecbb74b Merge branch '3.4' into 4.0
* 3.4:
  [Cache] Improve resiliency when calling doFetch() in AbstractTrait
  Fixing GlobResource when inside phar archive
2018-06-20 13:15:04 +02:00
Nicolas Grekas
b61c8fcb77 bug #26845 [Config] Fixing GlobResource when inside phar archive (vworldat)
This PR was merged into the 3.4 branch.

Discussion
----------

[Config] Fixing GlobResource when inside phar archive

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes if old broken behavior counts as stable
| Deprecations? | no
| Tests pass?   | no tests yet
| Fixed tickets |
| License       | MIT
| Doc PR        | N/A

When packaging an Sf4 application as a PHAR archive using globs at various locations (`Kernel`, `services.yaml`) most glob files are not found because the `glob()` PHP method [does not support PHAR streams](https://stackoverflow.com/questions/8203188/unexpected-problems-with-php-phar).

Using the regex fallback instead when operating inside PHAR archives fixes the behavior for me.

## Examples:

`src/Kernel.php::configureContainer()`:

```php
$loader->load($confDir.'/{services}'.self::CONFIG_EXTS, 'glob');
```

Expected behavior: `config/services.yaml` inside PHAR archive is found and parsed
Actual behavior: the file will not be loaded

`config/services.yaml` (hard-coded in Kernel without using glob pattern)

```yaml
    App\:
        resource: '../src/*'
        exclude: '../src/{Entity,Migrations,Tests,Kernel.php}'
```

Expected behavior: service classes in `src/` will be found and auto-wired
Actual behavior: services are not auto-wired because the class files are not found

Commits
-------

e336ebeecf Fixing GlobResource when inside phar archive
2018-06-20 13:09:19 +02:00
Nicolas Grekas
b7ccf10de7 [Cache] Improve resiliency when calling doFetch() in AbstractTrait 2018-06-20 13:05:06 +02:00
Nicolas Grekas
a278c79c6e Merge branch '4.0' into 4.1
* 4.0:
  [Cache] Fix exception handling in AbstractTrait::clear()
  Revert "minor #27649 [Cache] fix Memcached tests (nicolas-grekas)"
2018-06-20 11:26:39 +02:00
Nicolas Grekas
6d26d9a5ae Merge branch '3.4' into 4.0
* 3.4:
  [Cache] Fix exception handling in AbstractTrait::clear()
  Revert "minor #27649 [Cache] fix Memcached tests (nicolas-grekas)"
2018-06-20 11:26:33 +02:00
Massimiliano Braglia
d6b6e9658c [Messenger] Fixed MessengerPass::guessHandledClasses return type 2018-06-20 12:25:27 +03:00
Nicolas Grekas
bef15cebca [Cache] Fix exception handling in AbstractTrait::clear() 2018-06-20 11:24:39 +02:00
Nicolas Grekas
bb644c1df8 Revert "minor #27649 [Cache] fix Memcached tests (nicolas-grekas)"
This reverts commit dc323f084c, reversing
changes made to 917b07a5c6.
2018-06-20 10:56:56 +02:00
Nicolas Grekas
f68e19468d Merge branch '4.0' into 4.1
* 4.0:
  [Cache] fix Memcached tests
2018-06-20 09:27:12 +02:00
Nicolas Grekas
9b64e8bedf Merge branch '3.4' into 4.0
* 3.4:
  [Cache] fix Memcached tests
2018-06-20 09:27:05 +02:00
Nicolas Grekas
dc323f084c minor #27649 [Cache] fix Memcached tests (nicolas-grekas)
This PR was merged into the 3.4 branch.

Discussion
----------

[Cache] fix Memcached tests

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Clearing a memcached pool works only when versioning is enabled. Dunno why this has not be caught before, but it's making tests fail now.

Commits
-------

09ec9e7cce [Cache] fix Memcached tests
2018-06-20 09:26:50 +02:00
Nicolas Grekas
09ec9e7cce [Cache] fix Memcached tests 2018-06-20 09:02:04 +02:00
Nicolas Grekas
34acfe4bfb Merge branch '4.0' into 4.1
* 4.0:
  [HttpKernel] fix PHP 5.4 compat
  Fix surrogate not using original request
  [Finder] Update RealIteratorTestCase
  [Routing] remove unneeded dev dep on doctrine/common
  [minor] SCA
  [Validator] Remove BOM in some xlf files
  Fix #27378: Error when rendering a DateIntervalType form with exactly 0 weeks
  [HttpKernel] fix session tracking in surrogate master requests
2018-06-19 23:38:16 +02:00
Nicolas Grekas
b85f70e3bc Merge branch '3.4' into 4.0
* 3.4:
  [HttpKernel] fix PHP 5.4 compat
  Fix surrogate not using original request
  [Finder] Update RealIteratorTestCase
  [Routing] remove unneeded dev dep on doctrine/common
  [minor] SCA
  [Validator] Remove BOM in some xlf files
  Fix #27378: Error when rendering a DateIntervalType form with exactly 0 weeks
  [HttpKernel] fix session tracking in surrogate master requests
2018-06-19 22:54:48 +02:00
Nicolas Grekas
917b07a5c6 Merge branch '2.8' into 3.4
* 2.8:
  [HttpKernel] fix PHP 5.4 compat
  Fix surrogate not using original request
  [Finder] Update RealIteratorTestCase
  [Routing] remove unneeded dev dep on doctrine/common
  [Validator] Remove BOM in some xlf files
2018-06-19 22:52:10 +02:00
Nicolas Grekas
0f2b752138 [HttpKernel] fix PHP 5.4 compat 2018-06-19 22:37:28 +02:00
Michael Moravec
b5ee7c3ccd
Set serialize_precision explicitly to avoid fancy float rounding 2018-06-19 16:38:49 +02:00
David Herrmann
e336ebeecf Fixing GlobResource when inside phar archive
When packaging an Sf4 application as a PHAR archive using globs at various locations (`Kernel`, `services.yaml`) most glob files are not found because the `glob()` PHP method [does not support PHAR streams](https://stackoverflow.com/questions/8203188/unexpected-problems-with-php-phar).

Using the regex fallback instead when operating inside PHAR archives fixes the behavior for me.
2018-06-19 16:02:58 +02:00
Nicolas Grekas
c4d972ac20 bug #27382 [Form] Fix error when rendering a DateIntervalType form with exactly 0 weeks (krixon)
This PR was merged into the 3.4 branch.

Discussion
----------

[Form] Fix error when rendering a DateIntervalType form with exactly 0 weeks

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27378
| License       | MIT

Fixes the issue described in #27378

Commits
-------

dae704ad2f Fix #27378: Error when rendering a DateIntervalType form with exactly 0 weeks
2018-06-19 15:17:38 +02:00
Nicolas Grekas
3fc2200134 bug #27309 Fix surrogate not using original request (Toflar)
This PR was submitted for the 3.4 branch but it was squashed and merged into the 2.8 branch instead (closes #27309).

Discussion
----------

Fix surrogate not using original request

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Warning: This might need some close attention. It took me hours to wrap my head around that issue :-)

So the problem is that `HttpCache::forward()` (or essentially any part in your application) can modify the `$request` that is passed to the `HttpCache::handle()` and the surrogate can never access the original request using `HttpCache::getRequest()` anymore.

Example:

* Main request (GET `/foobar`)
* It's not in the cache, so `HttpCache::forward()` modifies `REMOTE_ADDR` to `127.0.0.1` and adds the `X-Forwarded-For` header.
* The request is sent to the application and any e.g. `kernel.request` listener might modify the `$request` further.
* Now the `/foobar` route returns `text/html` that contains some `<esi src="=/fragment_path"` tag.
* `HttpCache` has an instance of `SurrogateInterface` so (in our case `Esi`) will be asked to `process()` and then later on `handle()` the `/fragment_path` request. For that, `Esi` (or in fact `AbstractSurrogate` uses the following line to create a subrequest and pass it on to the application again:

```php
$subRequest = Request::create($uri, Request::METHOD_GET, array(), $cache->getRequest()->cookies->all(), array(), $cache->getRequest()->server->all());
```

What you can see here, is that it uses `$cache->getRequest()`. And here follows the problem:
We did not duplicate (clone) the original request so essentially `$cache->getRequest()` is a reference to the current request that `HttpKernel::forward()` modified and probably any other part of the application did so too. So for example the original `REMOTE_ADDR` (client IP) got lost.

What we should do instead is duplicate the original request so the surrogates can actually behave like a real reverse proxy such as Varnish would by keeping all the original request attributes.

Commits
-------

ab86f43d78 Fix surrogate not using original request
2018-06-19 15:13:23 +02:00
Yanick Witschi
ab86f43d78 Fix surrogate not using original request 2018-06-19 15:13:16 +02:00
Nicolas Grekas
18026dcc83 bug #27467 [HttpKernel] fix session tracking in surrogate master requests (nicolas-grekas)
This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] fix session tracking in surrogate master requests

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Spotted while looking at ESI fragments resolved by`HttpCache`: right now when the master request starts the session, fragments are not cacheable anymore, even when they do not use the session.

Commits
-------

146e01cb44 [HttpKernel] fix session tracking in surrogate master requests
2018-06-19 13:58:43 +02:00
Nicolas Grekas
9f1d1d82fb minor #27508 [Finder] Update RealIteratorTestCase (flip111)
This PR was submitted for the master branch but it was squashed and merged into the 2.8 branch instead (closes #27508).

Discussion
----------

[Finder] Update RealIteratorTestCase

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27480
| License       | MIT
| Doc PR        | n/a

Makes the entire test directory empty instead of trying to delete particular files and directories. The old method failed when trying to remove a directory which was not empty.

Commits
-------

7d0ebd41ab [Finder] Update RealIteratorTestCase
2018-06-19 13:07:17 +02:00
flip111
7d0ebd41ab [Finder] Update RealIteratorTestCase 2018-06-19 13:04:56 +02:00
Nicolas Grekas
ec6b941738 [Routing] remove unneeded dev dep on doctrine/common 2018-06-19 11:52:17 +02:00
Nicolas Grekas
1f439e5845 minor #27623 [minor] SCA (kalessil)
This PR was squashed before being merged into the 3.4 branch (closes #27623).

Discussion
----------

[minor] SCA

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

New findings: language level, greedy regex, array_column usages

Commits
-------

5922507dc5 [minor] SCA
2018-06-19 11:31:47 +02:00
Vladimir Reznichenko
5922507dc5 [minor] SCA 2018-06-19 11:31:41 +02:00
Nicolas Grekas
ee2f76e929 bug #27632 [HttpFoundation] Ensure RedisSessionHandler::updateTimestamp returns a boolean (MatTheCat)
This PR was merged into the 4.1 branch.

Discussion
----------

[HttpFoundation] Ensure RedisSessionHandler::updateTimestamp returns a boolean

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

Since v1.1.0 predis doesn't return the result of `EXPIRE` as a boolean, thus breaking `updateTimestamp` implementation.

Maybe it's worth mentioning error messages were useless here:

- Symfony: `User Warning: session_write_close(): Failed to write session data with Symfony\Component\HttpFoundation\Session\Storage\Handler\RedisSessionHandler handler`
- PHP: `session_write_close(): Session callback expects true/false return value`

Commits
-------

079b944077 Ensure updateTimestamp returns a boolean
2018-06-19 11:15:38 +02:00
Nicolas Grekas
ea53fb01ad bug #27630 [Validator][Form] Remove BOM in some xlf files (gautierderuette)
This PR was submitted for the 3.4 branch but it was merged into the 2.8 branch instead (closes #27630).

Discussion
----------

[Validator][Form] Remove BOM in some xlf files

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   |
| Fixed tickets | #...
| License       | MIT
| Doc PR        | symfony/symfony-docs#...

I removed first blank space from some xml files

It caused this error during cache:clear
[ERROR 4] Start tag expected, '<' not found (in n/a - line 1, column 1)

Commits
-------

0bc53d66c0 [Validator] Remove BOM in some xlf files
2018-06-19 10:02:23 +02:00
Gautier Deuette
0bc53d66c0 [Validator] Remove BOM in some xlf files 2018-06-19 10:02:14 +02:00
MatTheCat
079b944077 Ensure updateTimestamp returns a boolean 2018-06-18 15:48:40 +02:00
karl.rixon
dae704ad2f Fix #27378: Error when rendering a DateIntervalType form with exactly 0 weeks 2018-06-18 12:57:58 +01:00
Robin Chalas
15c0d48277 Merge branch '4.0' into 4.1
* 4.0:
  Fix merge
  [HttpKernel] Fix resetting DumpDataCollector::$isCollected
  [Framework][Workflow] Added support for interfaces
2018-06-18 11:31:19 +02:00
Robin Chalas
57fc15bde6 Merge branch '3.4' into 4.0
* 3.4:
  Fix merge
  [HttpKernel] Fix resetting DumpDataCollector::$isCollected
  [Framework][Workflow] Added support for interfaces
2018-06-18 11:30:06 +02:00