[Yum-devel] Bug in urlgrabber _parse_url

[Jim Cain]

On python 2.4.3 with yum 2.4.1 and urlgrabber 2.9.6 ond Fedora Core 4,
I was getting a 401 error with a repository that uses authorization
and a non-standard port. Previous versions were working okay. In this
case, yum was not passing along the auth info to the server, resulting
in the 401.

After some digging, I discovered the error is in the way that
urlgrabber uses urllib2. In the add_password call, an url is expected
as the second parameter, but urlgrabber passes the hostname. It looks
like this became a problem when urllib2 recently made a change to the
parameters it uses internally.

In any case, here is a quick and dirty fix to grabber.py. The
urlunparse call is repeated a few lines later, so it would probably be
better to rearrange the logic. add_password() wants an url as the
second parameter, without auth info, so that's why I reassemble the
url there. I was too lazy to come up with new logic. :)

*** grabber.py.orig     2005-03-02 19:54:23.000000000 -0500
--- grabber.py  2006-07-07 16:51:42.000000000 -0400
***************
*** 687,693 ****
              except ValueError, e:
                  raise URLGrabError(1, _('Bad URL: %s') % url)
              if DEBUG: print 'adding HTTP auth: %s, %s' % (user, password)
!             auth_handler.add_password(None, host, user, password)
          parts = (scheme, host, path, parm, query, frag)
          url = urlparse.urlunparse(parts)
          return url, parts
--- 687,693 ----
              except ValueError, e:
                  raise URLGrabError(1, _('Bad URL: %s') % url)
              if DEBUG: print 'adding HTTP auth: %s, %s' % (user, password)
!             auth_handler.add_password(None,
urlparse.urlunparse((scheme, host, path, parm, query, frag)), user,
password)
          parts = (scheme, host, path, parm, query, frag)
          url = urlparse.urlunparse(parts)
          return url, parts

If anyone cares, here is the relevant chunk of a diff between
urllib2.py in 2.4.1 and 2.4.3:

***************
*** 720,726 ****
                      return self.retry_http_basic_auth(host, req, realm)

      def retry_http_basic_auth(self, host, req, realm):
!         user,pw = self.passwd.find_user_password(realm, host)
          if pw is not None:
              raw = "%s:%s" % (user, pw)
              auth = 'Basic %s' % base64.encodestring(raw).strip()
--- 720,726 ----
                      return self.retry_http_basic_auth(host, req, realm)

      def retry_http_basic_auth(self, host, req, realm):
!         user, pw = self.passwd.find_user_password(realm, req.get_full_url())
          if pw is not None:
              raw = "%s:%s" % (user, pw)
              auth = 'Basic %s' % base64.encodestring(raw).strip()

You can see that it now passes the url to find_user_password() instead
of the host. This is probably what broke urlgrabber's use of host in
add_password().