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

Use the actual PHP test suite for validating mcrypt_compat behavior #9

Open
cweagans opened this issue Jul 31, 2017 · 4 comments
Open

Comments

@cweagans
Copy link

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: travis-ci/travis-ci#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.

@terrafrost
Copy link
Member

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
Copy link
Member

This weekend wound up being busier than I had anticipated with phpseclib/phpseclib#1162 and phpseclib/phpseclib#1157 (comment) and phpseclib/phpseclib#1160 (comment) (among other things). I'll try to take a look this week as time permits.

@cweagans
Copy link
Author

cweagans commented Aug 7, 2017

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

@terrafrost
Copy link
Member

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 :)

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

No branches or pull requests

2 participants