From f15516ff926d89c0853b1ce3262e5b70eecdb003 Mon Sep 17 00:00:00 2001 From: Adrien Destugues Date: Thu, 19 Nov 2020 23:01:58 +0100 Subject: [PATCH] Package Kit: re-use downloads from unfinished transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are three parts to this change: - In FetchFileJob, if the request fails with a timeout or IO error (probably because of unstable connection) attempt to resume the download with a range request. No limit on number of retries currently, maybe we should add one. - In PackageManager, before downloading a file, look around in other transaction directories in case it's already there. Partial and complete downloads are differentiated by an attribute which the fetch file job maintains. For complete downloads, no fetch job is scheduled, for partial downloads, the fetch job will request the remainder of the file. - In BHttpRequest, the implementation of SetRangeStart() and SetRangeEnd() have been added, along with some refactoring to handle listener notifications consistently. This also fixed a bug where the final notification for download progress was not emitted for compressed data. Fixes #12414. Change-Id: I3e285741ed0e5651594a7c2e1c7170644a9d297d Reviewed-on: https://review.haiku-os.org/c/haiku/+/3404 Reviewed-by: Stephan Aßmus Reviewed-by: Alex von Gluck IV --- headers/os/net/HttpRequest.h | 7 +- src/kits/network/libnetapi/HttpRequest.cpp | 114 +++++++++++++++----- src/kits/package/FetchFileJob.cpp | 52 ++++++--- src/kits/package/FetchUtils.cpp | 107 ++++++++++++++++++ src/kits/package/FetchUtils.h | 44 ++++++++ src/kits/package/Jamfile | 1 + src/kits/package/manager/PackageManager.cpp | 47 ++++++-- 7 files changed, 321 insertions(+), 51 deletions(-) create mode 100644 src/kits/package/FetchUtils.cpp create mode 100644 src/kits/package/FetchUtils.h diff --git a/headers/os/net/HttpRequest.h b/headers/os/net/HttpRequest.h index 0f0541155a..9b8d5df9d2 100644 --- a/headers/os/net/HttpRequest.h +++ b/headers/os/net/HttpRequest.h @@ -1,5 +1,5 @@ /* - * Copyright 2010-2013 Haiku Inc. All rights reserved. + * Copyright 2010-2021 Haiku Inc. All rights reserved. * Distributed under the terms of the MIT License. */ #ifndef _B_URL_PROTOCOL_HTTP_H_ @@ -95,6 +95,11 @@ private: // Utility methods bool _IsDefaultPort(); + // Listener notification + void _NotifyDataReceived(const char* data, + off_t pos, ssize_t length, + off_t bytesReceived, ssize_t bytesTotal); + private: bool fSSL; diff --git a/src/kits/network/libnetapi/HttpRequest.cpp b/src/kits/network/libnetapi/HttpRequest.cpp index 8b06851990..ef0786ee10 100644 --- a/src/kits/network/libnetapi/HttpRequest.cpp +++ b/src/kits/network/libnetapi/HttpRequest.cpp @@ -1,11 +1,12 @@ /* - * Copyright 2010-2015 Haiku Inc. All rights reserved. + * Copyright 2010-2021 Haiku Inc. All rights reserved. * Distributed under the terms of the MIT License. * * Authors: * Christophe Huriaux, c.huriaux@gmail.com * Niels Sascha Reedijk, niels.reedijk@gmail.com * Adrien Destugues, pulkomandy@pulkomandy.tk + * Stephan Aßmus, superstippi@gmx.de */ @@ -209,17 +210,36 @@ BHttpRequest::SetAutoReferrer(bool enable) void -BHttpRequest::SetHeaders(const BHttpHeaders& headers) +BHttpRequest::SetUserName(const BString& name) { - AdoptHeaders(new(std::nothrow) BHttpHeaders(headers)); + fOptUsername = name; } void -BHttpRequest::AdoptHeaders(BHttpHeaders* const headers) +BHttpRequest::SetPassword(const BString& password) { - delete fOptHeaders; - fOptHeaders = headers; + fOptPassword = password; +} + + +void +BHttpRequest::SetRangeStart(off_t position) +{ + // This field is used within the transfer loop, so only + // allow setting it before sending the request. + if (fRequestStatus == kRequestInitialState) + fOptRangeStart = position; +} + + +void +BHttpRequest::SetRangeEnd(off_t position) +{ + // This field could be used in the transfer loop, so only + // allow setting it before sending the request. + if (fRequestStatus == kRequestInitialState) + fOptRangeEnd = position; } @@ -230,6 +250,13 @@ BHttpRequest::SetPostFields(const BHttpForm& fields) } +void +BHttpRequest::SetHeaders(const BHttpHeaders& headers) +{ + AdoptHeaders(new(std::nothrow) BHttpHeaders(headers)); +} + + void BHttpRequest::AdoptPostFields(BHttpForm* const fields) { @@ -251,16 +278,10 @@ BHttpRequest::AdoptInputData(BDataIO* const data, const ssize_t size) void -BHttpRequest::SetUserName(const BString& name) +BHttpRequest::AdoptHeaders(BHttpHeaders* const headers) { - fOptUsername = name; -} - - -void -BHttpRequest::SetPassword(const BString& password) -{ - fOptPassword = password; + delete fOptHeaders; + fOptHeaders = headers; } @@ -740,17 +761,14 @@ BHttpRequest::_MakeRequest() ssize_t size = decompressorStorage.Size(); BStackOrHeapArray buffer(size); size = decompressorStorage.Read(buffer, size); - if (size > 0) { - fListener->DataReceived(this, buffer, bytesUnpacked, - size); - bytesUnpacked += size; - } + _NotifyDataReceived(buffer, bytesUnpacked, size, + bytesReceived, bytesTotal); + bytesUnpacked += size; } else if (bytesRead > 0) { - fListener->DataReceived(this, inputTempBuffer, - bytesReceived - bytesRead, bytesRead); + _NotifyDataReceived(inputTempBuffer, + bytesReceived - bytesRead, bytesRead, + bytesReceived, bytesTotal); } - fListener->DownloadProgress(this, bytesReceived, - std::max((off_t)0, bytesTotal)); } if (bytesTotal >= 0 && bytesReceived >= bytesTotal) @@ -768,11 +786,9 @@ BHttpRequest::_MakeRequest() ssize_t size = decompressorStorage.Size(); BStackOrHeapArray buffer(size); size = decompressorStorage.Read(buffer, size); - if (fListener != NULL && size > 0) { - fListener->DataReceived(this, buffer, - bytesUnpacked, size); - bytesUnpacked += size; - } + _NotifyDataReceived(buffer, bytesUnpacked, size, + bytesReceived, bytesTotal); + bytesUnpacked += size; } } } @@ -908,6 +924,20 @@ BHttpRequest::_SerializeHeaders() if (fOptReferer.CountChars() > 0) outputHeaders.AddHeader("Referer", fOptReferer.String()); + // Optional range requests headers + if (fOptRangeStart != -1 || fOptRangeEnd != -1) { + if (fOptRangeStart == -1) + fOptRangeStart = 0; + BString range; + if (fOptRangeEnd != -1) { + range.SetToFormat("bytes=%" B_PRIdOFF "-%" B_PRIdOFF, + fOptRangeStart, fOptRangeEnd); + } else { + range.SetToFormat("bytes=%" B_PRIdOFF "-", fOptRangeStart); + } + outputHeaders.AddHeader("Range", range.String()); + } + // Authentication if (fContext != NULL) { BHttpAuthentication& authentication = fContext->GetAuthentication(fUrl); @@ -1158,3 +1188,29 @@ BHttpRequest::_IsDefaultPort() } +void +BHttpRequest::_NotifyDataReceived(const char* data, off_t pos, ssize_t size, + off_t bytesReceived, ssize_t bytesTotal) +{ + if (fListener == NULL || size <= 0) + return; + if (fOptRangeStart > 0) { + pos += fOptRangeStart; + // bytesReceived and bytesTotal refer to the requested range, + // so that should technically not be adjusted for the range start. + // For displaying progress to the user, this is not ideal, though. + // But only for the case where we request the remainder of a file. + // Range requests can also be used to request any portion of a + // resource, so not modifying them is technically more correct. + // We can use a little trick, though: We know when the remainder + // is requested, because then fOptRangeEnd is -1. + if (fOptRangeEnd == -1) { + bytesReceived += fOptRangeStart; + if (bytesTotal > 0) + bytesTotal += fOptRangeStart; + } + } + fListener->DataReceived(this, data, pos, size); + fListener->DownloadProgress(this, bytesReceived, + std::max((off_t)0, bytesTotal)); +} diff --git a/src/kits/package/FetchFileJob.cpp b/src/kits/package/FetchFileJob.cpp index 07853d7cf0..50de8052ef 100644 --- a/src/kits/package/FetchFileJob.cpp +++ b/src/kits/package/FetchFileJob.cpp @@ -1,11 +1,12 @@ /* - * Copyright 2011-2015, Haiku, Inc. All Rights Reserved. + * Copyright 2011-2021, Haiku, Inc. All Rights Reserved. * Distributed under the terms of the MIT License. * * Authors: * Axel Dörfler * Rene Gollent * Oliver Tappe + * Stephan Aßmus */ @@ -22,6 +23,8 @@ # include #endif +#include "FetchUtils.h" + namespace BPackageKit { @@ -36,7 +39,7 @@ FetchFileJob::FetchFileJob(const BContext& context, const BString& title, inherited(context, title), fFileURL(fileURL), fTargetEntry(targetEntry), - fTargetFile(&targetEntry, B_CREATE_FILE | B_ERASE_FILE | B_WRITE_ONLY), + fTargetFile(&targetEntry, B_CREATE_FILE | B_WRITE_ONLY), fError(B_ERROR), fDownloadProgress(0.0) { @@ -90,13 +93,40 @@ FetchFileJob::Execute() if (result != B_OK) return result; - BUrlRequest* request = BUrlProtocolRoster::MakeRequest(fFileURL.String(), - this); - if (request == NULL) - return B_BAD_VALUE; + result = FetchUtils::SetFileType(fTargetFile, + "application/x-vnd.haiku-package"); + if (result != B_OK) { + fprintf(stderr, "failed to set file type for '%s': %s\n", + DownloadFileName(), strerror(result)); + } - thread_id thread = request->Run(); - wait_for_thread(thread, NULL); + do { + printf("downloading %s\n", DownloadURL()); + BUrlRequest* request = BUrlProtocolRoster::MakeRequest(DownloadURL(), + this); + if (request == NULL) + return B_BAD_VALUE; + + // Try to resume the download where we left off + off_t currentPosition; + BHttpRequest* http= dynamic_cast(request); + if (http != NULL && fTargetFile.GetSize(¤tPosition) == B_OK + && currentPosition > 0) { + printf("requesting range start %" B_PRIdOFF "\n", currentPosition); + http->SetRangeStart(currentPosition); + } + + thread_id thread = request->Run(); + wait_for_thread(thread, NULL); + } while (fError == B_IO_ERROR || fError == B_DEV_TIMEOUT); + + if (fError == B_OK) { + result = FetchUtils::MarkDownloadComplete(fTargetFile); + if (result != B_OK) { + fprintf(stderr, "failed to mark download '%s' as complete: %s\n", + DownloadFileName(), strerror(result)); + } + } return fError; } @@ -146,10 +176,8 @@ FetchFileJob::RequestCompleted(BUrlRequest* request, bool success) } switch (code) { case B_HTTP_STATUS_OK: - fError = B_OK; - break; case B_HTTP_STATUS_PARTIAL_CONTENT: - fError = B_PARTIAL_READ; + fError = B_OK; break; case B_HTTP_STATUS_REQUEST_TIMEOUT: case B_HTTP_STATUS_GATEWAY_TIMEOUT: @@ -198,7 +226,7 @@ FetchFileJob::FetchFileJob(const BContext& context, const BString& title, inherited(context, title), fFileURL(fileURL), fTargetEntry(targetEntry), - fTargetFile(&targetEntry, B_CREATE_FILE | B_ERASE_FILE | B_WRITE_ONLY), + fTargetFile(&targetEntry, B_CREATE_FILE | B_WRITE_ONLY), fDownloadProgress(0.0) { } diff --git a/src/kits/package/FetchUtils.cpp b/src/kits/package/FetchUtils.cpp new file mode 100644 index 0000000000..046d7f8d87 --- /dev/null +++ b/src/kits/package/FetchUtils.cpp @@ -0,0 +1,107 @@ +/* + * Copyright 2020, Haiku, Inc. All Rights Reserved. + * Distributed under the terms of the MIT License. + * + * Authors: + * Stephan Aßmus + */ + + +#include "FetchUtils.h" +#include "string.h" + +#include +#include +#include + +namespace BPackageKit { + +namespace BPrivate { + + +#ifdef HAIKU_TARGET_PLATFORM_HAIKU + +#define DL_COMPLETE_ATTR "Meta:DownloadCompleted" + + +/*static*/ bool +FetchUtils::IsDownloadCompleted(const char* path) +{ + BEntry entry(path, true); + BNode node(&entry); + return IsDownloadCompleted(node); +} + + +/*static*/ bool +FetchUtils::IsDownloadCompleted(BNode& node) +{ + bool isComplete; + status_t status = _GetAttribute(node, DL_COMPLETE_ATTR, + B_BOOL_TYPE, &isComplete, sizeof(isComplete)); + if (status != B_OK) { + // Most likely cause is that the attribute was not written, + // for example by previous versions of the Package Kit. + // Worst outcome of assuming a partial download should be + // a no-op range request. + isComplete = false; + } + return isComplete; +} + + +/*static*/ status_t +FetchUtils::MarkDownloadComplete(BNode& node) +{ + bool isComplete = true; + return _SetAttribute(node, DL_COMPLETE_ATTR, + B_BOOL_TYPE, &isComplete, sizeof(isComplete)); +} + + +/*static*/ status_t +FetchUtils::SetFileType(BNode& node, const char* type) +{ + return _SetAttribute(node, "BEOS:TYPE", + B_MIME_STRING_TYPE, type, strlen(type) + 1); +} + +status_t +FetchUtils::_SetAttribute(BNode& node, const char* attrName, + type_code type, const void* data, size_t size) +{ + if (node.InitCheck() != B_OK) + return node.InitCheck(); + + ssize_t written = node.WriteAttr(attrName, type, 0, data, size); + if (written != (ssize_t)size) { + if (written < 0) + return (status_t)written; + return B_IO_ERROR; + } + return B_OK; +} + + +status_t +FetchUtils::_GetAttribute(BNode& node, const char* attrName, + type_code type, void* data, size_t size) +{ + if (node.InitCheck() != B_OK) + return node.InitCheck(); + + ssize_t read = node.ReadAttr(attrName, type, 0, data, size); + if (read != (ssize_t)size) { + if (read < 0) + return (status_t)read; + return B_IO_ERROR; + } + return B_OK; +} + + +#endif // HAIKU_TARGET_PLATFORM_HAIKU + +} // namespace BPrivate + +} // namespace BPackageKit diff --git a/src/kits/package/FetchUtils.h b/src/kits/package/FetchUtils.h new file mode 100644 index 0000000000..9abb1682c9 --- /dev/null +++ b/src/kits/package/FetchUtils.h @@ -0,0 +1,44 @@ +/* + * Copyright 2020, Stephan Aßmus + * Distributed under the terms of the MIT License. + */ +#ifndef _PACKAGE__PRIVATE__FETCH_UTILS_H_ +#define _PACKAGE__PRIVATE__FETCH_UTILS_H_ + + +#include "SupportDefs.h" +#include + +namespace BPackageKit { + +namespace BPrivate { + + +class FetchUtils { +public: + static bool IsDownloadCompleted(const char* path); + static bool IsDownloadCompleted(BNode& node); + + static status_t MarkDownloadComplete(const char* path); + static status_t MarkDownloadComplete(BNode& node); + + static status_t SetFileType(BNode& node, const char* type); + +private: + static status_t _SetAttribute(BNode& node, + const char* attrName, + type_code type, const void* data, + size_t size); + static status_t _GetAttribute(BNode& node, + const char* attrName, + type_code type, void* data, + size_t size); +}; + + +} // namespace BPrivate + +} // namespace BPackageKit + + +#endif // _PACKAGE__PRIVATE__FETCH_UTILS_H_ diff --git a/src/kits/package/Jamfile b/src/kits/package/Jamfile index b4d2dce13a..9fb8563733 100644 --- a/src/kits/package/Jamfile +++ b/src/kits/package/Jamfile @@ -84,6 +84,7 @@ for architectureObject in [ MultiArchSubDirSetup ] { DownloadFileRequest.cpp DropRepositoryRequest.cpp FetchFileJob.cpp + FetchUtils.cpp InstallationLocationInfo.cpp Job.cpp PackageInfo.cpp diff --git a/src/kits/package/manager/PackageManager.cpp b/src/kits/package/manager/PackageManager.cpp index 6c9908dbdb..f3607b01ad 100644 --- a/src/kits/package/manager/PackageManager.cpp +++ b/src/kits/package/manager/PackageManager.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015, Haiku, Inc. All Rights Reserved. + * Copyright 2013-2020, Haiku, Inc. All Rights Reserved. * Distributed under the terms of the MIT License. * * Authors: @@ -10,6 +10,8 @@ #include +#include + #include #include #include @@ -31,6 +33,7 @@ #include #include "FetchFileJob.h" +#include "FetchUtils.h" #include "PackageManagerUtils.h" #undef B_TRANSLATION_CONTEXT @@ -38,6 +41,7 @@ using BPackageKit::BPrivate::FetchFileJob; +using BPackageKit::BPrivate::FetchUtils; using BPackageKit::BPrivate::ValidateChecksumJob; @@ -560,15 +564,40 @@ BPackageManager::_PreparePackageChanges( RemoteRepository* remoteRepository = dynamic_cast(package->Repository()); if (remoteRepository != NULL) { - // download the package - BString url = remoteRepository->Config().PackagesURL(); - url << '/' << fileName; + // first check if the package already exists in a previous + // transaction + bool alreadyDownloaded = false; + BPath path(&transaction->TransactionDirectory()); + BPath parent; + if (path.GetParent(&parent) == B_OK) { + BString globPath = parent.Path(); + globPath << "/*/" << fileName; + glob_t globbuf; + if (glob(globPath.String(), 0, NULL, &globbuf) == 0) { + path.Append(fileName); + if (BCopyEngine().CopyEntry(globbuf.gl_pathv[0], + path.Path()) == B_OK) { + alreadyDownloaded = FetchUtils::IsDownloadCompleted( + path.Path()); + printf("Re-using download '%s' from previous " + "transaction%s\n", globbuf.gl_pathv[0], + alreadyDownloaded ? "" : " (partial)"); + } + } + } - status_t error = DownloadPackage(url, entry, - package->Info().Checksum()); - if (error != B_OK) - DIE(error, "Failed to download package %s", - package->Info().Name().String()); + if (!alreadyDownloaded) { + // download the package + BString url = remoteRepository->Config().PackagesURL(); + url << '/' << fileName; + + status_t error = DownloadPackage(url, entry, + package->Info().Checksum()); + if (error != B_OK) { + DIE(error, "Failed to download package %s", + package->Info().Name().String()); + } + } } else if (package->Repository() != &installationRepository) { // clone the existing package LocalRepository* localRepository