spoofzu/DeepViolet

Fix types

Closed this issue · 3 comments

While working with DeepViolet, I discovered that some of the used types may be improved.
For example, I'd map the trust state of a certificate to a boolean (trusted = true, untrusted = false). Same for validity state. Also, actually getting time values when calling methods like getNotValidBefore() and getNotValidAfter() may be a good idea. I took a brief look at other interfaces, there's a type for IP addresses in Java (java.net.InetAddress) and TLS protocols as well as cipher strength evaluations may be encapsulated into enums.

There are some interfaces available you should review. Let me know if these address your concerns.

IDVX509Certificate.getTrustState()
IDVX509Certificate.TRUST_STATE_TRUSTED
IDVX509Certificate.TRUST_STATE_UNKNOWN
IDVX509Certificate.TRUST_STATE_UNTRUSTED

IDVX509Certificate.getValidityState()
IDVX509Certificate.VALID_STATE_EXPIRED
IDVX509Certificate.VALID_STATE_VALID
IDVX509Certificate.VALID_STATE_NOT_YET_VALID

I also have a couple methods to return before and after validity dates as Strings.
IDVX509Certificate.getNotValidBefore();
IDVX509Certificate.getNotValidAfter();

The enum comments, I'm not sure what you mean. Perhaps you can respond with a few lines of code to explain. Thanks for the review.

IDVX509Certificate.getTrustState()
IDVX509Certificate.TRUST_STATE_TRUSTED
IDVX509Certificate.TRUST_STATE_UNKNOWN
IDVX509Certificate.TRUST_STATE_UNTRUSTED

IDVX509Certificate.getValidityState()
IDVX509Certificate.VALID_STATE_EXPIRED
IDVX509Certificate.VALID_STATE_VALID
IDVX509Certificate.VALID_STATE_NOT_YET_VALID

I saw them, but I think wrapping them into an TrustState and ValidState enum may be better.
Stackoverflow #1
Stackoverflow #2

Apart from the benefit of having access to all features of enums, the compiler can also perform type checks and in my opinion it is easier to use the API because IDEs suggest TRUSTED. VALID and UNTRUSTED when autocompleting TrustState., so developers do not need to read source/docs to find those magic values.

// Definition
enum ValidState {
    EXPIRED,
    VALID,
    NOT_YET_VALID
}

// Usage
...
if (cert.getValidityState() == ValidState.EXPIRED) {
    ...
}
...

I also have a couple methods to return before and after validity dates as Strings.
IDVX509Certificate.getNotValidBefore();
IDVX509Certificate.getNotValidAfter();

Yes, I was referring to them. I think a method that returns a time value should use an appropriate datatype instead of a String. In my opinion there's no reason for returning a String since everybody who wants to use this information to work with its semantics has to convert it to a time datatype. Printing the information using time datatypes is also more powerful using a DateTimeFormatter

If you are interested in including my suggestions, I can work on them and send you a Pull Request.

Thanks for your thoughtful notes and the pull request. Merged! I will add data type improvements to my list for date/time.