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

Removed HAVE_MB_STRING code (PHP 5.4-incompatible) #19

Closed
wants to merge 1 commit into from

Conversation

jani
Copy link

@jani jani commented Jan 22, 2013

No description provided.

@ghost
Copy link

ghost commented Apr 9, 2013

Mhhh, i have compiled suhosin with PHP 5.4 without this changes and it compiles without a problem. Do we need to remove this?

@jani
Copy link
Author

jani commented Apr 9, 2013

It seems unlikely that your code will work when it references functions that are no longer part of PHP (HAVE_MB_STRING).

See here for the background for this patch:

#17

@jani jani closed this Apr 9, 2013
@jani jani reopened this Apr 9, 2013
@ghost
Copy link

ghost commented Apr 9, 2013

Ah, sorry. Missed this discussion. :-/

I'm using the PHP 5.4 package from dotdeb.org and PHP loads suhosin without the described "Unable to load dynamic library" problem in the linked issue.

@szepeviktor
Copy link

built OK on Debian Wheezy PHP 5.4.4-14+deb7u1

@shakaran
Copy link

shakaran commented Sep 5, 2013

Hi @jani, I cherry pick your commit in my branch. It solves perfect the problem during POST uploads with mb encoding in PHP 5.4 but I am not sure if we have to remove al references to mb_string like your commit.

For example Apache raises this errors without your fix:

/usr/bin/php: symbol lookup error: /usr/local/lib/php/extensions/no-debug-non-zts-20100525/suhosin.so: 
undefined symbol: php_mb_encoding_translation

I test this is in PHP 5.4.19 (used currently with CPanel). In PHP 5.4.19 source code tree php_mb_encoding_translation appears in /ext/mbstring/mbstring.c:102 (https://github.com/php/php-src/blob/PHP-5.4.19/ext/mbstring/mbstring.c#L102). Teorically the function exist in php native source code but it doesn't appear for suhosin.so like symbol (I know that cPanel have a custom version of PHP, and it is probably that the function was removed). But your patch solve this fatal error because it removes the need of use that function.

I would be nice if this get merged in main master project branch. Meanwhile I will keep my own fork with your pull request and others PR useful. Thanks for your contribution, this could help a lot people.

@stefanesser
Copy link
Collaborator

Should be no problem with current 0.9.35-DEV

@shakaran
Copy link

Thanks a lot @stefanesser!! Could you use a git tag for 0.9.35-DEV in repo when you release the next version? It would be nice could use the git tag for easy checkouts

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.

4 participants