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

remove dependency on php serialize #31

Open
KJLJon opened this issue Dec 15, 2017 · 2 comments
Open

remove dependency on php serialize #31

KJLJon opened this issue Dec 15, 2017 · 2 comments

Comments

@KJLJon
Copy link
Contributor

KJLJon commented Dec 15, 2017

This is less of an issue with the KeyDrop (#27)
but if KeyDrop isn't use it would be nice to create this code in a different language and use the same keys.

Right now it is using php serialize which can probably be replaced with json_encode / json_decode

$plaintext = "p" . serialize($value);

$this->value = unserialize(substr($value, 1));

@starekrow
Copy link
Owner

I used serialize advisedly, because JSON is utterly useless for binary strings unless you pre-encode them and secret tokens are fairly likely to contain such strings. Eventually it should be migrated to msgpack or some other less language-specific binary encoding. That's one reason for the encoding type prefix (that "p" there).

Though actually, I wouldn't mind trying to pre-flight a conversion to JSON ("j" prefix) and fall back to serialize only if the value contains non-UTF8 strings.

@alexmanno
Copy link
Contributor

We can use strategy pattern (https://github.com/alexmanno/DesignPatternsPHP/tree/master/Behavioral/Strategy)

For example:

interface SerializerInterface { 
public function serialize();
public function unseiralize();
}

class PHPSerializer implements SerializerInterface { /* implementation */ }

class JsonSerializer implements SerializerInterface { /* implementation */ }

class Secret
{
     // ....
     public function __constructor(SerializerInterface $serializer, $value, $_import = null)
    {
     // ...
     }
     //...
}

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

3 participants