Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Fix Modoboa crashes when Postfix maps become modified #1654

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] Fix Modoboa crashes when Postfix maps become modified #1654

wants to merge 3 commits into from

Conversation

ntninja
Copy link
Contributor

@ntninja ntninja commented Jan 13, 2019

Description of the issue/feature this PR addresses:

Modoboa's Postfix map file generator will generate a broken modoboa-postfix-maps.chk file if it fails to validate the existing map files due to user changes. On next start (ie: the second start after a user map file change!) modoboa will then crash inexplicably.

Explanation of each commit:

  1. Don't create invalid map files when detecting unexpected changes in a map file
  2. Don't crash on startup when encountering invalid lines in the checksum file
  3. Don't crash on startup when missing checksum file entries for only some (but not all) files

As can be seen from the commit dates these commits are rather old – it's been a while since I encountered this issue last.

Current behavior before PR:

modoboa    | Traceback (most recent call last):
modoboa    |   File "manage.py", line 22, in <module>
modoboa    |     execute_from_command_line(sys.argv)
modoboa    |   File "/usr/lib/python3.6/site-packages/django/core/management/__init__.py", line 364, in execute_from_command_line
modoboa    |     utility.execute()
modoboa    |   File "/usr/lib/python3.6/site-packages/django/core/management/__init__.py", line 356, in execute
modoboa    |     self.fetch_command(subcommand).run_from_argv(self.argv)
modoboa    |   File "/usr/lib/python3.6/site-packages/django/core/management/base.py", line 283, in run_from_argv
modoboa    |     self.execute(*args, **cmd_options)
modoboa    |   File "/usr/lib/python3.6/site-packages/django/core/management/base.py", line 330, in execute
modoboa    |     output = self.handle(*args, **options)
modoboa    |   File "/usr/lib/python3.6/site-packages/modoboa/core/management/commands/generate_postfix_maps.py", line 158, in handle
modoboa    |     self.__load_checksums(destdir)
modoboa    |   File "/usr/lib/python3.6/site-packages/modoboa/core/management/commands/generate_postfix_maps.py", line 53, in __load_checksums
modoboa    |     fname, dbtype, checksum = line.split(":")
modoboa    | ValueError: too many values to unpack (expected 3)

Desired behavior after PR is merged:

Modoboa should only log message: Cannot upgrade '…' map because it has been modified.

@ntninja
Copy link
Contributor Author

ntninja commented Jan 13, 2019

Example of a generated broken checksum file:

sql-domains.cf:postgres:1a4b72269319d42e03660993c634dfc6
sql-domain-aliases.cf:postgres:3eba920e79502e0c6dfb16d07ad5005f
sql-aliases.cf:postgres:12c47c08e2f828ab1937962b5244b097
sql-maintain.cf:postgres:c799b6e34e9c23173e96544966ba6543
sql-sender-login-map.cf:postgres:{'dbtype': 'postgres', 'checksum': '91716f8df65c736cbde6abec6f7c8c2d'}
sql-transport.cf:postgres:2a6d892cebccb696d8afa5cf0d12fcd1
sql-relaydomains.cf:postgres:73639173f9966e099c012ec5a060d3c6
sql-spliteddomains-transport.cf:postgres:2bf57b102e7509835771ecc1fd0e8227
sql-relay-recipient-verification.cf:postgres:14289023719219c5d6d31e8294b2a505

@modoboa modoboa deleted a comment Jan 13, 2019
@ntninja ntninja changed the title Fixes [WIP] Fix Modoboa crashes when Postfix maps become modified Jan 13, 2019
@ntninja
Copy link
Contributor Author

ntninja commented Jan 13, 2019

Hmm… the contents of mapobject.filename appears to be inconsistent: Sometimes it's a string, sometimes it's a dict…

if not self.__check_file(fullpath):
print(
"Cannot upgrade '{}' map because it has been modified."
.format(mapobject.filename))
return self.__checksums[mapobject.filename]
return self.__checksums[mapobject.filename["checksum"]]
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure about this line?

@tonioo
Copy link
Member

tonioo commented Jan 21, 2019

Hmm… the contents of mapobject.filename appears to be inconsistent: Sometimes it's a string, sometimes it's a dict…

Do you have an example?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants