697765 - Mozharness' download_file() method uses os.path.basename on urls

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Andrew Halberstadt [:ahal]]

http://mxr.mozilla.org/build/source/mozharness/mozharness/base/script.py#145

1) This could lead to invalid file names if the url has parameters or anchors in it (e.g ? is reserved in windows).
2) I don't have a Windows environment handy, but I'm pretty sure this will just return the whole url on Windows.

Instead use something similar to:

import urlparse
parsed = urlparse.urlsplit(url)
if parsed.path != '':
    file_name = parsed.path.rstrip('/').rsplit('/',1)[1]
else:
    # handle no path
    file_name = "default"

Comment 1

[Aki Sasaki (not active)]

wfm. do you want to write a patch or should i?

Comment 2

[Andrew Halberstadt [:ahal]]

I can do it, probably get around to it tomorrow.

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.0 - Use urlparse instead of os.path.basename

Comment 4

[Aki Sasaki (not active)]

Comment on attachment 570286 [details] [diff] [review]
Patch 1.0 - Use urlparse instead of os.path.basename

We found that the logic here doesn't work perfectly if:

a) you are downloading from http://site.domain/ your file_name will be ''
b) we may have issues if you're downloading from http://site.domain:portnum on windows

Since we allow for specifying file_name when calling download_file(), and I doubt we'll ever really be downloading from the root of a domain on a regular basis, I'm going to r+ and land anyway.

Comment 5

[Aki Sasaki (not active)]

Comment on attachment 570286 [details] [diff] [review]
Patch 1.0 - Use urlparse instead of os.path.basename

http://hg.mozilla.org/build/mozharness/rev/ae752c6819bd