john icon indicating copy to clipboard operation
john copied to clipboard

Tunable cost names containing string ", "

Open frank-dittrich opened this issue 4 years ago • 3 comments
trafficstars

From formats.h:

/*
 * Descriptions (names) of tunable cost parameters for this format
 *
 * These names shouldn't contain ',', because ", " is used
 * as a separator when listing tunable cost parameters
 * in --list=format-details and --list=format-all-details.
 * The sequence of names should match the sequence of functions
 * returning tunable cost values.
 */
	char *tunable_cost_name[FMT_TUNABLE_COSTS];

Nevertheless, several formats have tunable cost names which contain , (especially formats which report a list of different algorithms as a tunable cost.

I intend to fix the offending formats, e.g. by replacing

"algorithm [0=AES, 1=TwoFish, 2=ChaCha]"

with

"algorithm [0=AES 1=TwoFish 2=ChaCha]"

and then enhancing format self test (fail if a tunable cost contains , (or even just ,), and fail if a tunable cost name i empty or if the number of elements in tunable_cost_value[] and tunable_cost_name[] differ.

Any objections?

Should this test be executed only for `full_lvl >= 0 Probably it is best to but all these tunable cost specific into a separate function.

Anything else related to tunable costs that needs to be tested?

frank-dittrich avatar May 05 '21 13:05 frank-dittrich

@frank-dittrich I didn't realize we had this issue, but I have no objections to getting it fixed like you propose. Thanks!

I see no reason to limit the added tests to some high test levels. They sound inexpensive.

solardiz avatar May 05 '21 14:05 solardiz

I wasn't aware either, and the solution sounds good to me

magnumripper avatar May 05 '21 14:05 magnumripper

I intend to fix the offending formats [...] and then enhancing format self test

@frank-dittrich Do you still intend to implement these changes, please? @magnumripper and I approved them back then, but you just didn't proceed, and this issue hangs around for 3+ years now.

solardiz avatar Jul 06 '24 18:07 solardiz