User tests: Successful: Unsuccessful:
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Don't mind that. PHPCS is set up to ignore known third party libraries, that path isn't one in the exclude list.
I can move it to vendor/ if that will help.
I'll send you a PR in a few minutes (wrapping up paid work tasks). That'll take care of everything.
Do we actually need to add this? If the only reason to add it is to provide a secure alternative to JCrypt, that if I understand correctly, we're not using in the core, for extensions isn't it enough just to have the readme like in #8407 recommending that the extensions use defuse/php-encryption and telling them where to get it so that they can bundle it with their extension.
Adding a library just in case someone might need to use it for their extension just doesnt seem right to me
Getting defuse/php-encryption 1.1 (which is the only one that works with PHP 5.3) isn't as simple as composer require
, since it wasn't added to composer until 1.2
was released. It might make it easier for folks who are running 5.3.10 if you bundle it.
That said, if you'd rather not want to maintain this code, you're right to not merge the PR.
Joomla extensions typically do not install through composer
On 12 November 2015 at 22:21, Scott notifications@github.com wrote:
Getting defuse/php-encryption 1.1 (which is the only one that works with
PHP 5.3) isn't as simple as composer require, since it wasn't added to
composer until 1.2 was released. It might make it easier for folks who
are running 5.3.10 if you bundle it.That said, if you'd rather not want to maintain this code, you're right to
not merge the PR.—
Reply to this email directly or view it on GitHub
#8406 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
My suggestion is to put it in and look at a way to proxy JCrypt into it (even if means adding a new Cipher class that does the trick). Step one if this is going to be the encryption library going forward is to merge it now instead of wait for 4.0 and rip it all out at once.
@brianteeman I sent @paragonie-scott paragonie-scott#2 which implements the library as a new JCryptCipher
class. So that'll get around the "we aren't using this" argument and actually strengthens the suggestion to folks using JCrypt to use this new object over the existing methods.
Labels |
Added:
?
|
OK I've pinged this in our maintainers chat to test. Otherwise I'll get this tested tomorrow evening when I'm next free!
I have tested this item successfully on be3e3d2
I have tested this using this code
$cipher = new JCryptCipherCrypto();
$key = $cipher->generateKey(); // Store this for long-term use
$message = 'Joomla!Rocks';
$ciphertext = $cipher->encrypt($message, $key);
$decrypted = $cipher->decrypt($ciphertext, $key);
A key was generated, the ciphertext was generated and the message was decrypted again.
I have tested this item successfully on be3e3d2
Works as expected:
$cipher = new JCryptCipherCrypto();
$key = $cipher->generateKey(); // Store this for long-term use
$message = 'Joomla!Rocks';
echo $message;
echo '<br>';
$ciphertext = $cipher->encrypt($message, $key);
echo $ciphertext;
echo '<br>';
$decrypted = $cipher->decrypt($ciphertext, $key);
echo $decrypted;
exit;
Result
Joomla!Rocks
àœ™h°çg�íV|ܲ�p¸?Ã’a5ë¿Xë¼p¬Õ��¼Fç6ë æG¨œ�ÐÜ„QÂ�þEB�È×�™3�ÄÃcüˆ
Joomla!Rocks
Thnaks.
Milestone |
Added: |
||
Status | Pending | ⇒ | Ready to Commit |
Labels |
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-11-13 22:37:11 |
Closed_By | ⇒ | wilsonge |
Excellent.
Labels |
Removed:
?
|
Huh. Whitespace issues.