Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modifying length of file can break VCR integration #76

Open
lkysow opened this issue Nov 7, 2014 · 3 comments
Open

Modifying length of file can break VCR integration #76

lkysow opened this issue Nov 7, 2014 · 3 comments
Labels
Bug Something isn't working

Comments

@lkysow
Copy link

lkysow commented Nov 7, 2014

Tl;dr: Changing the length of your file by adding/removing code can break PHP VCR.

How The Code Works

PHP VCR modifies files when they're being read from the filesystem through the use of PHP's php_user_filter (http://php.net/manual/en/class.php-user-filter.php).

When a file is read from the filesystem, it goes through this function in PHP VCR

<?php
public function filter($in, $out, &$consumed, $closing)
{
        while ($bucket = stream_bucket_make_writeable($in)) {
            $bucket->data = $this->transformCode($bucket->data);
            $consumed += $bucket->datalen;
            stream_bucket_append($out, $bucket);
        }

        return PSFS_PASS_ON;
    }
}

which calls transformCode($code) in each code transformer

<?php

class CurlCodeTransform extends AbstractCodeTransform
{

    private static $patterns = array(
        '/(?<!::|->)\\\?curl_init\s*\(/i' => '\VCR\LibraryHooks\CurlHook::curl_init('
        ...
    );

    protected function transformCode($code)
    {
        return preg_replace(array_keys(self::$patterns), array_values(self::$patterns), $code);
    }
}

The transformCode($code) function does a preg_replace and replaces the original code with code that calls PHP VCR.

The Problem

You may have noticed that the filter function operates in a while loop calling stream_bucket_make_writeable. The bug is that this function can split up the stream, i.e. the file that is being read.

If it splits it between a line of code that we're trying to search and replace, then the preg_replace will fail and we won't modify this file.

Example

The class we're rewriting:

<?php
class TransformMe {
    public function test() {
        $curl = curl_init();
    }
}

Ideally, this will be re-written to

<?php
class TransformMe {
    public function test() {
        $curl = \VCR\LibraryHooks\CurlHook::curl_init();
    }
}

However if the file is split like this:

<?php
class TransformMe {
    public function test() {
        $curl = curl_
init();
    }
}

Then the transformation will never happen.

This was fun to debug :)

Solutions?

I'd say the best thing to do would be to read the entire file at once, and then do the search and replace, rather than relying on PHP's streams and bucketing.

@adri
Copy link
Contributor

adri commented Nov 7, 2014

Great analysis @lkysow. Thank you very much!
I agree, it should read the entire file before replacing data. I'll implement it asap.

adri added a commit that referenced this issue Nov 7, 2014
When reading a bucket it might happen that the search string
is split up between two buckets and then cannot be found.

The (easy) solution is to read in the whole file and then
search and replace.

Fixes #76.
adri added a commit that referenced this issue Nov 7, 2014
When reading a bucket it might happen that the search string
is split up between two buckets and then cannot be found.

The (easy) solution is to read in the whole file and then
search and replace.

Fixes #76.
@adri
Copy link
Contributor

adri commented Nov 7, 2014

Before merging #77 I would like to write a regression test. Do you have an example where it failed?
Otherwise I'll make something up ;-)

@lkysow
Copy link
Author

lkysow commented Nov 7, 2014

@adri thanks for the fast response! I've managed to reproduce it with this file: https://gist.github.com/lkysow/518afbebcaaefd486024

@adri adri added the Bug Something isn't working label Nov 18, 2014
adri added a commit that referenced this issue Dec 19, 2014
When reading a bucket it might happen that the search string
is split up between two buckets and then cannot be found.

The (easy) solution is to read in the whole file and then
search and replace.

Fixes #76.
adri added a commit that referenced this issue Feb 27, 2015
When reading a bucket it might happen that the search string
is split up between two buckets and then cannot be found.

The (easy) solution is to read in the whole file and then
search and replace.

Fixes #76.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants