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

Does PHP-VCR work with stream_context_create? #96

Closed
tylercollier opened this issue Jan 26, 2015 · 8 comments
Closed

Does PHP-VCR work with stream_context_create? #96

tylercollier opened this issue Jan 26, 2015 · 8 comments

Comments

@tylercollier
Copy link

I'm using the expert-sender-api library. The request in the cassette that ends up recorded is almost blank:

    request:
        method: GET
        url: [URL HERE]
        headers:
            Host: [HOST HERE]
            Content-Length: '0'

There should be a request body, and the content-length should not be zero (it's also odd it's a string, no?). Also, the method there is GET, but it should be POST. If I disable PHP-VCR, and perform a live web service call, the request goes out as I expect and then I get a proper response. So something is not right or fully handled by PHP-VCR here.

If I enabled PHP-VCR and let it send the blank request, it captures the response. As you might expect, it's a response to the blank request, but in my case, the response contains 9 headers and an XML body that I'd expect. So that part works.

The expert-sender-api library's query() method here makes use of PHP's stream_context_create I'm wondering if that has anything to do with it. I suppose not, because that's part of HTTP wrapper, which PHP-VCR explicitly states it supports.

If I manually create the cassette request, there's still another problem. Weirdly (this is directly from the php manual):

When using the HTTP wrapper, $http_response_header will be populated with the HTTP response headers. $http_response_header will be created in the local scope.

The fatal error I get with PHP-VCR is:

Undefined variable: http_response_header

That variable is from line 32. Is it possible for PHP-VCR to handle this? I know essentially how PHP-VCR works, and I can't imagine it dealing with that. Searching the source code for http_response_header yields nothing.

@tylercollier
Copy link
Author

Regarding the "there's still another problem". I just ran a different test with PHP-VCR off and not cassette loaded. I received a response using file_get_contents, but PHPUnit still choked, saying Undefined variable: http_response_header. I'm looking more into this mysterious http_response_header thing in PHP, and I'll report back.

@adri
Copy link
Contributor

adri commented Jan 26, 2015

Thank you very much. Indeed the stream wrapper interception is only tested with GET requests. Header support was added quite recently. The response headers seem to be indeed not recorded. Definitely some work and tests to be done here :-/ thank you for pointing this out.

This should be fixed. I would be very happy to merge a pull request as I'm again limited on time.

@adri
Copy link
Contributor

adri commented Jan 26, 2015

The $http_response_headers could be set by php-vcr, if that's just an ordinary global variable. To be honest I've never used the stream wrappers/contexts this way and wasn't aware that this variable is set.

@tylercollier
Copy link
Author

$http_response_header (note singular) is not global, it's magically brought into existence in the local scope as mentioned in the doc I linked. It's voodoo!

I will work on the stream wrapper with POSTs and submit a pull request.

@tylercollier
Copy link
Author

I've been looking into this. I think the problem I found originally was because the server I was connecting to is sending an intermediate HTTP/1.1 100 Continue header. This stackoverflow answer suggests that we change HttpUtil.php's parseResponse method to look like at the top:

list( $rawHeader, $rawBody ) = explode( "\r\n\r\n", $response , 2);
if (strpos($header," 100 Continue") !== false) {
    list($rawHeader, $rawBody) = explode( "\r\n\r\n", $response , 2);
}

Unfortunately, I don't know how to test this in the tests. Doing the above actually throws away the HTTP/1.1 100 Continue header; it doesn't store it in the cassette. I'm not sure if this is robust enough; the cassette doesn't capture the 'whole story'. I'm thinking that's good enough, or at least better than it is now. The alternative is to capture the initial HTTP/1.1 100 Continue as the header, and then everything after would count as the body. It looks pretty gross in the YAML, but we could also modify the cassette format to account for such scenarios.

@adri, I'd love your in-depth PHP knowledge on how to handle the $http_response_header situation. As I noted from the linked documentation, it's a local variable that's magically brought into existence by PHP when using the stream wrappers. How can PHP-VCR invoke this behavior? My googling got me nowhere!

@tylercollier
Copy link
Author

For anyone who is interested in the $http_response_header issue, a bug was filed on 2014-07-13 at https://bugs.php.net/bug.php?id=63897. I wrote some additional comments there. If you want it to get attention, please vote on that page.

@adri
Copy link
Contributor

adri commented Feb 19, 2015

This is a hard one. I'm just brainstorming a bit...

It is possible to overwrite $http_response_header with a call to a function defined by php-vcr, much like the other CodeTransforms.There is a high chance for risking false positives though since $http_response_header could be just used as a variable somewhere.

There are projects like PHP-Parser which can be used to parse PHP source code. We could implement a check if some stream wrapper function is called and $http_response_header is referenced in the local scope. Probably this has a big performance impact and should be only done if the PHP file includes $http_response_header at all.

I think it could be solved but it requires work and lots of testing.

@JeroenVanOort
Copy link
Member

As this hasn't been worked on in years, I think now is a good time to close this issue. If anyone wants to work on this, or any other objective, in the future, please open an new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants