Merge topic 'file-download-handle-write-error'

9476245dcd file(DOWNLOAD): Handle write errors
1994393f7a cmCurl: Provide CURL_WRITEFUNC_ERROR for curl < 7.87

Acked-by: Kitware Robot <[email protected]>
Acked-by: buildbot <[email protected]>
Merge-request: !10507

Source/cmCurl.h

@@ -11,6 +11,11 @@
 
 #include <cm3p/curl/curl.h>
 
+// curl versions before 7.87.0 did not provide CURL_WRITEFUNC_ERROR
+#if defined(LIBCURL_VERSION_NUM) && LIBCURL_VERSION_NUM < 0x075700
+#  define CURL_WRITEFUNC_ERROR 0xFFFFFFFF
+#endif
+
 void cmCurlInitOnce();
 cm::optional<int> cmCurlParseTLSVersion(cm::string_view tls_version);
 cm::optional<std::string> cmCurlPrintTLSVersion(int curl_tls_version);

Source/cmFileCommand.cxx

@@ -1703,7 +1703,9 @@ size_t cmWriteToFileCallback(void* ptr, size_t size, size_t nmemb, void* data)
   cmsys::ofstream* fout = static_cast<cmsys::ofstream*>(data);
   if (fout) {
     char const* chPtr = static_cast<char*>(ptr);
-    fout->write(chPtr, realsize);
+    if (!fout->write(chPtr, realsize)) {
+      return CURL_WRITEFUNC_ERROR;
+    }
   }
   return realsize;
 }
@@ -2283,6 +2285,14 @@ bool HandleDownloadCommand(std::vector<std::string> const& args,
   g_curl.release();
   ::curl_easy_cleanup(curl);
 
+  // Explicitly close the file so we can check for write errors.
+  if (!file.empty()) {
+    fout.close();
+    if (!fout) {
+      res = CURLE_WRITE_ERROR;
+    }
+  }
+
   if (!statusVar.empty()) {
     std::string m = curl_easy_strerror(res);
     if ((res == CURLE_SSL_CONNECT_ERROR ||
@@ -2306,13 +2316,6 @@ bool HandleDownloadCommand(std::vector<std::string> const& args,
     status.GetMakefile().AddDefinition(logVar, chunkDebug.data());
   }
 
-  // Explicitly flush/close so we can measure the md5 accurately.
-  //
-  if (!file.empty()) {
-    fout.flush();
-    fout.close();
-  }
-
   // Verify MD5 sum if requested:
   //
   if (hash) {

Tests/RunCMake/file-DOWNLOAD/RunCMakeTest.cmake

@@ -23,6 +23,13 @@ run_cmake(no-file)
 run_cmake(range)
 run_cmake(SHOW_PROGRESS)
 
+foreach(file IN ITEMS /dev/full /dev/urandom)
+  if(IS_WRITABLE "${file}")
+    run_cmake_with_options(file-write-error -Dfile=${file})
+    break()
+  endif()
+endforeach()
+
 if(NOT CMake_TEST_NO_NETWORK)
   run_cmake(bad-hostname)
 endif()

Tests/RunCMake/file-DOWNLOAD/common.cmake

@@ -2,7 +2,9 @@ if(NOT "${CMAKE_CURRENT_SOURCE_DIR}" MATCHES "^/")
   set(slash /)
 endif()
 set(url "file://${slash}${CMAKE_CURRENT_SOURCE_DIR}/input.png")
-set(file ${CMAKE_CURRENT_BINARY_DIR}/output.png)
+if (NOT file)
+  set(file ${CMAKE_CURRENT_BINARY_DIR}/output.png)
+endif ()
 
 function(file_download)
   file(DOWNLOAD "${url}"

Tests/RunCMake/file-DOWNLOAD/file-write-error-stdout.txt

@@ -0,0 +1 @@
+-- status='23;"Failed writing received data to disk/application"'

Tests/RunCMake/file-DOWNLOAD/file-write-error.cmake

@@ -0,0 +1,3 @@
+include(common.cmake)
+
+file_download()