Skip to content

BITMAG-1133: Reference client understands salt input as a Hex String. This is NOT clear from the docs#26

Merged
m-atlantis merged 8 commits intomasterfrom
BITMAG-1133-cmd-help-clearer
Jun 23, 2022
Merged

BITMAG-1133: Reference client understands salt input as a Hex String. This is NOT clear from the docs#26
m-atlantis merged 8 commits intomasterfrom
BITMAG-1133-cmd-help-clearer

Conversation

@Bohlski
Copy link
Copy Markdown
Contributor

@Bohlski Bohlski commented Jun 21, 2022

Fixed the issue and also changed underlying Base16Utils a bit.

Base16Utils now enforces correct use - i.e. input string has even length and consists only of hexadecimal characters.

This entails that e.g. running the following using the commandline client:
bitmag.sh put-file -f <file> -c <collection> -C <checksum> -R <checksum_type> -S salt
will now throw an error, as the string salt does not only consist of hexadecimals.

@Bohlski Bohlski requested review from m-atlantis and ole-v-v June 21, 2022 10:29
Copy link
Copy Markdown
Contributor

@ole-v-v ole-v-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like what I see.

cmdHandler.addOption(checksumSaltOption);

Option deleteOption = new Option(Constants.DELETE_FILE_ARG, Constants.NO_ARGUMENT,
"If this argument is present, then the file will be removed from the server, when the chosen operation is complete.");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking now we're at it: remove the second comma, the one between "server" and "when".

for (int i = 0; i < len; i += 2) {
data[i / 2] = (byte) ((Character.digit(hexString.charAt(i), 16) << 4)
+ Character.digit(hexString.charAt(i + 1), 16));
// TODO Java 17 has HexFormat.of().parseHex(s) - consider using instead
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this comment.

sb.append(Integer.toHexString(v));
}
return sb.toString();
return Hex.encodeHexString(data);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly to below one may consider adding a comment along the lines of TODO Java 17 has HexFormat.of().formatHex(bytes); - consider using instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually did write this, but somehow I deleted it again. Will add. :)

@m-atlantis m-atlantis merged commit 6f65ba2 into master Jun 23, 2022
@m-atlantis m-atlantis deleted the BITMAG-1133-cmd-help-clearer branch June 23, 2022 12:32
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.

3 participants