PHP 7 Support

[ahmeier]

I tried to compile this for php 7, but it does not seem to work. compiles with php 7, but the scrypt.so file is not used and does not show up in phpinfo.php, even though it is in the extension directory.

[DomBlack]

I just added PHP 7 to the build matrix on Travis. It looks like one of the PHP macros has changed; https://travis-ci.org/DomBlack/php-scrypt/jobs/92673120

I'll check it out when I get time (See #40)

[kingkorf]

See #41 also

[cbj4074]

Firstly, thank you for creating this, @DomBlack . I deeply appreciate the work and expertise that you've put into this library.

What can I do to help you prepare this for PHP 7?

[ghost]

And what about windows binary id like to test on windows

[redthor]

I'm just looking at running a php7 test.

If I re-apply @kingkorf's changes to php-scrypt.c in master (i.e. line 199, rather than 181) I get warning: initialization from incompatible pointer type when I make

Not sure if this is significant.

[cbj4074]

@redthor Does the extension compile successfully, despite the warning? Is it usable thereafter?

[redthor]

It compiles but I ran into a memory alloc issue when testing. I can give more info later. Cheers

[cbj4074]

@redthor Thanks!

This extension is the only thing keeping me from upgrading to PHP 7. My application relies on it and I lack the specialized knowledge required to massage the source into shape for PHP 7. Otherwise, I'd jump on it myself. :/

But at this point, I'm considering learning whatever I need to learn to make it work. Please let me know if I can facilitate your efforts in any way, and I'll do my best!

[redthor]

I've now changed the code to this:

diff --git a/php_scrypt.c b/php_scrypt.c
index e4c8e0e..82e884a 100644
--- a/php_scrypt.c
+++ b/php_scrypt.c
@@ -199,10 +199,10 @@ PHP_FUNCTION(scrypt)
         php_hash_bin2hex(hex, buf, keyLength);
         efree(buf);
         hex[keyLength*2] = '\0';
-        RETURN_STRINGL(hex, keyLength * 2, 0);
+        RETURN_STRINGL(hex, keyLength * 2);
     } else {
         buf[keyLength] = '\0';
-        RETURN_STRINGL((char *)buf, keyLength, 0);
+        RETURN_STRINGL((char *)buf, keyLength);
     }
 }
 /* }}} */

i.e. just removed the last 0 and it appears to be happy. More testing todo

[redthor]

@maki-chan has already done this in https://github.com/DomBlack/php-scrypt/pull/45

[joshuataylor]

The following works on Ubuntu (and probably other distros as well) to on PHP7 for testing:

git clone https://github.com/DomBlack/php-scrypt.git
cd php-scrypt
wget https://patch-diff.githubusercontent.com/raw/DomBlack/php-scrypt/pull/45.patch
patch -p1 < 45.patch
phpize
./configure --enable-scrypt
make -j4
sudo make install

Then add:

extension=scrypt.so

to your conf.d files

[cbj4074]

Thank you, @maki-chan , @redthor and @joshuataylor for pointing the rest of us to a working solution.

Hopefully, @DomBlack is still alive.

Certainly not to sound ungrateful, but this 6-month setback is all the motivation I need to migrate to the native password_* family of PHP functions.

[DomBlack]

Sorry I've been really busy recently without time to look into this. There is an open PR but I've not pulled it yet because I need to maintain backwards compatibly with PHP 5 as well.

I had planned to take that PR and put predef's in it to keep backwards compability, if somebody with time and a working PHP 7 enviroment could make that change, I will pull and publish an updated version to pecl.

(I had hopped that the password functions would include scrypt, but unfortunately that never happened)

[moderndeveloperllc]

@DomBlack Glad you still want to work on this great library! Have you thought about perhaps just making this a 2.0 release so that you can keep 5.x compatibility in the 1.x branch, and 7+ in the main branch? While in this case the fix is just a couple of lines, there may be larger API shifts in the future that you may not want to have to check for 5.x compatibility. PECL seems to support that kind of version set up.

[cbj4074]

@DomBlack No problem at all, and I really appreciate you jumping-in with a helpful reply. I hope I didn't come across as indignant (hopefully, my comment from https://github.com/DomBlack/php-scrypt/issues/39#issuecomment-177076377 makes it clear that I am very thankful for your work on this project).

@moderndeveloperllc 's suggestion makes a lot of sense to me (though, I am not particularly well-versed in this arena).

More than anything, I'm curious to know if you would recommend, in the spirit of candor, that we stick to PHP's native password_* functions until such time that said functions include scrypt?

The important thing is that you're alive. :) Thanks again for the pulse-check in the affirmative! :+1:

[DomBlack]

Hi all,

I made time at lunch today to get this sorted out, php-scrypt 1.4 has been published to pecl and works from PHP 5.3 up to and including PHP 7. I used change by @maki-chan with some predef's to keep backwards compatibility.

(Had to drop 5.2 support as travis no longer supports testing it)

[kingkorf]

Nice 👍

Pull request for homebrew-php is waiting: https://github.com/Homebrew/homebrew-php/pull/3176

[DomBlack]

@kingkorf you may want to hold off on that until #46 is resolved, it appears that RHEL 6 doesn't like the 1.4 release. (So I might be releasing 1.4.1 very shortly)

[kingkorf]

All right, I'll wait for that.

[DomBlack]

@kingkorf 1.4.1 released thanks to @remicollet for finding and fixing the bug

[kingkorf]

Reopened PR: https://github.com/Homebrew/homebrew-php/pull/3176

[redthor]

I added scrypt here: https://github.com/gophp7/gophp7-ext/wiki/extensions-catalog I hope that's ok @DomBlack