Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/bin/mita2-general-log-filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def main(self):
if query != "":
output = True

if '--user' in opts.keys() and not session_user[session_id] == opts['--user']:
if '--user' in opts.keys() and (session_id not in session_user or not session_user[session_id] == opts['--user']):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this change correctly fixes a potential KeyError, the condition is a bit verbose. You can make this check more concise and Pythonic by using dict.get().

Suggested change
if '--user' in opts.keys() and (session_id not in session_user or not session_user[session_id] == opts['--user']):
if '--user' in opts and session_user.get(session_id) != opts['--user']:

output = False

if '--command' in opts.keys() and cmd != opts['--command']:
Expand Down Expand Up @@ -67,7 +67,7 @@ def main(self):
else:
raise Exception('Unkown format %s' % line)

match = re.match(r' *([0-9]+) (\w+)', session_cmd)
match = re.match(r' *([0-9]+) (.+)$', session_cmd)
session_id = match.group(1)
cmd = match.group(2)

Expand All @@ -76,6 +76,11 @@ def main(self):
user = match.group(1)
session_user[session_id] = user

if cmd == 'Change user':
match = re.match(r'([^@]+)@[^@]+ on .+', query)
user = match.group(1)
session_user[session_id] = user
Comment on lines +79 to +82
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This block of code is nearly identical to the logic for the 'Connect' command on lines 74-77. This duplication can make the code harder to maintain.

Furthermore, both this block and the 'Connect' block are susceptible to a crash. re.match can return None, and calling .group(1) on None will raise an AttributeError. You should always check the result of re.match before using it.

I recommend refactoring to remove the duplication and add the necessary safety check. Here's how you could combine and fix lines 74-82:

if (cmd == 'Connect' and not re.match(r'Access denied for user', query)) or (cmd == 'Change user'):
    match = re.match(r'([^@]+)@[^@]+ on .+', query)
    if match:
        user = match.group(1)
        session_user[session_id] = user


else:
query = query + ' ' + line

Expand Down