--- a/src/client.py	Tue Apr 15 12:38:49 2008 -0700
+++ b/src/client.py	Tue Apr 15 15:13:20 2008 -0700
@@ -60,6 +60,7 @@
 import pkg.client.progress as progress
 import pkg.client.bootenv as bootenv
 import pkg.fmri as fmri
+import pkg.misc as misc
 
 def usage(usage_error = None):
         """ Emit a usage message and optionally prefix it with a more
@@ -339,13 +340,22 @@
         if verbose and quiet:
                 usage(_("image-update: -v and -q may not be combined"))
 
+        progresstracker = get_tracker(quiet)
+        img.load_catalogs(progresstracker)
+
+        try:
+                img.retrieve_catalogs()
+        except RuntimeError, failures:
+                if display_catalog_failures(failures) == 0:
+                        return 1
+                else:
+                        return 3
         try:
                 be = bootenv.BootEnv(img.get_root())
         except RuntimeError:
                 be = bootenv.BootEnvNull(img.get_root())
 
-        progresstracker = get_tracker(quiet)
-
+        # Reload catalog.  This picks up the update from retrieve_catalogs.
         img.load_catalogs(progresstracker)
 
         pkg_list = [ ipkg.get_pkg_stem() for ipkg in img.gen_installed_pkgs() ]
@@ -853,13 +863,10 @@
 
         # XXX will need to show available content series for each package
         full_refresh = False
-        try:
-                opts, pargs = getopt.getopt(args, None, ["full"])
-                for opt, arg in opts:
-                        if opt == "--full":
-                                full_refresh = True
-        except getopt.GetoptError, e:
-                usage("refresh: illegal option -- %s" % e.opt)
+        opts, pargs = getopt.getopt(args, "", ["full"])
+        for opt, arg in opts:
+                if opt == "--full":
+                        full_refresh = True
 
         # Ensure Image directory structure is valid.
         if not os.path.isdir("%s/catalog" % img.imgdir):
@@ -918,10 +925,19 @@
                             ) % ssl_cert)
                         return 1
 
+
         if not img.has_authority(auth) and origin_url == None:
                 error(_("set-authority: must define origin URL for new authority"))
                 return 1
 
+        elif not img.has_authority(auth) and not misc.valid_auth_prefix(auth):
+                error(_("set-authority: authority name has invalid characters"))
+                return 1
+
+        if origin_url and not misc.valid_auth_url(origin_url):
+                error(_("set-authority: authority URL is invalid"))
+                return 1
+
         img.set_authority(auth, origin_url = origin_url, ssl_key = ssl_key,
             ssl_cert = ssl_cert)
 
@@ -1067,9 +1083,17 @@
                     "the form '<prefix>=<url>'."))
 
         if auth_name.startswith(fmri.PREF_AUTH_PFX):
-                print >> sys.stderr, \
-                    _("pkg: image-create requires that a prefix not match: %s"
-                        % fmri.PREF_AUTH_PFX)
+                error(_("image-create requires that a prefix not match: %s"
+                        % fmri.PREF_AUTH_PFX))
+                return 1
+
+        if not misc.valid_auth_prefix(auth_name):
+                error(_("image-create: authority prefix has invalid characters"))
+                return 1
+
+        if not misc.valid_auth_url(auth_url):
+                error(_("image-create: authority URL is invalid"))
+                return 1 
 
         try:
                 img.set_attrs(imgtype, pargs[0], is_zone, auth_name, auth_url,

--- a/src/modules/client/image.py	Tue Apr 15 12:38:49 2008 -0700
+++ b/src/modules/client/image.py	Tue Apr 15 15:13:20 2008 -0700
@@ -771,7 +771,6 @@
                 total = 0
                 succeeded = 0
                 cat = None
-                full_refresh = False
                 ts = 0
 
                 for auth in self.gen_authorities():

--- a/src/modules/client/retrieve.py	Tue Apr 15 12:38:49 2008 -0700
+++ b/src/modules/client/retrieve.py	Tue Apr 15 15:13:20 2008 -0700
@@ -42,6 +42,9 @@
         try:
                 f, v = versioned_urlopen(url_prefix, "file", [0], hash,
                            ssl_creds = ssl_tuple, imgtype = img.type)
+        except urllib2.HTTPError, e:
+                raise NameError, "could not retrieve file '%s' from '%s'" % \
+                    (hash, url_prefix)
         except urllib2.URLError, e:
                 if len(e.args) == 1 and isinstance(e.args[0], socket.sslerror):
                         raise RuntimeError, e
@@ -67,6 +70,9 @@
                 m, v = versioned_urlopen(url_prefix, "manifest", [0],
                     fmri.get_url_path(), ssl_creds = ssl_tuple,
                     imgtype = img.type)
+        except urllib2.HTTPError, e:
+                raise NameError, "could not retrieve file '%s' from '%s'" % \
+                    (hash, url_prefix)
         except urllib2.URLError, e:
                 if len(e.args) == 1 and isinstance(e.args[0], socket.sslerror):
                         raise RuntimeError, e

--- a/src/modules/misc.py	Tue Apr 15 12:38:49 2008 -0700
+++ b/src/modules/misc.py	Tue Apr 15 15:13:20 2008 -0700
@@ -29,6 +29,7 @@
 import urlparse
 import httplib
 import platform
+import re
 
 import pkg.urlhelpers as urlhelpers
 import pkg.portable as portable
@@ -111,3 +112,43 @@
                 raise RuntimeError, \
                     "%s doesn't speak a known version of %s operation" % \
                     (base_uri, operation)
+
+
+_hostname_re = re.compile("^[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9]+\.?)*$")
+_invalid_host_chars = re.compile(".*[^a-zA-Z0-9\-\.]+")
+_valid_proto = ["http", "https"]
+
+def valid_auth_prefix(prefix):
+        """Verify that the authority prefix only contains valid characters."""
+
+        # This is a workaround for the the hostname_re being slow when
+        # it comes to finding invalid characters in the prefix string.
+
+        if _invalid_host_chars.match(prefix):
+                # prefix bad chars
+                return False
+
+        if _hostname_re.match(prefix):
+                return True
+
+        return False
+
+def valid_auth_url(url):
+        """Verify that the authority URL contains only valid characters."""
+
+        # First split the URL and check if the scheme is one we support
+        o = urlparse.urlsplit(url)
+
+        if not o[0] in _valid_proto:
+                return False
+
+        # Next verify that the network location is valid
+        host, port = urllib.splitport(o[1])
+
+        if not host or _invalid_host_chars.match(host):
+                return False
+
+        if _hostname_re.match(host):
+                return True
+
+        return False

--- a/src/tests/cli/t_commandline.py	Tue Apr 15 12:38:49 2008 -0700
+++ b/src/tests/cli/t_commandline.py	Tue Apr 15 15:13:20 2008 -0700
@@ -54,6 +54,9 @@
 
         def test_pkg_vq_1153(self):
                 """ test that -v and -q are mutually exclusive """
+                durl = self.dc.get_depot_url()
+                self.image_create(durl)
+
                 self.pkg("verify -vq", exit=2)
                 self.pkg("install -vq foo", exit=2)
                 self.pkg("uninstall -vq foo", exit=2)
@@ -74,7 +77,7 @@
                 self.pkg("set-authority -c", exit=2)
                 self.pkg("set-authority -O", exit=2)
                 self.pkg("unset-authority", exit=2)
-
+                self.pkg("refresh -F", exit=2)
 
         def test_pkgsend_bogus_opts(self):
                 """ pkgsend bogus option checks """
@@ -124,6 +127,18 @@
                 self.pkg("authority test3", exit=1)
                 self.pkg("authority -H | grep URL", exit=1)
 
+        def test_authority_validation(self):
+                """Verify that we catch poorly formed auth prefixes and URL"""
+                durl = self.dc.get_depot_url()
+                self.image_create(durl)
+
+                self.pkg("set-authority -O http://test1 test1")
+
+                self.pkg("set-authority -O http://test2 $%^8", exit=1)
+                self.pkg("set-authority -O http://test2 8^$%", exit=1)
+                self.pkg("set-authority -O http://*^5$% test2", exit=1)
+                self.pkg("set-authority -O http://test1:abcde test2", exit=1)
+                self.pkg("set-authority -O ftp://test2 test2", exit=1)
 
 if __name__ == "__main__":
         unittest.main()

--- a/src/tests/cli/t_image_create.py	Tue Apr 15 12:38:49 2008 -0700
+++ b/src/tests/cli/t_image_create.py	Tue Apr 15 15:13:20 2008 -0700
@@ -88,5 +88,20 @@
                 self.assertRaises(testutils.UnexpectedExitCodeException, \
                     self.pkg, "image-create -a foo")
 
+        def test_bad_authority_options(self):
+                """More tests that abuse the authority prefix and URL."""
+
+                self.assertRaises(testutils.UnexpectedExitCodeException, \
+                    self.pkg, "image-create -a $%^8=http://test1")
+
+                self.assertRaises(testutils.UnexpectedExitCodeException, \
+                    self.pkg, "image-create -a test1=http://$%^8")
+
+                self.assertRaises(testutils.UnexpectedExitCodeException, \
+                    self.pkg, "image-create -a test1=http://test1:abcde")
+
+                self.assertRaises(testutils.UnexpectedExitCodeException, \
+                    self.pkg, "image-create -a test1=ftp://test1")
+
 if __name__ == "__main__":
         unittest.main()