Fixes #5076, manifest creation for non-HTML URLs

Author: JudahGabriel

apps/pwabuilder/Common/HttpClientExtensions.cs

@@ -82,30 +82,71 @@ public static class HttpClientExtensions
     /// <param name="cancelToken">Cancellation token.</param>
     /// <returns>The string fetched from the URI.</returns>
     /// <exception cref="InvalidOperationException">The response was longer than the max size.</exception>
-    public static async Task<LimitedReadStreamWithMediaType> GetImageAsync(this HttpClient client, Uri requestUri, long maxSizeInBytes, CancellationToken cancelToken)
+    public static async Task<LimitedReadStreamWithMediaType> GetStreamAsync(this HttpClient client, Uri requestUri, IEnumerable<string> accepts, long maxSizeInBytes, CancellationToken cancelToken)
     {
-        var imageRequest = new HttpRequestMessage(HttpMethod.Get, requestUri);
+        var streamRequest = new HttpRequestMessage(HttpMethod.Get, requestUri);
 
         // Add the accept header for images.
-        imageRequest.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("image/*"));
+        // Add the accept header if provided.
+        foreach (var acceptType in accepts)
+        {
+            streamRequest.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue(acceptType));
+        }
 
         // Send it.
-        var imageFetch = await client.SendAsync(imageRequest, cancelToken);
+        var streamFetch = await client.SendAsync(streamRequest, cancelToken);
 
         // We have a max size, so we need to check the content length.
         // First, ensure we've got success.
-        imageFetch.EnsureSuccessStatusCode();
+        streamFetch.EnsureSuccessStatusCode();
 
         // See if we have a Content-Length header
-        var contentLength = imageFetch.Content.Headers.ContentLength;
+        var contentLength = streamFetch.Content.Headers.ContentLength;
         if (contentLength.HasValue && contentLength.Value > maxSizeInBytes)
         {
             throw new InvalidOperationException($"Attempted to fetch {requestUri}, but response content-length header says the response size ({contentLength.Value}) exceeds the maximum allowed size ({maxSizeInBytes}).");
         }
 
         // Read in a string as a stream to ensure we don't exceed the max size.
-        var stream = await imageFetch.Content.ReadAsStreamAsync(cancelToken);
-        return new LimitedReadStreamWithMediaType(stream, maxSizeInBytes, imageFetch.Content.Headers.ContentType?.MediaType);
+        var stream = await streamFetch.Content.ReadAsStreamAsync(cancelToken);
+        return new LimitedReadStreamWithMediaType(stream, maxSizeInBytes, streamFetch.Content.Headers.ContentType?.MediaType);
+    }
+
+    /// <summary>
+    /// Fetches an image from the specified URI while providing an expected content type and a maximum response size.
+    /// </summary>
+    /// <param name="client">The HTTP client.</param>
+    /// <param name="requestUri">The URI to request.</param>
+    /// <param name="maxSizeInBytes">The maximum size in bytes of the response.</param>
+    /// <param name="cancelToken">Cancellation token.</param>
+    /// <returns>The string fetched from the URI.</returns>
+    /// <exception cref="InvalidOperationException">The response was longer than the max size.</exception>
+    public static Task<LimitedReadStreamWithMediaType> GetImageAsync(this HttpClient client, Uri requestUri, long maxSizeInBytes, CancellationToken cancelToken)
+    {
+        return GetStreamAsync(client, requestUri, ["image/*"], maxSizeInBytes, cancelToken);
+    }
+
+    /// <summary>
+    /// Makes a request to the URL and reads only the response headers. It throws an HttpRequestException if the response headers don't contain the specified content type.
+    /// </summary>
+    /// <param name="http">The HTTP client.</param>
+    /// <param name="cancelToken">The cancellation token.</param>
+    /// <param name="responseContentType">The expected response content type. If the response doesn't contain this content type header, an HttpRequestException will be thrown.</param>
+    /// <exception cref="HttpRequestException">The request didn't have a response content-type containing <paramref name="responseContentType"/>.
+    public static void EnsureContentType(this HttpResponseMessage response, string responseContentType)
+    {
+        // If there is no content type header, consider this test skipped. 
+        var contentType = response.Content.Headers.ContentType;
+        if (string.IsNullOrEmpty(contentType?.MediaType))
+        {
+            throw new HttpRequestException($"Expected a response content-type header of {responseContentType}, but it had none.");
+        }
+
+        var hasMatchingContentType = contentType.MediaType.Contains("text/html");
+        if (!hasMatchingContentType)
+        {
+            throw new HttpRequestException($"Expected a response content-type header of {responseContentType}, but it only had {contentType.MediaType}.");
+        }
     }
 
     public class LimitedReadStreamWithMediaType : Stream

apps/pwabuilder/Controllers/ManifestsController.cs

@@ -14,14 +14,14 @@ public class ManifestsController : ControllerBase
     /// <param name="manifestCreatorService">The manifest creation service.</param>
     /// <returns>A new web manifest for the specified site.</returns>
     [HttpPost("create")]
-    public async Task<IActionResult> Create([FromQuery] Uri url, [FromServices] ManifestCreator manifestCreatorService)
+    public async Task<IActionResult> Create([FromQuery] Uri url, [FromServices] ManifestCreator manifestCreatorService, CancellationToken cancellationToken)
     {
         if (!url.IsAbsoluteUri)
         {
             url = new Uri($"https://{url}", UriKind.Absolute);
         }
 
-        var manifest = await manifestCreatorService.Create(url);
+        var manifest = await manifestCreatorService.Create(url, cancellationToken);
         return Ok(manifest);
     }
 }

apps/pwabuilder/Frontend/src/script/pages/app-report.ts

@@ -227,10 +227,10 @@ export class AppReport extends LitElement {
     }
 
     private async applyManifestContext(url: string, manifestUrl?: string, manifestRaw?: string) {
+        this.isAppCardInfoLoading = false;
         this.manifestContext = await this.processManifest(url, manifestUrl, manifestRaw);
         this.createdManifest = this.manifestContext.isGenerated || false;
         setManifestContext(this.manifestContext);
-        this.isAppCardInfoLoading = false;
         await this.populateAppCard(this.manifestContext, manifestUrl);
     }
 

apps/pwabuilder/Services/ManifestCreator.cs

@@ -25,11 +25,12 @@ public ManifestCreator(IHttpClientFactory httpClientFactory, ILogger<ManifestCre
     /// Creates a web manifest for the specified site. It will use any metadata on the page to help generate a manifest.
     /// </summary>
     /// <param name="siteUri">The URL of the site to generate the manifest for.</param>
+    /// <param name="cancelToken">The cancellation token.</param>
     /// <returns>A new web manifest.</returns>
-    public async Task<WebAppManifest> Create(Uri siteUri)
+    public async Task<WebAppManifest> Create(Uri siteUri, CancellationToken cancelToken)
     {
         // Fetch the site
-        var manifestResult = await this.LoadPage(siteUri)
+        var manifestResult = await this.LoadPage(siteUri, cancelToken)
             .PipeAsync(async html => await CreateManifestFromHtml(siteUri, html));
         if (manifestResult.Error != null)
         {
@@ -415,9 +416,9 @@ private static string GetMetaTagContent(HtmlNode? head, AttrQuery query, string
         return string.IsNullOrWhiteSpace(metaVal) ? fallbackValue : metaVal;
     }
 
-    private async Task<Result<HtmlDocument>> LoadPage(Uri url)
+    private async Task<Result<HtmlDocument>> LoadPage(Uri url, CancellationToken cancelToken)
     {
-        var fetchResult = await TryFetch(url, "text/html");
+        var fetchResult = await TryFetch(url, ["text/html"], cancelToken);
         return fetchResult.Pipe(CreateDocumentFromHtml);
     }
 
@@ -437,57 +438,48 @@ private static HtmlDocument CreateDocumentFromHtml(string html)
     /// <param name="url"></param>
     /// <param name="acceptHeaders"></param>
     /// <returns></returns>
-    private async Task<Result<string>> TryFetch(Uri url, params string[] acceptHeaders)
+    private async Task<Result<string>> TryFetch(Uri url, IEnumerable<string> acceptHeaders, CancellationToken cancelToken)
     {
+        // First, make sure we have HTML. Read just the headers and make sure it has the right content-type response.
         try
         {
-            using var httpRequest = new HttpRequestMessage(HttpMethod.Get, url);
-            if (acceptHeaders != null)
-            {
-                foreach (var header in acceptHeaders)
-                {
-                    httpRequest.Headers.Accept.Add(new System.Net.Http.Headers.MediaTypeWithQualityHeaderValue(header));
-                }
-            }
+            using var headersOnlyResponse = await http.GetAsync(url, HttpCompletionOption.ResponseHeadersRead, cancelToken);
+            headersOnlyResponse.EnsureSuccessStatusCode();
+            headersOnlyResponse.EnsureContentType("text/html");
+        }
+        catch (Exception contentTypeError)
+        {
+            logger.LogError(contentTypeError, "Failed to verify {url} serves HTML content during manifest creation process. Will return an empty manifest.", url);
+            return string.Empty;
+        }
 
-            var httpResponse = await http.SendAsync(httpRequest);
-            httpResponse.EnsureSuccessStatusCode();
-            var content = await httpResponse.Content.ReadAsStringAsync();
-            return content;
+        try
+        {
+            var htmlResponse = await http.GetStringAsync(url, acceptHeaders, 1024 * 1024 * 2, cancelToken); // 2MB max HTML            httpResponse.EnsureSuccessStatusCode();
+            return htmlResponse ?? string.Empty;
         }
         catch (InvalidOperationException invalidOpError) when (invalidOpError.Message.Contains("The character set provided in ContentType is invalid."))
         {
             // Invalid encoding? Sometimes webpages have incorrectly set their charset / content type.
             // See if we can just parse the thing using UTF-8.
             logger.LogWarning(invalidOpError, "Unable to parse using HTTP client due to invalid ContentType. Attempting to parse using UTF-8.");
-            return await TryFetchWithForcedUtf8(url, acceptHeaders);
+            return await TryFetchWithForcedUtf8(url, acceptHeaders, cancelToken);
         }
         catch (Exception httpException)
         {
             logger.LogWarning(httpException, "Failed to fetch {url} using HTTP client. Falling back to HTTP/2 fetch.", url);
-            return await TryFetchWithHttp2Client(url, acceptHeaders);
+            return await TryFetchWithHttp2Client(url, acceptHeaders, cancelToken);
         }
     }
 
-    private async Task<Result<string>> TryFetchWithForcedUtf8(Uri url, params string[] acceptHeaders)
+    private async Task<Result<string>> TryFetchWithForcedUtf8(Uri url, IEnumerable<string> acceptHeaders, CancellationToken cancelToken)
     {
         try
         {
-            using var httpRequest = new HttpRequestMessage(HttpMethod.Get, url);
-            if (acceptHeaders != null)
-            {
-                foreach (var header in acceptHeaders)
-                {
-                    httpRequest.Headers.Accept.Add(new System.Net.Http.Headers.MediaTypeWithQualityHeaderValue(header));
-                }
-            }
-
-            var httpResponse = await http.SendAsync(httpRequest);
-            httpResponse.EnsureSuccessStatusCode();
-            var contentBytes = await httpResponse.Content.ReadAsByteArrayAsync();
-            var responseString = Encoding.UTF8.GetString(contentBytes);
-            logger.LogInformation("Successfully parsed the HTML using forced UTF-8 mode");
-            return responseString;
+            var byteStream = await this.http.GetStreamAsync(url, acceptHeaders, 1024 * 1024 * 2, cancelToken);
+            using var memStream = new MemoryStream();
+            await byteStream.CopyToAsync(memStream, cancelToken);
+            return Encoding.UTF8.GetString(memStream.ToArray());
         }
         catch (Exception error)
         {
@@ -496,7 +488,7 @@ private async Task<Result<string>> TryFetchWithForcedUtf8(Uri url, params string
         }
     }
 
-    private async Task<Result<string>> TryFetchWithHttp2Client(Uri url, params string[] acceptHeaders)
+    private async Task<Result<string>> TryFetchWithHttp2Client(Uri url, IEnumerable<string> acceptHeaders, CancellationToken cancelToken)
     {
         try
         {