PATCH: Strip hostname of ,'s

[Stephen Shirley]

diff --git a/lib/config.py b/lib/config.py
index bbe0a0c..4161287 100644
--- a/lib/config.py
+++ b/lib/config.py
@@ -92,7 +92,7 @@ class Config(object):

"""
if _hostname is None:
- _hostname = socket.gethostname()
+ _hostname = utils.GetHostname()

cfg = _ReadConfig(filename)

diff --git a/lib/node.py b/lib/node.py
index 536d4d6..26582c7 100644
--- a/lib/node.py
+++ b/lib/node.py
@@ -61,10 +61,6 @@ CMD_GET_SHADOW_COOKIE = "getshadowcookie"
PROTO_SEPARATOR = "\0"

-def GetHostname():
- return socket.getfqdn()
-
-
def _GetUserShell(username):
return pwd.getpwnam(username).pw_shell

@@ -116,7 +112,7 @@ class NodeSession(session.SessionBase):
def __init__(self, ctx, clientargs, _env=None):
self._ctx = ctx

- hostname = GetHostname()
+ hostname = utils.GetHostname()
display = FindUnusedDisplay()

session.SessionBase.__init__(self, ctx.sessid, hostname, display,
diff --git a/lib/utils.py b/lib/utils.py
index 24f4e8d..df2c699 100644
--- a/lib/utils.py
+++ b/lib/utils.py
@@ -32,6 +32,7 @@ import pwd
import resource
import re
import signal
+import socket
import sys
import syslog
import tempfile
@@ -877,3 +878,10 @@ def GetCurrentUserName():

"""
return pwd.getpwuid(os.getuid())[0]
+
+def GetHostname():
+ """Returns hostname, minus illegal characters.
+
+ """
+ host = socket.getfqdn()
+ return host.replace(",", "")

Steve
--
"You are technically correct, the best kind of correct."
- Bureaucrat 1.0, Futurama

[Michael Hanselmann]

2009/10/24 Stephen Shirley <kor...@gmail.com>:

> +def GetHostname():
> +  """Returns hostname, minus illegal characters.
> +
> +  """
> +  host = socket.getfqdn()
> +  return host.replace(",", "")

Why would a hostname include commas and/or what problem is fixed by this?

Thanks,
Michael

[Stephen Shirley]

See http://code.google.com/p/neatx/issues/detail?id=34 - a mistake in
/etc/hosts caused there to be a comma in the hostname, and nxagent
barfed when parsing options because of it.

[Michael Hanselmann]

2009/12/1 Stephen Shirley <kor...@gmail.com>:

> On Mon, Oct 26, 2009 at 11:07, Michael Hanselmann <han...@google.com> wrote:
>> 2009/10/24 Stephen Shirley <kor...@gmail.com>:
>>> +def GetHostname():

>>> +  host = socket.getfqdn()
>>> +  return host.replace(",", "")
>>
>> Why would a hostname include commas and/or what problem is fixed by this?
>
> See http://code.google.com/p/neatx/issues/detail?id=34 - a mistake in
> /etc/hosts caused there to be a comma in the hostname, and nxagent
> barfed when parsing options because of it.

Then the function should rather throw an exception than just silently
drop the comma. The function building the NX options string should
also check for no commas in values.

Regards,
Michael

[Stephen Shirley]

On Wed, Dec 2, 2009 at 11:32, Michael Hanselmann <han...@google.com> wrote:
> Then the function should rather throw an exception than just silently
> drop the comma. The function building the NX options string should
> also check for no commas in values.

Good point. Actually, if we do the latter, we don't need this function
at all. I'll put a patch together to do , checking.

[Stephen Shirley]

On Sun, Dec 6, 2009 at 17:08, Stephen Shirley <kor...@gmail.com> wrote:
> Good point. Actually, if we do the latter, we don't need this function
> at all. I'll put a patch together to do , checking.

commit 2d21eab49228ceae0f0135672113443a426a0a0d
Author: Stephen Shirley <dia...@nonado.net>
Date: Sun Dec 6 20:25:19 2009 +0100

Disallow illegal commas in options

Also treat sess.display as an int, so it's value cannot contain a comma.

diff --git a/lib/agent.py b/lib/agent.py
index e304f31..7ba59ad 100644
--- a/lib/agent.py
+++ b/lib/agent.py
@@ -491,7 +491,7 @@ class NxAgentProgram(daemon.Program):
formatted = ",".join(["%s=%s" % (name, value)
for name, value in opts.iteritems()])

- return "nx/nx,%s:%s\n" % (formatted, sess.display)
+ return "nx/nx,%s:%d\n" % (formatted, sess.display)

def _GetDisplayWithOptions(self):
"""Returns the value for the DISPLAY variable for nxagent.
@@ -499,7 +499,9 @@ class NxAgentProgram(daemon.Program):
"""
sess = self._ctx.session

- return "nx/nx,options=%s:%s" % (sess.optionsfile, sess.display)
+ self.__CheckStrChars(sess.optionsfile, "Session options file")
+
+ return "nx/nx,options=%s:%d" % (sess.optionsfile, sess.display)

def _GetOptions(self):
"""Returns session options for nxagent.
@@ -592,7 +594,7 @@ class NxAgentProgram(daemon.Program):
# nxagent port).
"-nolisten", "tcp",

- ":%s" % sess.display,
+ ":%d" % sess.display,
]

if sess.type == constants.SESS_TYPE_SHADOW:
@@ -615,6 +617,7 @@ class NxAgentProgram(daemon.Program):
"""
sess = self._ctx.session
filename = sess.optionsfile
+ self.__CheckOptsChars(opts)
formatted = self._FormatNxAgentOptions(opts)

logging.debug("Writing session options %r to %s", formatted, filename)
@@ -622,3 +625,30 @@ class NxAgentProgram(daemon.Program):

def __EmitDisplayReady(self):
self.emit(self.DISPLAY_READY_SIGNAL)
+
+ def __CheckOptsChars(self, opts):
+ """Checks to make sure option name/values don't contain illegal characters.
+
+ @type opts: dict
+ @param opts: Options
+
+ """
+
+ for name, value in opts.iteritems():
+ self.__CheckStrChars(name, "Name of option %r" % name)
+ self.__CheckStrChars(value, "Value of option %r (%r)" % (name, value))
+
+ def __CheckStrChars(self, s, description):
+ """Checks to make sure string don't contain illegal characters.
+
+ @type s: string
+ @param s: text to test
+ @type description: string
+ @param description: description of text
+
+ """
+ illegal_chars = [","]
+ for c in illegal_chars:
+ if c in s:
+ raise errors.IllegalCharacterError("%s contains illegal character %r" %
+ (description, c))
diff --git a/lib/errors.py b/lib/errors.py
index 8b6efd8..2c12f9c 100644
--- a/lib/errors.py
+++ b/lib/errors.py
@@ -81,6 +81,11 @@ class SessionParameterError(GenericError):

"""

+class IllegalCharacterError(GenericError):
+ """String contains illegal character (e.g. a comma in session options).
+
+ """
+

# Exception classes should be added above

[Michael Hanselmann]

2009/12/6 Stephen Shirley <kor...@gmail.com>:

>    Also treat sess.display as an int, so it's value cannot contain a comma.

s/it's/its/, rest LGTM