Skip to content

Fixed bug #74600 and #74738 #2570

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

Merged
merged 2 commits into from
Jun 12, 2017
Merged

Fixed bug #74600 and #74738 #2570

merged 2 commits into from
Jun 12, 2017

Conversation

manuelm
Copy link
Contributor

@manuelm manuelm commented Jun 8, 2017

Make sure the hash entry is an array.

The origin fix broke support for HOST/PATH ini sections. Only the beginning of the string has to match. Revert this check but use zend_binary_strncasecmp instead of strncasecmp.

@manuelm manuelm changed the base branch from master to PHP-7.0 June 8, 2017 23:24
Make sure the hash entry is an array.

The origin fix broke support for HOST/PATH ini sections. Only the
beginning of the string has to match. Revert this check but use
zend_binary_strncasecmp instead of strncasecmp.
@laruence
Copy link
Member

laruence commented Jun 9, 2017

what kind of usage does the origin fix break? could you make a example

@manuelm
Copy link
Contributor Author

manuelm commented Jun 9, 2017

It broke support for HOST/PATH ini sections. See http://php.net/manual/en/ini.sections.php

e.g.

# cat a.ini
memory_limit=10M
[PATH=/should/not/match]
memory_limit=20M

# 7.0.20
# ./sapi/cli/php -c a.ini -i | grep memory_limit
memory_limit => 20M => 20M

# 7.0.19
# php7.0 -c a.ini -i | grep memory_limit
memory_limit => 10M => 10M

To better understand the bug #74600 the ini file can also be rewritten as:

memory_limit = 10M
[PATH=memory_limit]
a = b

I can add tests later as well. Most important was to fix this issue as it broke all customer ini settings.

@manuelm manuelm changed the title Fixed bug #74600 Fixed bug #74600 and #74738 Jun 10, 2017
@php-pulls php-pulls merged commit 91f129e into php:PHP-7.0 Jun 12, 2017
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

Successfully merging this pull request may close these issues.

3 participants