narfbg / SimpleEncryption

Simple Encryption for PHP
22 stars 6 forks source link

Make it stateless #2

Open ircmaxell opened 10 years ago

ircmaxell commented 10 years ago

I strongly recommend that you make the class stateless rather than statefull as you have it now.

Basically, don't pass in input to the constructor, but instead have discrete encrypt($text) and decrypt($text) methods.

This will make it far easier to reason about what's going on, because object state doesn't need to be considered. And separating encryption flows from decryption flows will also provide clarity.

And as a stateless object, it can be re-used (which means things like setup can be shared across encryption/decryption runs).

narfbg commented 10 years ago

Right, but my goal was to have something different. The web is full of stateles and procedural crypto libraries, while at the same time, it seems, people like working with objects.

I also don't think I'd be able to provide automatic key generation in a stateless library.

ircmaxell commented 10 years ago

The web is full of stateles and procedural crypto libraries

Well, that leaves me 2 thoughts:

  1. It's full of them for a reason
  2. Most of the ones that exist leave a lot to be desired.

I get what you're trying to do. And I'm not saying it's wrong. I'm just saying I'd recommend not doing it that way (as it makes it less useful and harder to reason about).

I also don't think I'd be able to provide automatic key generation in a stateless library.

I don't think you should provide automatic key generation. Key generation is something that shouldn't really happen often, and shouldn't happen on a web thread (IMHO) simply because there are enough concerns that need to be addressed.

But fair enough.

narfbg commented 10 years ago

Well, I do intend to write a more flexible library that would most likely be stateless.

Here I wanted something that every newbie could use by taking all responsibilities away from them and sqeezed into a few methods. This would include online key generation, becase that's practically what every newbie would do, so I figured I could at least make it more sane than using a simple password (which is another thing a newbie would do).

I understand your concern of course and with the addition of a key store, I might achieve the same goals. But I fear that the moment that this library grows beyond a single class and/or file, it'd no longer be that user-friendly. :/