From deae1499ed66a732432876e412bf2f0d6208b4dd Mon Sep 17 00:00:00 2001
From: jhawkesworth <jhawkesworth@users.noreply.github.com>
Date: Mon, 10 Jul 2017 05:30:55 +0100
Subject: [PATCH] win_get_url improvements; allow dir for dest and helpful
 error if dest parent dir doesn't exist (#25453)

* win_get_url now allows dir for destination and gives helpful message if download parent dir does not exist

* fix yaml lint failure
---
 lib/ansible/modules/windows/win_get_url.ps1   | 20 +++++++++--
 .../targets/win_get_url/defaults/main.yml     |  8 +++--
 .../targets/win_get_url/tasks/main.yml        | 34 +++++++++++++++++--
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/lib/ansible/modules/windows/win_get_url.ps1 b/lib/ansible/modules/windows/win_get_url.ps1
index e278304f7d6..e13542f1bd3 100644
--- a/lib/ansible/modules/windows/win_get_url.ps1
+++ b/lib/ansible/modules/windows/win_get_url.ps1
@@ -51,7 +51,23 @@ if (-not $validate_certs) {
     [System.Net.ServicePointManager]::ServerCertificateValidationCallback = {$true}
 }
 
+
 Function Download-File($result, $url, $dest, $username, $password, $proxy_url, $proxy_username, $proxy_password) {
+    # use last part of url for dest file name if a directory is supplied for $dest
+    If ( Test-Path -PathType Container $dest ) {
+       $url_basename = Split-Path -leaf $url
+       If ( $url_basename.Length -gt 0 ) {
+          $dest = Join-Path -Path $dest -ChildPath $url_basename
+          $result.win_get_url.actual_dest = $dest
+       }
+    }
+    # check $dest parent folder exists before attempting download, which avoids unhelpful generic error message.
+    $dest_parent = Split-Path -Path $dest
+    $result.win_get_url.dest_parent = $dest_parent
+    If ( -not (Test-Path -Path $dest_parent -PathType Container)) {
+        $result.changed = $false
+        Fail-Json $result "The path '$dest_parent' does not exist for destination '$dest', or is not visible to the current user.  Ensure download destination folder exists (perhaps using win_file state=directory) before win_get_url runs."
+    }
     $webClient = New-Object System.Net.WebClient
     if($proxy_url) {
         $proxy_server = New-Object System.Net.WebProxy($proxy_url, $true)
@@ -75,11 +91,11 @@ Function Download-File($result, $url, $dest, $username, $password, $proxy_url, $
     Catch {
         Fail-Json $result "Error downloading $url to $dest $($_.Exception.Message)"
     }
-
 }
 
 
 If ($force -or -not (Test-Path -Path $dest)) {
+    
     Download-File -result $result -url $url -dest $dest -username $username -password $password -proxy_url $proxy_url -proxy_username $proxy_username -proxy_password $proxy_password
 }
 Else {
@@ -119,4 +135,4 @@ Else {
 
 }
 
-Exit-Json $result;
+Exit-Json $result
diff --git a/test/integration/targets/win_get_url/defaults/main.yml b/test/integration/targets/win_get_url/defaults/main.yml
index 28e5de659f6..c912ed24e82 100644
--- a/test/integration/targets/win_get_url/defaults/main.yml
+++ b/test/integration/targets/win_get_url/defaults/main.yml
@@ -1,6 +1,8 @@
 ---
 
-test_win_get_url_link: https://www.redhat.com
+test_win_get_url_host: www.redhat.com
+test_win_get_url_link: "https://{{test_win_get_url_host}}"
 test_win_get_url_invalid_link: https://www.redhat.com/skynet_module.html
-test_win_get_url_invalid_path: "Q:\\Filez\\Cyberdyne.html"
-test_win_get_url_path: "{{ test_win_get_url_dir_path }}\\docs_index.html"
\ No newline at end of file
+test_win_get_url_invalid_path: 'Q:\Filez\Cyberdyne.html'
+test_win_get_url_invalid_path_dir: 'Q:\Filez\'
+test_win_get_url_path: '{{ test_win_get_url_dir_path }}\docs_index.html'
diff --git a/test/integration/targets/win_get_url/tasks/main.yml b/test/integration/targets/win_get_url/tasks/main.yml
index 17482fcd4c6..9e818e318b1 100644
--- a/test/integration/targets/win_get_url/tasks/main.yml
+++ b/test/integration/targets/win_get_url/tasks/main.yml
@@ -99,7 +99,37 @@
   register: win_get_url_result_dir_path
   ignore_errors: true
 
-- name: check that the download failed if dest is a directory
+- name: check that the download did NOT fail, even though dest was directory
   assert:
     that:
-      - "win_get_url_result_dir_path|failed"
+      - "win_get_url_result_dir_path|changed"
+
+- name: test win_get_url with a valid url path and a dest that is a directory (from 2.4 should use url path as filename)
+  win_get_url:
+    url: "{{test_win_get_url_link}}"
+    dest: "{{test_win_get_url_dir_path}}"
+  register: win_get_url_result_dir_path_urlpath
+  ignore_errors: true
+
+- name: set expected destination path fact
+  set_fact:
+    expected_dest_path: '{{test_win_get_url_dir_path}}\{{test_win_get_url_host}}'
+
+- name: check that the download succeeded (changed) and dest is as expected
+  assert:
+    that:
+      - "win_get_url_result_dir_path_urlpath|changed"
+      - "win_get_url_result_dir_path_urlpath.win_get_url.actual_dest==expected_dest_path"
+
+- name: since 2.4 check you get a helpful message if the parent folder of the dest doesnt exist
+  win_get_url:
+    url: "{{test_win_get_url_link}}"
+    dest: "{{test_win_get_url_invalid_path_dir}}"
+  register: win_get_url_result_invalid_dest
+  ignore_errors: true
+
+- name: check if dest parent dir does not exist, module fails and you get a specific error message
+  assert:
+    that:
+      - "win_get_url_result_invalid_dest|failed"
+      - "win_get_url_result_invalid_dest.msg is search('does not exist')"