ValdikSS / tor-relay-scanner

Tor Relay availability checker, for using it as a bridge in countries with censorship
374 stars 38 forks source link

off-by-one error in main_async() #16

Closed wildekat closed 3 months ago

wildekat commented 5 months ago

Пожалуйста, посмотрите на этот фрагмент. Здесь не надо вычитать 1:

relaynum = min(NUM_RELAYS, len(relays) - relaypos - 1)

В следующей строке relaynum используется так:

test_relays = [TorRelay(relays[x])
           for x in range(relaypos, relaypos+relaynum)]

И range послушно выдаёт ровно relaynum значений — от relaypos до relaypos+relaynum-1 включительно. Например, при relaypos = 8 и len = 9 получаем неправильный relaynum = 0 и по определению пустой range(8, 8). Таким образом, последний элемент массива relays теряется.


Мне кажется, в этой строчке логичнее использовать relaynum вместо NUM_RELAYS:

print(f"\nTry {ntry}/{numtries}, We'll test the following {NUM_RELAYS} random relays:", file=sys.stderr)

И {ntry+1}/{numtries}, а то оно начинает с "Try 0/2" и заканчивает "Try 1/2". Инициализация ntry = 0 перед циклом не нужна, её можно удалить.


Ещё важный момент. Вот это часто даёт неправильный результат:

numtries = round(len(relays) / NUM_RELAYS)

Например, round(10/4) == 2. Протестирует 4*2 == 8 элементов, а про два последних забудет. Тут можно использовать math.ceil, но лучше — один из более надёжных приёмов.


А вот здесь квадратные скобки можно удалить за ненадобностью:

return "\n".join([prefix + r for r in list_])

Опечатка: constrains -> constraints

Извините, что так много всего. Надеюсь на понимание. Большое спасибо за программу.

wildekat commented 5 months ago

Предлагаю упростить цикл for ntry in range(numtries):

def chunked_list(l, size):
    for i in range(0, len(l), size):
        yield l[i:i+size]

for ntry, chunk in enumerate(chunked_list(relays, NUM_RELAYS)):
    relaynum = len(chunk)
    test_relays = [TorRelay(r) for r in chunk]
    ...

Массив аккуратно делится на кусочки, хвосты размером меньше NUM_RELAYS учитываются автоматически, relaypos исчезает сам собой и ошибиться тут просто негде. И можно будет удалить за ненадобностью некоторые другие части цикла, например:

    if not test_relays:
    break
plimut commented 5 months ago

Форкните плиз, если не затруднит, интересно посмотреть в работе. ` working_relays = list() ntry = 0 relaypos = 0 numtries = round(len(relays) / NUM_RELAYS)

def chunked_list(l, size):                                      
for i in range(0, len(l), size):
    yield l[i:i+size]

for ntry, chunk in enumerate(chunked_list(relays, NUM_RELAYS)):
    relaynum = len(chunk)
    test_relays = [TorRelay(r) for r in chunk]
    relaypos += NUM_RELAYS

    print(
        f"\nTry {ntry}/{numtries}, We'll test the following {NUM_RELAYS} random relays:", file=sys.stderr)
    for relay in test_relays:
        print(relay, file=sys.stderr)
    print("", file=sys.stderr)

`

Не уверен что правильно.

wildekat commented 5 months ago

Исправил всё перечисленное в первом посте, немного потестировал - работает как надо. Пробуйте

plimut commented 5 months ago

Собрал torparse.pyz с доработкой wildekat, работает норм, хорошая оптимизация.

ValdikSS commented 3 months ago

Merged