mcrypt_compat icon indicating copy to clipboard operation
mcrypt_compat copied to clipboard

Use the actual PHP test suite for validating mcrypt_compat behavior

Open cweagans opened this issue 7 years ago • 4 comments

I started writing a polyfill a couple years ago, and while I didn't make much progress on the actual implementation of the mcrypt functions, the test suite was pretty robust. Maybe you'd like to take some inspiration from it: https://github.com/cweagans/mcrypt-polyfill/tree/d261945011d06273fa3221e68cb2b7e75a40b3ee

Travis CI's PHP has mcrypt compiled in (rather than loaded as an extension - bug report here: https://github.com/travis-ci/travis-ci/issues/4701), so I chose to run everything in Docker containers. I'm happy to provide details if it's something that you'd be interested in. The basic idea is that all of the tests from PHP itself that were used to validate ext-mcrypt can also be run against a polyfill, which would guarantee full compatibility.

cweagans avatar Jul 31 '17 02:07 cweagans

That looks interesting! I didn't know you could do docker on Travis CI. I'll try to take a look this weekend.

Thanks!!

terrafrost avatar Jul 31 '17 13:07 terrafrost

This weekend wound up being busier than I had anticipated with https://github.com/phpseclib/phpseclib/pull/1162 and https://github.com/phpseclib/phpseclib/pull/1157#issuecomment-320464970 and https://github.com/phpseclib/phpseclib/pull/1160#issuecomment-320468102 (among other things). I'll try to take a look this week as time permits.

terrafrost avatar Aug 07 '17 05:08 terrafrost

No hurry. Just wanted to make you aware that this was possible. :)

cweagans avatar Aug 07 '17 18:08 cweagans

So I was messing around with this (finally lol) and I was getting errors like this:

1) /home/travis/build/terrafrost/mcrypt_compat/tests/5.6/bug35496.phpt
Failed asserting that format description matches text.
--- Expected
+++ Actual
@@ @@
-Warning: mcrypt_generic(): Operation disallowed prior to mcrypt_generic_init(). in %sbug35496.php on line 3
+Warning: mcrypt_generic(): Operation disallowed prior to mcrypt_generic_init(). in - on line 3
 
-Warning: mdecrypt_generic(): Operation disallowed prior to mcrypt_generic_init(). in %sbug35496.php on line 4
+Warning: mdecrypt_generic(): Operation disallowed prior to mcrypt_generic_init(). in - on line 4

Before I dig into it too much myself I thought I'd ask you about it!

So in composer.json you're applying this patch:

https://gist.githubusercontent.com/cweagans/48711a05bce931075eff/raw/9705afa9b989473783461dc9cef89dd0e9cd2c1c/PHPUnit_Extensions_PhptTestCase.patch

That does, among other things, this:

--- a/src/Extensions/PhptTestCase.php
+++ b/src/Extensions/PhptTestCase.php
@@ -109,6 +109,10 @@ private function assertPhptExpectation(array $sections, $output)
                 $assertion      = $sectionAssertion;
                 $expected       = $sectionName == 'EXPECTREGEX' ? "/{$sectionContent}/" : $sectionContent;
 
+                // HACK: Get rid of filenames in expected output.
+                $expected = str_replace('%s%e' . basename($this->filename, 't'), '-', $expected);
+                $expected = str_replace('%s' . basename($this->filename, 't'), '-', $expected);
+
                 break;
             }
         }

It seems like that should make the errors I'm encountering go away? It's not immediately obvious to me why it isn't.

It's also not immediately obvious to me why your repo has all the unit tests passing:

https://travis-ci.org/cweagans/mcrypt-polyfill/jobs/259065367

In fact, it doesn't appear that your library is outputting "Warning: mcrypt_generic(): Operation disallowed prior to mcrypt_generic_init()" at all:

https://github.com/cweagans/mcrypt-polyfill/blob/d261945011d06273fa3221e68cb2b7e75a40b3ee/src/mcrypt.php

...and yet despite that you have all the unit tests passing?

Anyway, like I said, I could dig into it further myself but figured I'd ask you, first, before I grind my wheels :)

terrafrost avatar Aug 31 '17 04:08 terrafrost