Re: [meta-swupd] allow username/password encoded in SWUPD_VERSION_URL and SWUPD_CONTENT_URL

[Patrick Ohly <patrick.ohly@intel.com>]

Hello Ingo!

Sorry for the late reply. There were quite a few things in the patch
that needed further discussion, so I kept postponing dealing with it.
That, and I am not sure due to staffing questions whether I am really
supposed to maintain meta-swupd at the moment :-/

Right now I refrain from applying patches to it until that gets
clarified.

On Sat, 2017-03-25 at 21:14 +0100, Ingo Flaschberger wrote:
> requested patch attached

Instead of attaching patches, please use "git send-email" to send the
patch directly. I needs a proper commit message, too:
https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines#Patch_Headers_and_Commit_Messages

> From f2526a7ed47b3f3c8f0cb893eadb5e6981255d4c Mon Sep 17 00:00:00
> 2001
> From: ingo <ingo.flaschberger@gmail.com>
> Date: Sat, 25 Mar 2017 21:13:33 +0100
> Subject: [PATCH] bundles.py: allow username/password encoded into
> HTTP server
>  URLs example: https://user:password@server/path
> 
> ---
>  lib/swupd/bundles.py | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/lib/swupd/bundles.py b/lib/swupd/bundles.py
> index b4c6f49..223fd3c 100644
> --- a/lib/swupd/bundles.py
> +++ b/lib/swupd/bundles.py
> @@ -4,6 +4,8 @@ import subprocess
>  import shutil
>  import urllib.request
>  import urllib.error
> +import urllib.parse
> +import re
>  from bb.utils import export_proxies
>  from oe.package_manager import RpmPM
>  from oe.package_manager import OpkgPM
> @@ -164,6 +166,15 @@ def download_manifests(content_url, version,
> component, to_dir):
>      base_versions = set()
>      if not os.path.exists(target):
>          bb.debug(1, 'Downloading %s -> %s' % (source, target))
> +        parsed_source = urllib.parse.urlsplit(source)
> +        if( parsed_source.username != None):
> +            new_source = ( parsed_source.scheme, re.sub( re.escape(
> parsed_source.username+':'+parsed_source.password+'@'),
> '',parsed_source.netloc), parsed_source.path, parsed_source.query,
> parsed_source.fragment)

Wouldn't it be simpler to do this?
   parsed_source.netloc = parsed_source.hostname

We want the original URL, just with a simpler netloc part. Mucking
around with a regex to achieve that seems overly complicated when
urllib.parse() has already done the parsing for us.

> +            source = urllib.parse.urlunsplit( new_source)
> +            manager =
> urllib.request.HTTPPasswordMgrWithDefaultRealm()
> +            manager.add_password(None, new_source,
> parsed_source.username, parsed_source.password)
> +            authHandler =
> urllib.request.HTTPBasicAuthHandler(manager)
> +            opener = urllib.request.build_opener(authHandler)
> +            urllib.request.install_opener(opener)

This opener gets installed over and over again, each time some URL
derived from content_url is used. Isn't it enough to check content_url
once and then use a simpler version of it for constructing URLs?

>          response = urllib.request.urlopen(source)
>          archive = response.read()
>          bb.utils.mkdirhier(to_dir)
> @@ -228,6 +239,15 @@ def download_old_versions(d):
>      for format in range(3, current_format + 1):
>          try:
>              url = '%s/version/format%d/latest' % (content_url,
> format)
> +            parsed_url = urllib.parse.urlsplit(url)
> +            if( parsed_url.username != None):
> +                new_url = ( parsed_url.scheme, re.sub( re.escape(
> parsed_url.username+':'+parsed_url.password+'@'),
> '',parsed_url.netloc), parsed_url.path, parsed_url.query,
> parsed_url.fragment)
> +                url = urllib.parse.urlunsplit( new_url)
> +                manager =
> urllib.request.HTTPPasswordMgrWithDefaultRealm()
> +                manager.add_password(None, new_url,
> parsed_url.username, parsed_url.password)
> +                authHandler =
> urllib.request.HTTPBasicAuthHandler(manager)
> +                opener = urllib.request.build_opener(authHandler)
> +                urllib.request.install_opener(opener)

Cut-and-paste... this should be in a helper function.

I tried to come up with a cleaner patch that implements the same
behavior. But I don't have a way to test it. Can you perhaps try out
the patch that I will post as a followup?

Note that it applies cleanly only on top of
https://github.com/pohly/meta-swupd/tree/master

You can also check out the patch from
https://github.com/pohly/meta-swupd/tree/basic_auth

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.