New packages: paste link instead of uploading a DMG

[mkuron]

When adding a new package, it should be possible to optionally paste a link to a DMG instead of uploading the DMG. MunkiServer should then download the DMG from that link and treat it as if it were uploaded directly.

[jnraine]

Thanks for the suggestion Michael. Sounds like a good idea -- avoids the obligatory download then upload when creating a package.

[mkuron]

I've been testing this for a few months now and it works quite nicely. Could you please merge the pull request?

[jnraine]

Thanks for the commits Michael.

[mkuron]

Thanks for cleaning up my code. As far as I can tell, your changes break in one situation that my original code didn't:
If the URL does a HTTP redirect to the actual DMG, the filename is not correct (might be something like download.php instead of Application.dmg). That's why I used u.base_uri.request_uri, which contains the URL after following any Location headers. So I propose the following to replace line 148:

if defined? u.base_uri
  file_url_path = u.base_uri.request_uri
else
  file_url_path = URI.parse(url).path
end

[jnraine]

Ooooooh that's what that was doing! :)

Thanks for the review. I'll make the change shortly.

Jordan

On Wednesday, 8 May, 2013 at 6:48 AM, Michael Kuron wrote:

Thanks for cleaning up my code. As far as I can tell, your changes break in one situation that my original code didn't:
If the URL does a HTTP redirect to the actual DMG, the filename is not correct (might be something like download.php instead of Application.dmg). That's why I used u.base_uri.request_uri, which contains the URL after following any Location headers. So I propose the following to replace line 148 (16126c0#L1R148):
if defined? u.base_uri file_url_path = u.base_uri.request_uri else file_url_path = URI.parse(url).path end

—
Reply to this email directly or view it on GitHub (#168 (comment)).

[jnraine]

Checkout aeb10f0 for the addition. I went with a ||= assignment instead of an if statement and opted for respond_to? instead of defined. Do you know of a redirecting download URL to test this use case with?

[mkuron]

Here's one: http://download.mozilla.org/?product=firefox-20.0&os=osx&lang=en-US and it works just fine.

On a side note (not trivially fixable, but no big deal): the official link from Mozilla is https, but open-uri doesn't follow redirects from https to http. If you try to enter the https link, the resulting error message is

A problem occurred while processing package upload: Download failed: redirection forbidden: https://download.mozilla.org/?product=firefox-20.0&os=osx&lang=en-US -> http://download.cdn.mozilla.net/pub/mozilla.org/firefox/releases/20.0/mac/en-US/Firefox%2020.0.dmg

[jnraine]

Mmm, good find. Since the server catches the error and gives a reasonable message to the user, I'm fine with the current implementation. Do you agree?

[mkuron]

Sure. The message even contains the redirect URL, so it's just a matter of copy/paste-ing.