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

Эта поделка умудрилась даже попасть в одну из статей журнала «Хакер» — что-то там было про 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

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

Refactoring, v2

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

Refactoring, v3

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

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

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

Понравилась статья? Поделиться с друзьями:
Добавить комментарий

;-) :| :x :twisted: :smile: :shock: :sad: :roll: :razz: :oops: :o :mrgreen: :lol: :idea: :grin: :evil: :cry: :cool: :arrow: :???: :?: :!: