PhpStorage: past, present and future

There are (to the best of my knowledge) four phpstorage issues open at this moment and to me it's not particularly clear which ones are potential duplicates. So I decided to write up what I can see -- perhaps it will help others as well. If everyone else sees clearly, please close this issue.

So. The current phpstorage subsystem was born out of the need to store the container on disk when an action in the UI forces a rebuild. The original issues are #1668892: Enable secure compiled information on disk and #1675260: Implement PHP reading/writing secured against "leaky" script. Very early we realized Twig has a similar need.

There are various implementations for various hosts:

1. FileStorage is a very basic solution writing raw php files with very little protection: the directory it writes into gets a .htaccess file stopping Apache from executing PHP in this directory. However, as Drupal includes these files if somehow a rogue file gets written, it will lead to an arbitrary code execution anyways. This is not recommended for usage. Bug: this "avoid!" is not documented at all. Possibly we should make this abstract, even to make sure it's well avoided.
2. MTimeProtectedStorage is the default. It writes to a subdirectory of sites/default/files by default, base directory can be changed in settings.php. Also, it uses some complicated file operations to make sure a badly written (test, example etc) script writing files using user input for a filename can't write a file Drupal would include later. It does not even intend to protect against anything else. This threat is very real and have been the direct cause of several high profile breaches.
3. FileReadOnlyStorage is a sort of anti-solution: it does not allow writing. The working theory was that massive hosts won't allow any actions that force a rebuild on production and would deploy everything anyways from git so there's no need for Drupal to actually write a thing. This is a very secure solution.

There are currently a number of bugs / problems however.

The complicated magic in MTimeProtectedStorage has a serious problem with the Container: if it takes more than a second to rebuild the Container and there are multiple requests in parallel trying to do a rebuild (which will obviously slow the system to the point where it takes more than a second...) it will never finish. To emphasize:

1. every request starts a rebuild - major bug
2. the rebuild never finishing - critical bug.

There's another problem: if you run multiple webfrontends and actually want to solve the original problem, you do not have a good solution: putting code on a slow shared filesystem sucks. There might not be a shared filesystem even: Drupal uses stream wrappers, it is a well-solvable problem to write files to some cluster which is very fast to serve them to a browser -- but that can't compete with the speed of local disk/OS cache and also you would have some real fun chicken-egg problems trying to have your stream wrapper in place before the container is loaded. If you put writable code on a local filesystem and do a rebuild then your dumped PHP files will get out of sync.

There are a number of solutions offered in the issue queue which solve one or the other problem -- neither of them was opened to solve both so it's an accident and typically not tested whether they do solve both. I will write my understanding below:

1. Do not write the container to php storage; instead store the definitions in an array and put that array in cache. #2497243: Replace Symfony container with a Drupal one, stored in cache. Avoids No1 and solves No2 for the container because cache is distributed. But it offers no solution for Twig at all. As compiling any given Twig file is likely faster than a container rebuild the problem would be much more rare. It's not clear whether it would go away entirely or just become much harder to reproduce. And that's just No1 for Twig, there's no solution for Twig and No2.
2. Add the value of central, atomic counter to the directory name in MTimeProtectedStorage #2301163: Create a phpstorage backend that works with a local disk but coordinates across multiple webheads. This is offered as a solution to No2 and with a bit of configuration to point to a local directory it solves it. However, I believe it doesn't solve No1.
3. Write the PHP code as a string to cache and make sure it's opcode cached when read. #2513326: Performance: create a PHP storage backend directly backed by cache. I believe using this would avoid No1 and solve No2 too. The drawback to this solution is it's a hack. However, it works with PHP 5.5, 5.6 and 7.0 and when 7.1 comes we can investigate what, if any changes are necessary. It only relies on phar:// files being opcode cached so it's somewhat unlikely to break. Have not investigated HHVM compatibility.
4. Just fix MTimeProtectedStorage: #2527478: Resolve infinite stampede in mtime protected PHP storage Fixes No1 solid but does not do anything for No2.

Finally, FileReadOnlyStorage is of dubious use right now as we do not have a way to compile everything. This can't be done in contrib because we need a central signal (event, likely) for everything using the storage to dump it all. We can't rely on just twig and container using phpstorage, there are already contrib using it. Also, one possible future contrib would be the sane and safe resurrection of contemplate ie the ability to provide templates from the UI -- this actually was one of the reasons we went with Twig if I remember correctly. This is problem No3.

Next steps:

1. Decide whether we want to keep MTimeProtectedStorage. Using #2513326: Performance: create a PHP storage backend directly backed by cache would allow dropping it. Note: I am not advocating for or against keeping it.
2. If what I wrote so far is correct and we keep it then #2527478: Resolve infinite stampede in mtime protected PHP storage is necessary: nothing else actually solves No1.
3. Decide how do we want to solve No2. The candidates are, in their current state #2513326: Performance: create a PHP storage backend directly backed by cache and #2301163: Create a phpstorage backend that works with a local disk but coordinates across multiple webheads.
4. File or find an issue for No3. An extremely minimal solution is at #2548273: Add a phpstorage populate event
5. Decide if we want to reccommend avoiding FileStorage and if yes, how harshly: just document it or actually make the class abstract.