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

Add serial number formatting #480

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bennypi
Copy link
Contributor

@bennypi bennypi commented Feb 16, 2024

The second attempt to add formatting options for serial numbers, this time based on the current main branch. Again, it's far from finished, but I would like to get some feedback if I'm heading into the right direction.

Some questions which I had during the process:

  • Where are the other places where a serial number should be formatted?
  • Which variants should be implemented?
  • Is the look&feel tab the right place for this setting?
  • Which rules are used for formatting?

Feedback is greatly appreciated, don't hold back :)

Fixes #406

- Add serial number formatting to the certificate view
- Add two formatters
- Add the formatter preference to the settings
@kaikramer
Copy link
Owner

Thank you very much for this PR! I am very happy that someone is helping out regarding this topic. I'll take a closer look at it tomorrow and also answer your questions.

@kaikramer
Copy link
Owner

  • Where are the other places where a serial number should be formatted?

What I have done so far is to centralize all the code for generating the hex strings in KSE into one class: org/kse/utilities/io/HexUtil.java

So you can find all places where hex strings are displayed by looking for the usages of the methods in this class.

What causes me most headache is that there are some places where the data is too long to fit into one line, but is also a hex string:

  • Private and public key values
  • OCTET STRINGS in the ASN.1 viewer

We should probably ignore those for now and only make the format for "one-line" hex strings configurable.

In any case please combine your code with the existing code in Hexutil. You can replace HexUtil, that's fine for me, but in the end there should be only one location in the code where hex strings are formatted.

  • Which variants should be implemented?

The following list seems most reasonable (more exotic variants can be added later if requested):

  • Separator: none, colon, space
    • 0x1234567890ABCDEF
    • 09:65:2d:f2:73:e8:fb:bd:2c
    • 9e 76 b4 db f9 42 e7 09 (maybe also in groups of 2 and 4 bytes)
  • Lower case / upper case characters (either as separate setting or as additional items in the list)

Not sure if we should also provide an option for hex marker ("0x") on/off. There could be only numbers in the hex string and then it is not distinguishable from decimal without the "0x". But on the other hand the format is either stated in the label (e.g. for serial number) or it is clear that it is hex (e.g. fingerprint)...

  • Is the look&feel tab the right place for this setting?

Yes and I like that you put the example right in the select box.

  • Which rules are used for formatting?

You mean for formatting the Java code? I have just noticed that it is probably a bad idea to have two web pages with contributing guidelines. The one on keystore-explorer.org contains infos on code formatting, but the CONTRIBUTING.md here in the repo does not... If you use IntelliJ, there is a formatter config linked on the website, otherwise just change the setting for indentation from tabs to spaces and you should be fine.

@The-Lum
Copy link
Contributor

The-Lum commented Feb 19, 2024

Hi @bennypi, and all,

And here are a proposal for the French 'resources.properties':

DPreferences.jcbSerialNumberFormatter.tooltip = S\u00E9lectionner un format pour afficher les num\u00E9ros de s\u00E9rie (S/N)
DPreferences.jlSerialNumberFormatter.text = Format des num\u00E9ros de s\u00E9rie (S/N)\u00A0:

Awaiting your new proposals,
Regards,
Th.

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

Successfully merging this pull request may close these issues.

Pretty format the hex. serial number in Certificate Details
3 participants