@rdeutz is this an Issue you can Comment?

The reason for this is a polyfill for a crypto library there is not so much we can do

@rdeutz thanks for Comment.

@clayfreeman any Comment?

Which crypto library specifically? At what file path?

I'll take a look at the tests to see if there is any room for optimization.

If you have docker (or an php5.6 env) you can see yourself, run phpunit --testdox and you will see that the JCryptCipherSodiumTest takes a very long time. IIRC then it is the $cipher->generateKey(); method what takes so long. There is not so much room for improvement without running this method the test is useless. We might need to skip these tests.

I mean, the test suite passes currently. Nothing will change that unless the source for the polyfill changes. I'm all for skipping the Sodium test.

@clayfreeman can you make a PR?

maybe add $this->markTestSkipped('Takes too long on php5.6 and windows'); we can decide later if we need the test at all

we can decide later if we need the test at all

Oh, right, let's just remove tests validating that our cryptography API actually does what it's supposed to. THAT makes sense.

Its better than the entire unit test infrastructure failing due to timeouts. This can hide more serious issues under the veil of “Oh, it’s just another timeout; no need to actually investigate the failure since I already know what went wrong.”

Personally I think if we can reduce the key size for only the unit tests, it would be the best of both worlds. We can still test key generation and all related cryptography that way, and in theory the time required to generate keys can be cut down significantly.

Thoughts?

On Aug 15, 2018, at 7:15 AM, Michael Babker notifications@github.com wrote:

we can decide later if we need the test at all

Oh, right, let's just remove tests validating that our cryptography API actually does what it's supposed to. THAT makes sense.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

I've just looked through the problematic library and it doesn't seem that reducing the key size is possible. I did, however, find that \ParagonIE\Sodium\Compat::$fastMult can be set to true at runtime which might help alleviate the issue. I could see this only reducing the time required to run by little bit though since it may not even be related to the primary issue.

ParagonIE_Sodium_Core_Util::declareScalarType() is the culprit for the slow performance. This method looks like a fancy type cast proxy to facilitate strict types on PHP <= 5.6.

I personally vote that the entire test case is skipped or that pre-generated keys are used for testing instead. The likelihood of this functionality breaking in a way that doesn't result in an exception is extremely low in my opinion.

As we pin point issue to paragonie polyfill let me ping the author @paragonie-scott.

I looked at the documentation and they give some advice what you can do to make it faster, tried it but I couldn't see that it is really faster

closed as we have a PR #21633