aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
433 stars 186 forks source link

missing validation of command line parameters #3301

Closed BangyuXing closed 4 years ago

BangyuXing commented 5 years ago

I tried to set up a new profile. Because of some misunderstanding, I entered "!" for "First name", "Last name" and "Institution", and encountered this error.

verdi quicksetup --db-username aiida --db-password ne9N_LDK-*JSS
Info: enter "?" for help
Info: enter "!" to ignore the default and set no value
Profile name [quicksetup]: b
User email [aiida@localhost]: b
First name [Max]: !
Last name [Scientist]: !
Institution [Quantum Mobile]: !
Warning: Found host 'localhost' but dropping '-h localhost' option for psql since this may cause psql to switch to password-based authentication.
Success: created new profile `b`.
Info: migrating the database.
Operations to perform:
  Apply all migrations: auth, contenttypes, db
Running migrations:
  Applying contenttypes.0001_initial... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  Applying auth.0001_initial... OK
  Applying auth.0002_alter_permission_name_max_length... OK
  Applying auth.0003_alter_user_email_max_length... OK
  Applying auth.0004_alter_user_username_opts... OK
  Applying auth.0005_alter_user_last_login_null... OK
  Applying auth.0006_require_contenttypes_0002... OK
  Applying auth.0007_alter_validators_add_error_messages... OK
  Applying auth.0008_alter_user_username_max_length... OK
  Applying db.0001_initial... OK
  Applying db.0002_db_state_change... OK
  Applying db.0003_add_link_type... OK
  Applying db.0004_add_daemon_and_uuid_indices... OK
  Applying db.0005_add_cmtime_indices... OK
  Applying db.0006_delete_dbpath... OK
  Applying db.0007_update_linktypes... OK
  Applying db.0008_code_hidden_to_extra... OK
  Applying db.0009_base_data_plugin_type_string... OK
  Applying db.0010_process_type... OK
  Applying db.0011_delete_kombu_tables... OK
  Applying db.0012_drop_dblock... OK
  Applying db.0013_django_1_8... OK
  Applying db.0014_add_node_uuid_unique_constraint... OK
  Applying db.0015_invalidating_node_hash... OK
  Applying db.0016_code_sub_class_of_data... OK
  Applying db.0017_drop_dbcalcstate... OK
  Applying db.0018_django_1_11... OK
  Applying db.0019_migrate_builtin_calculations... OK
  Applying db.0020_provenance_redesign... OK
  Applying db.0021_dbgroup_name_to_label_type_to_type_string... OK
  Applying db.0022_dbgroup_type_string_change_content... OK
  Applying db.0023_calc_job_option_attribute_keys... OK
  Applying db.0024_dblog_update... OK
  Applying db.0025_move_data_within_node_module... OK
  Applying db.0026_trajectory_symbols_to_attribute... OK
  Applying db.0027_delete_trajectory_symbols_array... OK
  Applying db.0028_remove_node_prefix... OK
  Applying db.0029_rename_parameter_data_to_dict... OK
  Applying db.0030_dbnode_type_to_dbnode_node_type... OK
  Applying db.0031_remove_dbcomputer_enabled... OK
  Applying db.0032_remove_legacy_workflows... OK
  Applying db.0033_replace_text_field_with_json_field... OK
  Applying db.0034_drop_node_columns_nodeversion_public... OK
  Applying db.0035_simplify_user_model... OK
  Applying db.0036_drop_computer_transport_params... OK
  Applying db.0037_attributes_extras_settings_json... OK
  Applying db.0038_data_migration_legacy_job_calculations... OK
  Applying db.0039_reset_hash...Warning: Invalidating all the hashes of all the nodes. Please run verdi rehash
 OK
  Applying db.0040_data_migration_legacy_process_attributes... OK
Success: database migration completed.
Traceback (most recent call last):
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
psycopg2.errors.NotNullViolation: null value in column "first_name" violates not-null constraint
DETAIL:  Failing row contains (1, b, null, null, null).

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/max/codes/aiida-core/aiida/orm/implementation/django/utils.py", line 91, in save
    self._model.save()
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/models/base.py", line 808, in save
    force_update=force_update, update_fields=update_fields)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/models/base.py", line 838, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/models/base.py", line 924, in _save_table
    result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/models/base.py", line 963, in _do_insert
    using=using, raw=raw)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/models/query.py", line 1079, in _insert
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 1112, in execute_sql
    cursor.execute(sql, params)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/utils/six.py", line 685, in reraise
    raise value.with_traceback(tb)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
django.db.utils.IntegrityError: null value in column "first_name" violates not-null constraint
DETAIL:  Failing row contains (1, b, null, null, null).

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/max/.virtualenvs/aiida/bin/verdi", line 10, in <module>
    sys.exit(verdi())
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/click/core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/home/max/codes/aiida-core/aiida/cmdline/commands/cmd_setup.py", line 181, in quicksetup
    ctx.invoke(setup, **setup_parameters)
  File "/home/max/.virtualenvs/aiida/lib/python3.6/site-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/home/max/codes/aiida-core/aiida/cmdline/commands/cmd_setup.py", line 93, in setup
    user.store()
  File "/home/max/codes/aiida-core/aiida/orm/entities.py", line 266, in store
    self._backend_entity.store()
  File "/home/max/codes/aiida-core/aiida/orm/implementation/django/entities.py", line 107, in store
    self._dbmodel.save()
  File "/home/max/codes/aiida-core/aiida/orm/implementation/django/utils.py", line 93, in save
    raise exceptions.IntegrityError(str(exception))
aiida.common.exceptions.IntegrityError: null value in column "first_name" violates not-null constraint
DETAIL:  Failing row contains (1, b, null, null, null).
ltalirz commented 5 years ago

Thanks @BangyuXing for reporting this issue. The problem seems to be that the string "!" is transformed to an empty string at some point along the way, which then violates a database constraint. We should really add some basic sanity checks of strings for user names etc. at the click level

ltalirz commented 5 years ago

Note to self: After encountering this issue in the tutorial, verdi quicksetup was broken for some reason, even when using sensible values for the input. The reason seemed to be that it was trying to use a combination of database user and password that was not accepted by the postgresql server.

Forcing to use the same --db-user --db-password as the generic profile resolved the issue.

ltalirz commented 5 years ago

Just encountered another instance, where a sanity check on input variables would be useful:

When using yaml config files, you can put comments, e.g.

host_name: 123.123.123.123  # IP address

and we use this in the tutorial to let people what the values mean and where to replace things.

However, this is only considered a comment, when there is a space before the hash, i.e. the following is not a comment:

host_name: 123.123.123.123# IP address

A basic validation of the IP address would have caught this.

ltalirz commented 5 years ago

And another one, where it seems leaving the hostname for the postgres DB empty resulted in "NoneNone" ending up as the hostname in psycopg2.

return self.dbapi.connect(*cargs, **cparams)
 File "/home/tco/.virtualenvs/aiida/lib/python3.6/site-packages/psycopg2/__init__.py", line 126, in connect
   conn = _connect(dsn, connection_factory=connection_factory, **kwasync)
sqlalchemy.exc.OperationalError: (psycopg2.OperationalError) could not translate host name "NoneNone" to address: Name or service not known
File "/home/tco/.virtualenvs/aiida/lib/python3.6/site-packages/aiida/cmdline/commands/cmd_setup.py", line 90, in setup
   email=email, first_name=first_name, last_name=last_name, institution=institution
 File "/home/tco/.virtualenvs/aiida/lib/python3.6/site-packages/aiida/orm/users.py", line 47, in get_or_create
   return False, self.get(email=email)
 File "/home/tco/.virtualenvs/aiida/lib/python3.6/site-packages/aiida/orm/entities.py", line 120, in get
   res = self.find(filters=filters)
 File "/home/tco/.virtualenvs/aiida/lib/python3.6/site-packages/aiida/orm/entities.py", line 154, in find
   return [[0] for in query.all()]
 File "/home/tco/.virtualenvs/aiida/lib/python3.6/site-packages/aiida/orm/querybuilder.py", line 2204, in all
   return list(self.iterall(batch_size=batch_size))
 File "/home/tco/.virtualenvs/aiida/lib/python3.6/site-packages/aiida/orm/querybuilder.py", line 2161, in iterall
   for item in self._impl.iterall(query, batch_size, self._attrkeys_as_in_sql_result):
 File "/home/tco/.virtualenvs/aiida/lib/python3.6/site-packages/aiida/orm/implementation/django/querybuilder.py", line 450, in iterall
   for rowitem in results:
 File "/home/tco/.virtualenvs/aiida/lib/python3.6/site-packages/sqlalchemy/orm/query.py", line 3317, in __iter__