torproject / nyx

Command-line monitor for Tor.
https://nyx.torproject.org/
GNU General Public License v3.0
123 stars 26 forks source link

Nyx crashes when it reads a 'Log' entry from the torrc file with tabs in it. #60

Closed ayerlock closed 2 years ago

ayerlock commented 2 years ago

If nyx encounters a 'Log' entry in the torrc file that is spaced out with tabs rather than spaces it crashes. i.e.

Log<TAB>info<TAB>file<TAB>/var/log/tor/info.log

will cause nyx to crash with the following error:


Exception in thread Thread-6 (halt_panels):
Traceback (most recent call last):
  File "/usr/lib64/python3.10/threading.py", line 1009, in _bootstrap_inner
    self.run()
  File "/usr/lib64/python3.10/threading.py", line 946, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib/python3.10/site-packages/nyx/__init__.py", line 745, in halt_panels
    panel.join()
  File "/usr/lib64/python3.10/threading.py", line 1084, in join
    raise RuntimeError("cannot join thread before it is started")
RuntimeError: cannot join thread before it is started
Traceback (most recent call last):
  File "/usr/bin/nyx", line 33, in <module>
    sys.exit(load_entry_point('nyx==2.1.0', 'console_scripts', 'nyx')())
  File "/usr/lib/python3.10/site-packages/nyx/__init__.py", line 176, in main
    nyx.starter.main()
  File "/usr/lib/python3.10/site-packages/stem/util/conf.py", line 289, in wrapped
    return func(*args, config = config, **kwargs)
  File "/usr/lib/python3.10/site-packages/nyx/starter.py", line 128, in main
    nyx.curses.start(nyx.draw_loop, acs_support = config.get('acs_support', True), transparent_background = True, cursor = False)
  File "/usr/lib/python3.10/site-packages/nyx/curses.py", line 219, in start
    curses.wrapper(_wrapper)
  File "/usr/lib64/python3.10/curses/__init__.py", line 94, in wrapper
    return func(stdscr, *args, **kwds)
  File "/usr/lib/python3.10/site-packages/nyx/curses.py", line 217, in _wrapper
    function()
  File "/usr/lib/python3.10/site-packages/nyx/__init__.py", line 194, in draw_loop
    interface = nyx_interface()
  File "/usr/lib/python3.10/site-packages/nyx/__init__.py", line 256, in nyx_interface
    Interface()  # constructor sets NYX_INTERFACE
  File "/usr/lib/python3.10/site-packages/nyx/__init__.py", line 601, in __init__
    first_page_panels.append(nyx.panel.log.LogPanel())
  File "/usr/lib/python3.10/site-packages/nyx/panel/log.py", line 110, in __init__
    log_location = nyx.log.log_file_path(tor_controller())
  File "/usr/lib/python3.10/site-packages/nyx/log.py", line 88, in log_file_path
    if entry_comp[1] == 'file':
IndexError: list index out of range
h3xagonal commented 2 years ago

Correcting for this issue would be trivial. But does the upstream project want to have this particular condition handled?

Thank you

atagar commented 2 years ago

I'd be delighted to merge a patch if it's correct. On first glance nyx's split() call should be fine with tabs...

>>> "hello world".split()
['hello', 'world']

>>> "hello\tworld".split()
['hello', 'world']
h3xagonal commented 2 years ago

May have been too hasty in pinning it as the wrong delimiter with string-splitting. That is not the issue.

It appears when nyx retrieves the log path using the stem library functions get_conf/get_conf_map the value gets mangled when tab-delimited in torrc

notice\\tfile\\t/var/log/tor

This is why splitting of the tokens fail.

Raw ControlPort access with the GETCONF command shows the response is normally escaped so I suspect the issue involves string handling when the response is already escaped.

Thank you

atagar commented 2 years ago

Thanks, makes sense. I'd be delighted to merge a patch that addresses it.

h3xagonal commented 2 years ago

Did some more tracing.

When the message is received by stem the string-formatting of Python double-escapes any escaped tabs in the response so the issue probably could be fixed by un-escaping the value. The question is if to fix it when retrieving the log path or globally for all messages received using stem. Only caveat with suggesting the latter is that the concern with unforeseen breakage exists.

Thank you.

atagar commented 2 years ago

Thanks h3xagonal! Sorry for the long delay.

Performing the conversion within nyx is a bit of a hack but I merged this because...

  1. Your patch will certainly do the trick.
  2. The proper solution would be to look into tor's codebase, see how it handles tabs, if it changed recently, then either adjust tor or normalize in stem's get_conf_map.

I no longer work with tor so opting for the simple approach and merged this. ;)

h3xagonal commented 2 years ago

Happy to have provided assistance.

I no longer work with tor so opting for the simple approach and merged this. ;)

Off-topic question but is the status of nyx unmaintained or will others be able to merge pull-requests for correction of issues or new features and issue new releases?

Good luck to other ventures.

Thank you.

atagar commented 2 years ago

is the status of nyx unmaintained or will others be able to merge pull-requests for correction of issues or new features and issue new releases?

I continue to merge pull requests for Nyx and Stem but my chief reason for leaving tor was their disinterest in my projects so nope, further releases aren't presently in the cards. Sorry to disappoint.