From 28c0a27ddf03dcf07a11c291699428a32f381fbc Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Fri, 18 Feb 2022 17:56:31 -0600 Subject: Handle missing content length with range request and empty files. Add test for server without range request support. --- src/libs/dutil/WixToolset.DUtil/dlutil.cpp | 46 ++++++++++-- .../burn/TestData/CacheTests/BundleC/BundleC.wxs | 2 +- src/test/burn/WixToolsetTest.BurnE2E/CacheTests.cs | 81 ++++++++++++++++++++-- src/test/burn/WixToolsetTest.BurnE2E/IWebServer.cs | 1 + .../WebServer/CoreOwinWebServer.cs | 44 +++++++++++- 5 files changed, 161 insertions(+), 13 deletions(-) diff --git a/src/libs/dutil/WixToolset.DUtil/dlutil.cpp b/src/libs/dutil/WixToolset.DUtil/dlutil.cpp index f6b793b2..9c704ee8 100644 --- a/src/libs/dutil/WixToolset.DUtil/dlutil.cpp +++ b/src/libs/dutil/WixToolset.DUtil/dlutil.cpp @@ -295,7 +295,10 @@ static HRESULT DownloadResource( HANDLE hPayloadFile = INVALID_HANDLE_VALUE; DWORD cbMaxData = 64 * 1024; // 64 KB BYTE* pbData = NULL; - BOOL fRangeRequestsAccepted = TRUE; + BOOL fUseRangeRequest = TRUE; + BOOL fRangeRequestsAccepted = FALSE; + BOOL fRequestedRangeRequest = FALSE; + BOOL fInvalidRangeRequestResponse = FALSE; LPWSTR sczRangeRequestHeader = NULL; HINTERNET hConnect = NULL; HINTERNET hUrl = NULL; @@ -315,10 +318,19 @@ static HRESULT DownloadResource( // are not supported we'll have to start over and accept the fact that we only get one shot // downloading the file however big it is. Hopefully, not more than 2 GB since wininet doesn't // like files that big. - while (fRangeRequestsAccepted && (0 == dw64ResourceLength || dw64ResumeOffset < dw64ResourceLength)) + for (;;) { - hr = AllocateRangeRequestHeader(dw64ResumeOffset, 0 == dw64ResourceLength ? dw64AuthoredResourceLength : dw64ResourceLength, &sczRangeRequestHeader); - DlExitOnFailure(hr, "Failed to allocate range request header."); + fInvalidRangeRequestResponse = FALSE; + + if (fUseRangeRequest) + { + hr = AllocateRangeRequestHeader(dw64ResumeOffset, 0 == dw64ResourceLength ? dw64AuthoredResourceLength : dw64ResourceLength, &sczRangeRequestHeader); + DlExitOnFailure(hr, "Failed to allocate range request header."); + } + else + { + ReleaseNullStr(sczRangeRequestHeader); + } ReleaseNullInternet(hConnect); ReleaseNullInternet(hUrl); @@ -326,6 +338,13 @@ static HRESULT DownloadResource( hr = MakeRequest(hSession, psczUrl, L"GET", sczRangeRequestHeader, wzUser, wzPassword, pAuthenticate, &hConnect, &hUrl, &fRangeRequestsAccepted); DlExitOnFailure(hr, "Failed to request URL for download: %ls", *psczUrl); + fRequestedRangeRequest = sczRangeRequestHeader && *sczRangeRequestHeader; + + if (fRequestedRangeRequest && !fRangeRequestsAccepted) + { + LogStringLine(REPORT_VERBOSE, "Range request not supported for URL: %ls", *psczUrl); + } + // If we didn't get the size of the resource from the initial "HEAD" request // then let's try to get the size from this "GET" request. if (0 == dw64ResourceLength) @@ -337,23 +356,36 @@ static HRESULT DownloadResource( } else // server didn't tell us the resource length. { + LogStringLine(REPORT_VERBOSE, "Content-Length not returned for URL: %ls", *psczUrl); + // Fallback to the authored size of the resource. However, since we // don't really know the size on the server, don't try to use // range requests either. dw64ResourceLength = dw64AuthoredResourceLength; + fInvalidRangeRequestResponse = fRequestedRangeRequest; fRangeRequestsAccepted = FALSE; } } - // If we just tried to do a range request and found out that it isn't supported, start over. - if (!fRangeRequestsAccepted) + // If we just tried to do a range request and found out that it isn't supported, ignore the offset. + if (fRequestedRangeRequest && !fRangeRequestsAccepted) { - // TODO: log a message that the server did not accept range requests. dw64ResumeOffset = 0; + fUseRangeRequest = FALSE; + } + + if (fInvalidRangeRequestResponse) + { + continue; } hr = WriteToFile(hUrl, hPayloadFile, &dw64ResumeOffset, hResumeFile, dw64ResourceLength, pbData, cbMaxData, pCache); DlExitOnFailure(hr, "Failed while reading from internet and writing to: %ls", wzDestinationPath); + + if (!fUseRangeRequest || dw64ResumeOffset >= dw64ResourceLength) + { + break; + } } LExit: diff --git a/src/test/burn/TestData/CacheTests/BundleC/BundleC.wxs b/src/test/burn/TestData/CacheTests/BundleC/BundleC.wxs index ca21cc6e..865f7578 100644 --- a/src/test/burn/TestData/CacheTests/BundleC/BundleC.wxs +++ b/src/test/burn/TestData/CacheTests/BundleC/BundleC.wxs @@ -5,7 +5,7 @@ - + diff --git a/src/test/burn/WixToolsetTest.BurnE2E/CacheTests.cs b/src/test/burn/WixToolsetTest.BurnE2E/CacheTests.cs index 4a3ffa2e..ee61baff 100644 --- a/src/test/burn/WixToolsetTest.BurnE2E/CacheTests.cs +++ b/src/test/burn/WixToolsetTest.BurnE2E/CacheTests.cs @@ -15,8 +15,7 @@ namespace WixToolsetTest.BurnE2E { public CacheTests(ITestOutputHelper testOutputHelper) : base(testOutputHelper) { } - [Fact] - public void CanCache5GBFile() + private bool Is5GBFileAvailable() { // Recreate the 5GB payload to avoid having to copy it to the VM to run the tests. const long FiveGB = 5_368_709_120; @@ -28,8 +27,8 @@ namespace WixToolsetTest.BurnE2E var drive = new DriveInfo(targetFilePath.Substring(0, 1)); if (drive.AvailableFreeSpace < FiveGB + OneGB) { - Console.WriteLine("Skipping CanCache5GBFile() test because there is not enough disk space available to run the test."); - return; + Console.WriteLine($"Skipping {this.TestContext.TestName} because there is not enough disk space available to run the test."); + return false; } if (!File.Exists(targetFilePath)) @@ -42,6 +41,17 @@ namespace WixToolsetTest.BurnE2E testTool.Run(true); } + return true; + } + + [Fact] + public void CanCache5GBFile() + { + if (!this.Is5GBFileAvailable()) + { + return; + } + var packageA = this.CreatePackageInstaller("PackageA"); var bundleC = this.CreateBundleInstaller("BundleC"); @@ -53,6 +63,69 @@ namespace WixToolsetTest.BurnE2E packageA.VerifyInstalled(true); } + private string Cache5GBFileFromDownload(bool disableRangeRequests) + { + if (!this.Is5GBFileAvailable()) + { + return null; + } + + var packageA = this.CreatePackageInstaller("PackageA"); + var bundleC = this.CreateBundleInstaller("BundleC"); + var webServer = this.CreateWebServer(); + + webServer.AddFiles(new Dictionary + { + { "/BundleC/fivegb.file", Path.Combine(this.TestContext.TestDataFolder, "fivegb.file") }, + { "/BundleC/PackageA.msi", Path.Combine(this.TestContext.TestDataFolder, "PackageA.msi") }, + }); + webServer.DisableRangeRequests = disableRangeRequests; + webServer.Start(); + + using var dfs = new DisposableFileSystem(); + var separateDirectory = dfs.GetFolder(true); + + // Manually copy bundle to separate directory and then run from there so the non-compressed payloads have to be resolved. + var bundleCFileInfo = new FileInfo(bundleC.Bundle); + var bundleCCopiedPath = Path.Combine(separateDirectory, bundleCFileInfo.Name); + bundleCFileInfo.CopyTo(bundleCCopiedPath); + + packageA.VerifyInstalled(false); + + var installLogPath = bundleC.Install(bundleCCopiedPath); + bundleC.VerifyRegisteredAndInPackageCache(); + + packageA.VerifyInstalled(true); + + return installLogPath; + } + + [Fact] + public void CanCache5GBFileFromDownloadWithRangeRequestSupport() + { + var logPath = this.Cache5GBFileFromDownload(false); + if (logPath == null) + { + return; + } + + Assert.False(LogVerifier.MessageInLogFile(logPath, "Range request not supported for URL: http://localhost:9999/e2e/BundleC/fivegb.file")); + Assert.True(LogVerifier.MessageInLogFile(logPath, "Content-Length not returned for URL: http://localhost:9999/e2e/BundleC/fivegb.file")); + } + + [Fact] + public void CanCache5GBFileFromDownloadWithoutRangeRequestSupport() + { + var logPath = this.Cache5GBFileFromDownload(true); + if (logPath == null) + { + return; + } + + Assert.True(LogVerifier.MessageInLogFile(logPath, "Range request not supported for URL: http://localhost:9999/e2e/BundleC/fivegb.file")); + Assert.True(LogVerifier.MessageInLogFile(logPath, "Content-Length not returned for URL: http://localhost:9999/e2e/BundleC/fivegb.file")); + } + [Fact] public void CanDownloadPayloadsFromMissingAttachedContainer() { diff --git a/src/test/burn/WixToolsetTest.BurnE2E/IWebServer.cs b/src/test/burn/WixToolsetTest.BurnE2E/IWebServer.cs index 3846ae94..a4d46d48 100644 --- a/src/test/burn/WixToolsetTest.BurnE2E/IWebServer.cs +++ b/src/test/burn/WixToolsetTest.BurnE2E/IWebServer.cs @@ -8,6 +8,7 @@ namespace WixToolsetTest.BurnE2E public interface IWebServer : IDisposable { bool DisableHeadResponses { get; set; } + bool DisableRangeRequests { get; set; } /// /// Registers a collection of relative URLs (the key) with its absolute path to the file (the value). diff --git a/src/test/burn/WixToolsetTest.BurnE2E/WebServer/CoreOwinWebServer.cs b/src/test/burn/WixToolsetTest.BurnE2E/WebServer/CoreOwinWebServer.cs index 025e01ff..8066cea5 100644 --- a/src/test/burn/WixToolsetTest.BurnE2E/WebServer/CoreOwinWebServer.cs +++ b/src/test/burn/WixToolsetTest.BurnE2E/WebServer/CoreOwinWebServer.cs @@ -5,8 +5,10 @@ namespace WixToolsetTest.BurnE2E using System; using System.Collections.Generic; using System.IO; + using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; + using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.StaticFiles; using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.FileProviders.Physical; @@ -15,11 +17,14 @@ namespace WixToolsetTest.BurnE2E public class CoreOwinWebServer : IWebServer, IFileProvider { + const string StaticFileBasePath = "/e2e"; + private Dictionary PhysicalPathsByRelativeUrl { get; } = new Dictionary(); private IHost WebHost { get; set; } public bool DisableHeadResponses { get; set; } + public bool DisableRangeRequests { get; set; } public void AddFiles(Dictionary physicalPathsByRelativeUrl) { @@ -38,10 +43,11 @@ namespace WixToolsetTest.BurnE2E webBuilder.UseUrls("http://localhost:9999"); webBuilder.Configure(appBuilder => { + appBuilder.Use(this.CustomStaticFileMiddleware); appBuilder.UseStaticFiles(new StaticFileOptions { FileProvider = this, - RequestPath = "/e2e", + RequestPath = StaticFileBasePath, ServeUnknownFileTypes = true, OnPrepareResponse = this.OnPrepareStaticFileResponse, }); @@ -51,6 +57,42 @@ namespace WixToolsetTest.BurnE2E this.WebHost.Start(); } + private async Task CustomStaticFileMiddleware(HttpContext context, Func next) + { + if (!this.DisableRangeRequests || (!HttpMethods.IsGet(context.Request.Method) && !HttpMethods.IsHead(context.Request.Method))) + { + await next(); + return; + } + + // Only send Content-Length header. + // Don't support range requests. + // https://github.com/dotnet/aspnetcore/blob/60abfafe32a4692f9dc4a172665524f163b10012/src/Middleware/StaticFiles/src/StaticFileMiddleware.cs + if (!context.Request.Path.StartsWithSegments(StaticFileBasePath, out var subpath)) + { + context.Response.StatusCode = 404; + return; + } + + var fileInfo = this.GetFileInfo(subpath); + if (!fileInfo.Exists) + { + context.Response.StatusCode = 404; + return; + } + + var responseHeaders = context.Response.GetTypedHeaders(); + var fileLength = fileInfo.Length; + responseHeaders.ContentLength = fileLength; + + this.OnPrepareStaticFileResponse(new StaticFileResponseContext(context, fileInfo)); + + if (HttpMethods.IsGet(context.Request.Method)) + { + await context.Response.SendFileAsync(fileInfo, 0, fileLength); + } + } + private void OnPrepareStaticFileResponse(StaticFileResponseContext obj) { if (this.DisableHeadResponses && obj.Context.Request.Method == "HEAD") -- cgit v1.2.3-55-g6feb