[erlang-questions] Request for comment or contributions

[Barry Ezell]

Hi Jiri,

Thanks for the comments.  I've made the changes you recommended in #1 like such:

chr_array(C, N, MinMax) when N > 0

Is that a good way of implementing a sanity check or would you
recommend something else?

I'll work on your #2 suggestion as it's an excellent recommendation.
Besides allowing for more than 26 processes, they would be evenly
distributed which isn't always the case with the current
implementation.

In your #3 recommendation, it's missing an increment from non-z
characters like b to c.  I've used the below test code (from
test_pwd.erl):

next_test() ->
	?assertEqual("b", pwd:next("a")),
	?assertEqual("ba", pwd:next("az")),
	?assertEqual("zzz", pwd:next("zzy")),
	?assertEqual("aba", pwd:next("aaz")),
	?assertEqual("zba", pwd:next("zaz")),
	?assertEqual("baa", pwd:next("azz")).

The test fails with:

2> test_pwd:test().
test_pwd:next_test...*failed*
::error:{assertEqual_failed,[{module,test_pwd},
                           {line,12},
                           {expression,"pwd : next ( \"a\" )"},
                           {expected,"b"},
                           {value,"a"}]}
  in function test_pwd:'-next_test/0-fun-0-'/1
  in call from test_pwd:next_test/0

=======================================================
  Failed: 1.  Aborted: 0.  Skipped: 0.  Succeeded: 3.


I very well may be missing something and I'll keep thinking about it.
I'm new to Erlang and am trying to get up to speed on best practices.
Thanks again for your help.

http://github.com/btedev/erlang_md5crack/tree/master


Barry


On Thu, Sep 4, 2008 at 11:13 AM, Jiří Šofka <jiri.sofka@REDACTED> wrote:
> Hi, just few quick comments:
>
> 1. The function partition_alphabet/2 never terminates when Processes < 0. It
> is a good practice to include these kinds of sanity checks. The same applies
> to chr_array/3 function and its length argument.
>
> 2. You cannot use more than 26 processes which really kills the point of
> this application. I'd suggest to calculate how many different strings of the
> specified length are there (let's call that number N) and divide the range
> (0; N) into equal subranges according to the number of desired processes.
> You'd then need a function that translates subrange boundaries to
> starting/ending strings - which shouldn't be hard to write. Each process
> will then take care of one of the subranges.
>
> 3. It is pointless to loop over the rest of the string and check whether the
> head equals to $z once the increment in next/1 changes to false. In that
> case simply reverse the remainder of the string and append the accumulator
> to it. In the end, it's probably about as fast as your version but more
> readable:
>
> next(L) ->
>  next(lists:reverse(L), []).
> next([$z|T], Acc) ->
>  next(T, [$a|Acc]);
> next(List, Acc) -> lists:reverse(List, Acc).
>
>
> JS
>
> Barry Ezell wrote:
>>
>> Hello Bengt,
>>
>> Thank you very much for the suggestions.  I've commented the code
>> better to explain the intent of the module.  This is my first Erlang
>> program and I'm trying to prepare a talk on concurrency in Erlang to
>> be given in October.  The module is demonstration code for breaking
>> MD5 passwords using a brute-force approach.  For example, if the
>> string "hi" is hashed by erlang:md5, the program would partition every
>> 2-character variant into as many arrays as processes to be spawned,
>> then each process would test the md5 hash of the character arrays
>> until "hi" is found as a match.  This is a contrived example but easy
>> enough to grasp for the purposes of a talk.
>>
>> I have implemented all of the changes you suggested except for #1.
>> There is a reason why the list is reversed before next/3 is evaluated.
>>  The character array is examined in reverse order because the
>> increment of one character may affect the increment of the character
>> in front of it.  For instance, next("aaz") must yield "aba".  When
>> reversed, "z" is matched first, changed to "a", then the increment is
>> passed to the next character in the reversed array, thus changing "a"
>> to "b".  If I am missing something I should be doing differently,
>> please let me know.
>>
>> The changes are posted at:
>> http://github.com/btedev/erlang_md5crack/tree/master/pwd.erl
>>
>> Thanks again,
>>
>> Barry
>>
>>
>>
>> On Thu, Sep 4, 2008 at 9:22 AM, Bengt Kleberg
>> <bengt.kleberg@REDACTED> wrote:
>>
>>>
>>> Greetings,
>>>
>>> Unfortunately I do not understand what your program does. So this is a
>>> very low level comment.
>>>
>>> 1 If possible try to change next() to the following:
>>> next(L) ->
>>>  next(L, [], true).
>>>
>>> next([], L, _Incr) ->
>>>  lists:reverse(L);
>>> %% the rest as in your code
>>>
>>> Unless there is a specific reason to reverse before use the normal
>>> Erlang way is to reverse afterwards.
>>>
>>>
>>> 2 In partition_alphabet() I would use a tuple for Arr. Like this:
>>> Arr = {chr_array(CurChar, Len, min) , chr_array($z, Len, max)},
>>>
>>> and also change decrypt()'s fun to fun({Min,Max}) ->
>>>
>>> Lists are normally used for a variable amount of items. Here you always
>>> have 2 items.
>>>
>>> 3 As a personal habit I would change analyze() to use case instead of
>>> if. Like this:
>>> case Test =:= Crypted of
>>> true ->
>>> false ->
>>> end.
>>>
>>> This is just because I find if very different to other langugaes if.
>>>
>>>
>>> bengt
>>>
>>>
>>> On Wed, 2008-09-03 at 17:04 -0400, Barry Ezell wrote:
>>>
>>>>
>>>> In my request for comments post yesterday, I forgot to mention that
>>>> the code can be viewed at the following URL without requiring a "git
>>>> clone".
>>>>
>>>> http://github.com/btedev/erlang_md5crack/tree/master/pwd.erl
>>>>
>>>> Thanks again for any suggestions.
>>>> _______________________________________________
>>>> erlang-questions mailing list
>>>> erlang-questions@REDACTED
>>>> http://www.erlang.org/mailman/listinfo/erlang-questions
>>>>
>>>
>>>
>>
>> _______________________________________________
>> erlang-questions mailing list
>> erlang-questions@REDACTED
>> http://www.erlang.org/mailman/listinfo/erlang-questions
>>
>
>
>