Главная > Code Review, Программирование > Python: генератор списка паролей для перебора (brute-force). Code Review.

Python: генератор списка паролей для перебора (brute-force). Code Review.

Оригинальная статья тут:
Python: генератор списка паролей для перебора (brute-force).

Эта поделка умудрилась даже попасть в одну из статей журнала "Хакер" - что-то там было про brute forcing для VoIP-серверов. Автор обращался за разрешением, которое я ему и дал, предупредив что скорее всего можно лучше.

Code Style

В этой части больше придирки к стилю кода. И так, оригинал с некоторыми комментариями:


# -*- coding: utf-8 -*-
# Review: строку выше, оказывается, можно заменить на просто на
# "coding: utf-8", без всяких звёздочек и дефисов.

# Review: некоторые статические анализаторы кода (pylint в частности)
# считают что переменные на уровне модуля должны именоваться в верхнем
# регистре. Более того, состоять из 3 символов минимум. В общем-то,
# что значить переменная "p" - достаточно трудно догадаться,
# PUCNTUATION или PUNCT - было бы лучше.
EN = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
en = "abcdefghijklmnopqrstuvwxyz"
RU = "АБВГДЕЁЖЗИЙКЛМНОПРСТУФХЦЧШЩЪЫЬЭЮЯ"
ru = "абвгдеёжзийклмнопрстуфхцчшщъыьэюя"
digits = "1234567890"
space = " "
p = ",.-!?;:’\"/()"
op = "+-*/:^()<>="
all_spec = "`~!@#$%^&*-_=+\\|/?.>,< '\";:[]{}" # Review: перед объявлением класса должно быть дву пустых строки # (PEP8). Также, тут используется old-style класс, лучше наследоваться # от object. class ABCIterator: # Review: first_usage или first_usage_flag смотрелось бы органичней. # А _first_usage было бы ещё лучше, т.к. по сути переменная для # внутренеего пользования. firstuse_flag = True stop = None # Review: лучше соблюдать ограничение длины строки в 79 символов # (PEP8). def __init__(self, start="", stop=None, start_len=None, stop_len=None, abc=en+EN+digits+all_spec): # Review: assert - это только для отладки. Лучше было бы # if ...: raise ValueError('abc must be non empty'). # abc - должен содержать только уникальные значения, поэтому # лучше использовать frozenset или специальную реализацию # OrderedSet (нет в built-in, но гуглится легко). assert len(abc) > 0
if start_len is not None:
# Review: опять assert и лишний закомменченынй кусок.
assert start_len > 0
#assert start == ""
# Review: по PEP8 должны быть пробелы между операндами.
self.current_str = list(abc[0]*start_len)
else:
# Review: [x for x in start if x in abc]. Но если в
# стартовой строке символы не из допустимого алфавита, то
# лучше кинуть exception, не умничать и не прогибаться под
# неправильное использование.
self.current_str = list(filter(lambda x: x in abc, start))
if stop_len is not None:
# Review: опять assert.
assert (start_len is None) or (start_len <= stop_len) assert stop_len > 0
# Review: пробелы между операндами, 79 символов, 2 пробела
# перед in-line комментариями.
self.stop = list(abc[0]*(stop_len+1)) # т.к. итератор работает с полуотрезками
else:
# Review: лучше не лепить в одну строку. filter опять можно
# заменить на списковое выражение. 79 символов. И опять
# лучше кинуть exception.
if stop is not None: self.stop = list(filter(lambda x: x in abc, stop))
self.abc = list(abc)

def __iter__(self):
return self

def next(self):
if (self.stop is not None) and (self.stop == self.current_str):
raise StopIteration
# Review: наверное лучше было бы проверять длину.
if self.current_str == []:
self.current_str.append(self.abc[0])
self.firstuse_flag = False
return self.abc[0]
elif self.firstuse_flag:
self.firstuse_flag = False
return "".join(self.current_str)
# Review: цикл можно было бы сделать по current_str с
# использованием функции enumerate.
offset = 0
while offset < len(self.current_str): # Review: self.current_str[offset] используется 4 раза, # было бы удобней завести переменную для быстрого доступа. if self.current_str[offset] != self.abc[-1]: # Review: 79 символов, пробелы. self.current_str[offset] = self.abc[self.abc.index(self.current_str[offset])+1] # выпендрёшь для полуотрезка if (self.stop is not None) and (self.current_str == self.stop): raise StopIteration return "".join(self.current_str) self.current_str[offset] = self.abc[0] offset += 1 # Review: self.current_str.insert(0, self.abc[0]). Хотя вставка # справа должна работать скорее, если не ошибаюсь. Поэтому, # возможно, было бы удобней держать текущую строку в # реверсивном виде. self.current_str = [self.abc[0]] + self.current_str if (self.stop is not None) and (self.current_str == self.stop): raise StopIteration return "".join(self.current_str)[/code] Также, можно было бы обойтись генератором с использованием yield вместо итератора, в данном случае он лишний. На самом деле, оригинальная задумка была реализовать простой интерфейс с возможностью деления последовательностей на части, тут бы итератор был уместен, но в текущем варианте вполне можно обойтись и генератором.

Refactoring, v1

С учётом замечаний по стилю:

# coding: utf-8

EN_UPPER = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
EN_LOWER = "abcdefghijklmnopqrstuvwxyz"
RU_UPPER = "АБВГДЕЁЖЗИЙКЛМНОПРСТУФХЦЧШЩЪЫЬЭЮЯ"
RU_LOWER = "абвгдеёжзийклмнопрстуфхцчшщъыьэюя"
DIGITS = "1234567890"
SPACE = " "
PUNCTUATIONS = ",.-!?;:’\"/()"
OPERATORS = "+-*/:^()<>="
ALL_SPECS = "`~!@#$%^&*-_=+\\|/?.>,< '\";:[]{}"
DEFAULT_ABC = EN_LOWER + EN_UPPER + DIGITS + ALL_SPECS


class ABCIterator(object):
    def __init__(self, start="", stop=None, start_len=None, stop_len=None,
                 abc=DEFAULT_ABC):
        self._first_usage = True
        self.abc = tuple(abc)

        # FIXME: quick check for uniqueness, could be better
        if not abc or len(frozenset(abc)) != len(abc):
            raise ValueError('invalid abc')

        if start_len is not None:
            if start_len < 0:
                raise ValueError('invalid start len')
            self.current_str = list(abc[0] * start_len)
        else:
            self._check_str(start)
            self.current_str = start

        if stop_len is not None:
            # Review: опять assert.
            if start_len is not None and start_len > stop_len or stop_len < 0:
                raise ValueError('invalid stop_len')
            # т.к. итератор работает с полуотрезками, прибавляем 1
            self.stop = list(abc[0] * (stop_len + 1))
        else:
            if stop is not None:
                self._check_str(stop)
            self.stop = stop

    def _check_str(self, str_):
        """Check that str_ is allowed for current abc."""
        if not hasattr(self, '_abc_set'):
            self._abc_set = frozenset(self.abc)
        if not self._abc_set.issuperset(str_):
            raise ValueError('invalid start string for given abc')

    def __iter__(self):
        return self

    def next(self):
        if (self.stop is not None) and (self.stop == self.current_str):
            raise StopIteration()

        if not self.current_str:
            self.current_str = [self.abc[0]]
            self._first_usage = False
            return self.current_str[0]
        elif self._first_usage:
            self._first_usage = False
            return "".join(self.current_str)

        for offset, value in enumerate(self.current_str):
            if value != self.abc[-1]:
                self.current_str[offset] = self.abc[self.abc.index(value) + 1]
                if (self.stop is not None) and (self.current_str == self.stop):
                    raise StopIteration()
                return "".join(self.current_str)
            self.current_str[offset] = self.abc[0]
        self.current_str.insert(0, self.abc[0])

        # опять же оно же
        if (self.stop is not None) and (self.current_str == self.stop):
            raise StopIteration()

        return "".join(self.current_str)

Refactoring, v2

Итератор вместо генератора:

# coding: utf-8

EN_UPPER = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
EN_LOWER = "abcdefghijklmnopqrstuvwxyz"
RU_UPPER = "АБВГДЕЁЖЗИЙКЛМНОПРСТУФХЦЧШЩЪЫЬЭЮЯ"
RU_LOWER = "абвгдеёжзийклмнопрстуфхцчшщъыьэюя"
DIGITS = "1234567890"
SPACE = " "
PUNCTUATIONS = ",.-!?;:’\"/()"
OPERATORS = "+-*/:^()<>="
ALL_SPECS = "`~!@#$%^&*-_=+\\|/?.>,< '\";:[]{}"
DEFAULT_ABC = EN_LOWER + EN_UPPER + DIGITS + ALL_SPECS


def _check_str(str_, abc):
    """Check that str_ is allowed for current abc."""
    if not frozenset(abc).issuperset(str_):
        raise ValueError('invalid start string for given abc')


def abc_iter(start="", stop=None, start_len=None, stop_len=None,
             abc=DEFAULT_ABC):
    # prepearing stuff
    abc = tuple(abc)

    # FIXME: quick check for uniqueness, could be better
    if not abc or len(frozenset(abc)) != len(abc):
        raise ValueError('invalid abc')

    if start_len is not None:
        if start_len < 0:
            raise ValueError('invalid start len')
        current_str = list(abc[0] * start_len)
    else:
        _check_str(start, abc)
        current_str = start

    if stop_len is not None:
        # Review: опять assert.
        if start_len is not None and start_len > stop_len or stop_len < 0:
            raise ValueError('invalid stop_len')
        # т.к. итератор работает с полуотрезками, прибавляем 1
        stop = list(abc[0] * (stop_len + 1))
    else:
        if stop is not None:
            _check_str(stop, abc)
        stop = stop

    # start iterations
    if not current_str:
        current_str = [abc[0]]

    while True:
        for offset, value in enumerate(current_str):
            if stop is not None and current_str == stop:
                raise StopIteration()
            yield ''.join(current_str)

            if value != abc[-1]:
                current_str[offset] = abc[abc.index(value) + 1]
            else:
                current_str[offset] = abc[0]

        if stop is not None and current_str == stop:
            raise StopIteration()
        yield ''.join(current_str)
        # we can't just insert new first value from abc because then
        # we would miss all-first combination (e.g. '0000' for digits)
        current_str = list(abc[0] * (len(current_str) + 1))

Refactoring, v3

На самом деле больше кода не будет, просто хотел упомянуть что можно было реализовать часть подобного функционала с помощью itertools.product, как было сказано в комментариях к оригиналу.

P.S. Переделывал код достаточно поздно, поэтому, может где-то ошибся. Но на первый взгляд всё в порядке.
P.P.S. Понял что подсветка кода немного корявая и портит исходники, поэтому часть кода без неё.

Пожалуйста, оцените полезность и качество данной статьи. Одна звезда - плохо, 5 - хорошо.
1/5. Мы будем признательны, если вы напишете комментарий с причиной низкой оценки.2/5. Мы будем признательны, если вы напишете комментарий с причиной низкой оценки.3/5. Мы будем признательны, если вы напишете комментарий с причиной низкой оценки.4/5.5/5. (3 голосов, средний: 5,00 из 5)
Загрузка...
  1. Аноним
    19 января 2017 в 01:07 | #1

    import string
    import itertools

    chars = list(string.digits) + list(string.ascii_letters) # + list(string.punctuation)

    pas_list = open('brute_force.txt', 'w')

    for j in range(1, 10 + 1):
    for i in itertools.product(chars, repeat=j):
    pas_list.write(''.join(i) + '\n')

    pas_list.close()

  1. Пока что нет уведомлений.