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

Don't start server if session_config.json is syntactically incorrect #1151

Merged
merged 1 commit into from
Nov 24, 2017

Conversation

whisperity
Copy link
Contributor

Fixes #1149.

If the clientside file is badly formatted, execution can still continue, because worst thing that can happen is that there is no automatic password login for the user, which will result in a valid prompt of "Please authenticate".

However, serverside, as @sylvestre pointed out, it's indeed dangerous to default back to unauthenticated and whatever mode.

Copy link
Contributor

@Xazax-hun Xazax-hun left a comment

Choose a reason for hiding this comment

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

LG! With one question.

@@ -748,6 +749,12 @@ def start_server(config_directory, package_data, port, db_conn_string,
.format(root_file))
root_sha = __make_root_file(root_file)

try:
manager = session_manager.SessionManager(root_sha, force_auth)
except (IOError, ValueError):
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the parens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, we don't.

@whisperity whisperity force-pushed the bailout-if-sessioncfg-bogus branch from 7073802 to f572483 Compare November 23, 2017 12:17
@whisperity whisperity requested a review from Xazax-hun November 23, 2017 12:17
Copy link
Member

@dkrupp dkrupp left a comment

Choose a reason for hiding this comment

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

LGTM

@dkrupp dkrupp merged commit 54492bc into Ericsson:master Nov 24, 2017
@whisperity whisperity deleted the bailout-if-sessioncfg-bogus branch November 30, 2017 14:16
@gyorb gyorb removed this from the release 6.3 milestone Dec 8, 2017
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.

5 participants