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

Fix checking of $HTTP_RAW_POST_DATA for PHP7 #8823

Closed
wants to merge 1 commit into from
Closed

Fix checking of $HTTP_RAW_POST_DATA for PHP7 #8823

wants to merge 1 commit into from

Conversation

jonnybarnes
Copy link

Due to populating of raw post data being deprecated in PHP5.6 We
needed to set the ini value always_populate_raw_post_data to
-1 in order to avoid deprecation warnings and the like. However
this feature has been removed in PHP7. The knock on to this is
that regardless of wether the php.ini contains the line
always_populate_raw_post_data=-1 the ini_get function evaluates
to 0/false. Thus we only need to check for this ini setting being
-1 for PHP5.6

I haven’t added any tests for this yet.

Due to populating of raw post data being deprecated in PHP5.6 We
needed to set the ini value `always_populate_raw_post_data` to
`-1` in order to avoid deprecation warnings and the like. However
this feature has been removed in PHP7. The knock on to this is
that regardless of wether the `php.ini` contains the line
`always_populate_raw_post_data=-1` the `ini_get` function evaluates
to 0/false. Thus we only need to check for this ini setting being
-1 for PHP5.6
@sgiehl
Copy link
Member

sgiehl commented Sep 18, 2015

I'm not into the PHP release plans, but if there might be a PHP 5.7 still having that "feature" your check would fail, so guess it would be better to check for >= 5.6 and < 7

@jonnybarnes
Copy link
Author

@sgiehl as far as I am aware there are no plans for a PHP5.7 release: https://wiki.php.net/todo, as can be seen from the strikethrough there were plans for 6.0 but that’s a whole other story :/

@mattab
Copy link
Member

mattab commented Sep 20, 2015

Hello, @tsteur made the change in another PR already. will be fixed in https://github.com/piwik/piwik/pull/8706/files

@mattab mattab closed this Sep 20, 2015
@mattab mattab added the duplicate For issues that already existed in our issue tracker and were reported previously. label Sep 20, 2015
@jonnybarnes
Copy link
Author

Excellent, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate For issues that already existed in our issue tracker and were reported previously.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants