feat(common): KeyConverter class to and from PEM/hex/buffe #255
Conversation
suyukuoacn
requested review from
jonathan-m-hamilton and
petermetz
as code owners
suyukuoacn
force-pushed
the
key_converter
branch
from
31edc65
to
b358980
Compare
petermetz
force-pushed
the
key_converter
branch
2 times, most recently
from
54d8a26
to
06af537
Compare
Signed-off-by: suyukuoacn <su-yu.kuo@accenture.com>
suyukuoacn
force-pushed
the
key_converter
branch
from
06af537
to
d3d54b1
Compare
|
|
||
| export enum KeyFormat { | ||
| Raw = "raw", | ||
| Hex = "hex", |
There was a problem hiding this comment.
I think original code https://github.com/blockstack/key-encoder-js defined this as 'der'. Can you use same value to avoid confusion?
There was a problem hiding this comment.
Currently Cactus’s key converter does not support 'DER' format.
Here's list of format and enum for the Cactus and the external key encoder representation.
PEM Format - Cactus' enum 'PEM', - Key-Encoder's enum 'PEM'
DER Format - Cactus not support, - Key-Encoder's enum 'DER'
HEX Format - Cactus' enum 'HEX', - Key-Encoder's enum 'RAW'
Buffer/Unit8Array Format - Cactus' enum 'RAW', - Key-Encoder not support.
There was a problem hiding this comment.
My comment intended to use a meaningful name for the format.
Actually key-encoder obviously supports DER(Distinguished Encoding Rule), which is the official name for the format.
Please refer related description at https://github.com/blockstack/key-encoder-js#encoding-public-keys
I believe you are aligned to the library used, but I just hope to have consistent names that represent their data format actually used.
| Suggested | CACTUS | DataEncoder | Description |
|---|---|---|---|
| PEM | PEM | PEM | PEM format |
| DER | (none) | RAW | portable binary format |
| HEX | HEX | (none) | typescript value |
There was a problem hiding this comment.
OK. Sorry for my misunderstanding.r
Is there any reason to eliminate the DER format which widely is used at may key management implementations.
I will approve this PR, but I will create new issue to support DER, for the case to support portable keys for va
I will create new issue for support DER format, then approve this PR for moment.
I will withdraw my comment. |
I will withdraw my comment. |
I will withdraw my comment see my new comment below. |
|
@sfuji822 Why did you close the pull request? Misclick? |
|
@yochliu8 Sorry I don't get it. Could you please clarify? |
This PR resolve issue #247
This PR depend on PR #245 and #246 as it uses functionality and test from other PRs.