Add support for streaming and deterministic key generation

[hellais]

This branch implements what we discussed in: #260.

We are relying on this feature being merged as it's needed for our new release of GlobaLeaks.

Can you please review and merge 😺

[vecna]

P.S. there are a meat cleaver because I was cooking, no threat intended ;)

[hellais]

Note: the phantomjs unittest is not passing, I believe due to the fact that the buffer polyfill is not being inserted into the unittest code properly, though when I use this in a browser it actually works well.

Do you have any idea why that is happening?

[tanx]

Nice pic. Looks like good times :) I will try to review the PR as soon as I find the time.

Note: the phantomjs unittest is not passing, I believe due to the fact that the buffer polyfill is not being inserted into the unittest code properly, though when I use this in a browser it actually works well.

I had this issue as well. It seems phantomjs does not play nice with v2.x of the ES6-Promise polyfill. I reverted to v1.x in 957d346 and created a patch release v1.0.1 of openpgp.js.

Just rebase your PR to the current master and the tests should run again.

[evilaliv3]

i think the best here would be to split the pull request in two:

• streaming capability (that would be needed by us in areaonable short term)
• deterministic key generation (wich integration can be postponed to a later stage)

@tanx if do you agree on this i will proceed splitting them and simplify a little the pull request making it ready for integration together with #315

[tanx]

Yes. Please always split PRs into small atomic batches. That will greatly increase chances of the reviewer finding the time to review and merge it ;)

I have not looked over the PR yet. One thing that I saw though, is that you added dist builds. Please always remove those.

Thanks

[hellais]

@tanx can you confirm or deny that it's ok or not to use the node.js streams and Buffer for this?

I believe this is the most standard way of implementing a stream API and buffer by @feross is very well written.

Can we use Buffer (pretty please with a cherry on the top 🍒 🍰)?

[tanx]

@tanx can you confirm or deny that it's ok or not to use the node.js streams and Buffer for this?

I believe this is the most standard way of implementing a stream API and buffer by @feross is very well written.

Can we use Buffer (pretty please with a cherry on the top )?

I'm not religious about using or not using any api. My main concern is that we keep the codebase lean and maintainable so that security audits are (financially) feasible. The more third party dependencies we add, the harder it will be to audit.

I'm sorry that I haven't gotten to review this yet. I'll try to get to it once I'm back on the grid. Until then perhaps @toberndo has some feedback?

[evilaliv3]

@tanx that's no problem. i'm still working on separating the pull into two parts.

so it's ok if we keep the Buffer in order to have a more compatible API for future develepement?
if not it would be important to know it now as i was already going to implement the minimum functionalities without using buffer but i want to avoid to waste time of something not useful.

[tanx]

so it's ok if we keep the Buffer in order to have a more compatible API for future develepement?
if not it would be important to know it now as i was already going to implement the minimum functionalities without using buffer but i want to avoid to waste time of something not useful.

If it makes your life easier go ahead and use buffer. Feross' module looks pretty clean.

[evilaliv3]

yep, it's not a matter of making the life easier for current development, but @hellais sugg4st that having already a buffer compliant API would make more easy a future development.

anyhow i've quite finalized a patch without the usage of Buffer, i wanna make a PoC where around this i will wrap the Buffer API that @hellais would like; if it will work with the same exact performances (i will jsperf both) we can go for this option what will make you both happy!

[hellais]

Closing this since superseeded by #321