Skip to content

Loading…

Fix #2244 #2574

Merged
merged 3 commits into from

4 participants

@dkua

Reformatted the warning message as per the issue and changed some of the original logic to be more readable as advised by Fernando.

@fperez
IPython member

Test results for commit 614ba8c merged into master (ecf061e)
Platform: linux2

  • python2.7: OK (libraries not available: azure)
  • python3.2: OK (libraries not available: azure cython matplotlib oct2py pygments pymongo rpy2 wx wx.aui)

Not available for testing: python2.6

@fperez
IPython member

This looks good, but before we merge it, you should rebase it and remove commits 2928c78 and fe022e5 from the history, since they have nothing to do with this problem and they just undo one another, so they're just noise in the history.

@bfroehle bfroehle commented on the diff
IPython/frontend/html/notebook/notebookapp.py
@@ -470,11 +470,13 @@ def init_webapp(self):
ssl_options = None
self.web_app.password = self.password
self.http_server = httpserver.HTTPServer(self.web_app, ssl_options=ssl_options)
- if ssl_options is None and not self.ip and not (self.read_only and not self.password):
- self.log.critical('WARNING: the notebook server is listening on all IP addresses '
- 'but not using any encryption or authentication. This is highly '
- 'insecure and not recommended.')
-
+ if not self.ip:

You have accidentally changed the logic here so that it'll print warning messages almost always:

WARNING: The notebook server is listening on all IP addresses. This is highly insecure and not recommended.

I don't think that is intentional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bfroehle

For clarity: what situations do we consider insecure?

@fperez
IPython member

@bfroehle, good catch.

Insecure:

  • Public IP without SSL should prompt a warning.
  • Public IP in read-write mode and no password, also a warning.

And perhaps, as you mentioned in #2244, it should indeed be split into two messages...

@Carreau

The new logic seem fine to me and seem to do what Fernando asked.

@dkua, we don't get notified of new push, so we haven't seen your updates. Don't hesitate to ping us if we don't reply.

@Carreau

I'll go ahead and merge.

@Carreau Carreau merged commit 4e73029 into ipython:master
@minrk minrk added a commit that referenced this pull request
@minrk minrk Backport PR #2574: Fix #2244
Reformatted the warning message as per the issue and changed some of the original logic to be more readable as advised by Fernando.
f5f55a0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Showing with 8 additions and 5 deletions.
  1. +8 −5 IPython/frontend/html/notebook/notebookapp.py
View
13 IPython/frontend/html/notebook/notebookapp.py
@@ -470,11 +470,14 @@ def init_webapp(self):
ssl_options = None
self.web_app.password = self.password
self.http_server = httpserver.HTTPServer(self.web_app, ssl_options=ssl_options)
- if ssl_options is None and not self.ip and not (self.read_only and not self.password):
- self.log.critical('WARNING: the notebook server is listening on all IP addresses '
- 'but not using any encryption or authentication. This is highly '
- 'insecure and not recommended.')
-
+ if not self.ip:

You have accidentally changed the logic here so that it'll print warning messages almost always:

WARNING: The notebook server is listening on all IP addresses. This is highly insecure and not recommended.

I don't think that is intentional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ warning = "WARNING: The notebook server is listening on all IP addresses"
+ if ssl_options is None:
+ self.log.critical(warning + " and not using encryption. This"
+ "is not recommended.")
+ if not self.password and not self.read_only:
+ self.log.critical(warning + "and not using authentication."
+ "This is highly insecure and not recommended.")
success = None
for port in random_ports(self.port, self.port_retries+1):
try:
Something went wrong with that request. Please try again.