fix fopen headers for php7

[garemoko]

With PHP 7 and the current version of the code, for some reason the headers get included in the stream context but are not used by fopen. I think it has something to do with array push inserting a reference to each individual header array, rather than a copy. I'm not sure that's how array push works or why that would cause the issue though.

The specific error you get (from fopen, it doesn't get as far as hitting the LRS) without this patch is that no content type is specified.

Fatal error: Uncaught ErrorException: fopen(): Content-type not specified assuming application/x-www-form-urlencoded

See http://stackoverflow.com/questions/39085235/fopen-no-authorization-header-with-php-7?noredirect=1#comment65518910_39085235 for a related issue that used similar code.

Whatever the reason, this patch fixes the issue.

As a side note, this is the only change I found was required for POST Statements to work under PHP7. I haven't tested any of the other calls. This change isn't in https://github.com/RusticiSoftware/TinCanPHP/pull/47 - I'm not sure why this problem didn't come up there.

[garemoko]

@brianjmiller ready for review.

[brianjmiller]

I don't really understand this and it seems like either a fundamental flaw in how PHP7 handles arrays or something else is going on, I'd like to know what because it could have larger ramifications for how we are using arrays elsewhere in the lib.

[brianjmiller]

Can you post your re-production code, the one that sends to requestb.in?

[garemoko]

This is scaled down from what's in TinCanPHP, but it shows the issue:

<?php
  $url = "http://requestb.in/waoevowa";

  $http = array(
    'max_redirects' => 0,
    'request_fulluri' => 1,
    'ignore_errors' => true,
    'method' => 'GET',
    'header' => array()
  );

  // array_push($http['header'], "foo: bar");
  $http['header'] = array("foo: bar");

  $context = stream_context_create(array( 'http' => $http ));

  var_dump(stream_context_get_options($context));echo('<hr/>');
  $fp = fopen($url, 'rb', false, $context);
  if (! $fp) {
    throw new \Exception('Request failed: $php_errormsg');
  }
  $metadata = stream_get_meta_data($fp);
  $content  = stream_get_contents($fp);
  $responseCode = (int)explode(' ', $metadata['wrapper_data'][0])[1];

  fclose($fp);

If you uncomment array_push($http['header'], "foo: bar"); you'll see no foo header, regardless of whether or not you comment out $http['header'] = array("foo: bar");.

You'll see from the var_dump that stream_context_create is correctly including the header by reference (note the ampersand). fopen is then not using it. Remove the array push and the array is included directly and the header works.

array(1) {
  ["http"]=>
  array(5) {
    ["max_redirects"]=>
    int(0)
    ["request_fulluri"]=>
    int(1)
    ["ignore_errors"]=>
    bool(true)
    ["method"]=>
    string(3) "GET"
    ["header"]=>
    &array(1) {
      [0]=>
      string(8) "foo: bar"
    }
  }
}

[brianjmiller]

With your code I get a "Foo" header correctly (see 2nd header above).

Using

PHP 7.0.8 (cli) (built: Jun 26 2016 12:30:44) ( NTS ) Copyright (c) 1997-2016 The PHP Group Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies with Zend OPcache v7.0.8, Copyright (c) 1999-2016, by Zend Technologies with Xdebug v2.4.0RC3, Copyright (c) 2002-2015, by Derick Rethans

I've also run an altered version of the code in the sample/ directory (I adjusted RemoteLRS' sendRequest method to be public) and ran it after using the setHeaders method to store a "map" of headers and it works as expected as well.

Can you re-test, cause I can't reproduce this issue?

[garemoko]

Re-tested. Any chance you can try PHP 7.0.0? This is the version shipped with MAMP, and the version my client is using.

[brianjmiller]

I don't know of an easy way for me to set that up, I'm using PHP versions from:

http://php-osx.liip.ch/

But reviewing the PHP change log suggests that this is the issue:

https://bugs.php.net/bug.php?id=71245

Which was fixed in PHP 7.0.3. See:

http://php.net/ChangeLog-7.php

I'm not entirely clear on why the newer code you have would work, but I didn't look at the actual change set.

[garemoko]

Yes, that looks like the issue. The modified code fixes the issue because the array_push is causing the headers property to contain the array by reference whereas without it, the array is included directly. I'm not sure why array_push causes that though.

So what's the best next steps here? Do we merge this or do we say that PHP 7.0.0-7.0.2 are unsupported?

[garemoko]

You can get php 7.0.0 here: http://php.net/releases/

[garemoko]

I upgraded to PHP 7.0.8 and that did resolve the issue. Interestingly, the array is no longer passed by reference, so it looks like an additional fix to the one you linked.

[brianjmiller]

What I don't understand is that ultimately you have to put that array in as a reference, it seems more likely that it was dissociating that array reference or skipping it all together. But whatever, it is fixed :-).

My preference is to just say you need something newer than 7.0.2, I don't think there is a reason to backpedal code for the first major release in the first few patch releases. This is partially why I'm not in a super hurry to get 7 supported, people running 7 should be very cautious at the moment.