OTA-3716: Fallback to a full file download if a byte range request is…

… not supported by a server Signed-off-by: Mykhaylo Sul <myk.sul@gmail.com>
advancedtelematic · Oct 16, 2019 · 8131bed · 8131bed
1 parent e64e868
commit 8131bed
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 20 deletions.
diff --git a/src/libaktualizr/package_manager/packagemanagerinterface.cc b/src/libaktualizr/package_manager/packagemanagerinterface.cc
@@ -96,18 +96,19 @@ bool PackageManagerInterface::fetchTarget(const Uptane::Target& target, Uptane::
       LOG_WARNING << "Skipping download of target with length 0";
       return true;
     }
-    DownloadMetaStruct ds(target, std::move(progress_cb), token);
+    std::unique_ptr<DownloadMetaStruct> ds = std_::make_unique<DownloadMetaStruct>(target, progress_cb, token);
+    assert(ds);
     if (exists == TargetStatus::kIncomplete) {
       auto target_check = storage_->checkTargetFile(target);
-      ds.downloaded_length = target_check->first;
+      ds->downloaded_length = target_check->first;
       auto target_handle = storage_->openTargetFile(target);
-      ::restoreHasherState(ds.hasher(), target_handle.get());
+      ::restoreHasherState(ds->hasher(), target_handle.get());
       target_handle->rclose();
-      ds.fhandle = target_handle->toWriteHandle();
+      ds->fhandle = target_handle->toWriteHandle();
     } else {
       // If the target was found, but is oversized or the hash doesn't match,
       // just start over.
-      ds.fhandle = storage_->allocateTargetFile(false, target);
+      ds->fhandle = storage_->allocateTargetFile(false, target);
     }
 
     std::string target_url = target.uri();
@@ -117,17 +118,28 @@ bool PackageManagerInterface::fetchTarget(const Uptane::Target& target, Uptane::
 
     HttpResponse response;
     for (;;) {
-      response = http_->download(target_url, DownloadHandler, ProgressHandler, &ds,
-                                 static_cast<curl_off_t>(ds.downloaded_length));
+      response = http_->download(target_url, DownloadHandler, ProgressHandler, ds.get(),
+                                 static_cast<curl_off_t>(ds->downloaded_length));
+
+      if (response.curl_code == CURLE_RANGE_ERROR) {
+        LOG_WARNING << "The image server doesn't support byte range requests,"
+                       " try to download the image from the beginning: "
+                    << target_url;
+        ds = std_::make_unique<DownloadMetaStruct>(target, progress_cb, token);
+        assert(ds);
+        ds->fhandle = storage_->allocateTargetFile(false, target);
+        continue;
+      }
+
       if (!response.wasInterrupted()) {
         break;
       }
-      ds.fhandle.reset();
+      ds->fhandle.reset();
       // sleep if paused or abort the download
       if (!token->canContinue()) {
         throw Uptane::Exception("image", "Download of a target was aborted");
       }
-      ds.fhandle = storage_->openTargetFile(target)->toWriteHandle();
+      ds->fhandle = storage_->openTargetFile(target)->toWriteHandle();
     }
     LOG_TRACE << "Download status: " << response.getStatusStr() << std::endl;
     if (!response.isOk()) {
@@ -136,11 +148,11 @@ bool PackageManagerInterface::fetchTarget(const Uptane::Target& target, Uptane::
       }
       throw Uptane::Exception("image", "Could not download file, error: " + response.error_message);
     }
-    if (!target.MatchHash(Uptane::Hash(ds.hash_type, ds.hasher().getHexDigest()))) {
-      ds.fhandle->wabort();
+    if (!target.MatchHash(Uptane::Hash(ds->hash_type, ds->hasher().getHexDigest()))) {
+      ds->fhandle->wabort();
       throw Uptane::TargetHashMismatch(target.filename());
     }
-    ds.fhandle->wcommit();
+    ds->fhandle->wcommit();
     result = true;
   } catch (const Uptane::Exception& e) {
     LOG_WARNING << "Error while downloading a target: " << e.what();

diff --git a/tests/test_backend_failure.py b/tests/test_backend_failure.py
@@ -89,10 +89,7 @@ def test_backend_failure_sanity_imagerepo_update_after_metadata_download_failure
 @with_uptane_backend(start_generic_server=True)
 @with_images(images_to_install=[(('primary-hw-ID-001', 'primary-ecu-id'), 'primary-image.img')])
 @with_imagerepo(handlers=[
-                            # TODO: test fails because aktualizr issues byte range request
-                            # that are not supported by server
-                            # https://saeljira.it.here.com/browse/OTA-3716
-                            #DownloadInterruptionHandler(number_of_failures=1, url='/targets/primary-image.img'),
+                            DownloadInterruptionHandler(number_of_failures=1, url='/targets/primary-image.img'),
                             MalformedImageHandler(number_of_failures=1, url='/targets/primary-image.img'),
                         ])
 @with_director(start=False)
@@ -113,10 +110,7 @@ def test_backend_failure_sanity_imagerepo_update_after_image_download_failure(in
 """
 @with_uptane_backend(start_generic_server=True)
 @with_customrepo(handlers=[
-                            # TODO: This test fails because the issue with image download
-                            #  from a server that doesn't support byte range requests
-                            # DownloadInterruptionHandler(number_of_failures=1, url='/primary-image.img'),
-                            # https://saeljira.it.here.com/browse/OTA-3716
+                            DownloadInterruptionHandler(number_of_failures=1, url='/primary-image.img'),
                             MalformedImageHandler(number_of_failures=1, url='/primary-image.img')
                             # TODO: this test fails too, although httpclient.cc sets
                             # CURLOPT_LOW_SPEED_TIME and CURLOPT_LOW_SPEED_TIME