Skip to content

Conversation

javiercn
Copy link
Member

@javiercn javiercn commented Jun 10, 2024

  • When fingerprinting assets the headers were not being correctly applied to the compressed endpoints with fingerprint.
  • Added a fix and a test to cover the scenario.

@ghost ghost added Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch untriaged Request triage from a team member labels Jun 10, 2024
else if (!string.IsNullOrEmpty(part.Value))
{
// Token was embedded, so add it to the dictionary.
dictionary[part.Name] = part.Value;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the token was embedded in the expression (for example, file#[.{fingerprint=value}].ext we weren't adding it to the tokens returned, so it wasn't being added as a property and we weren't setting the caching headers correctly because of that.


var compressedAssets = assetsById.Values.Where(a => a.AssetTraitName == "Content-Encoding").ToList();
var updatedEndpoints = new List<StaticWebAssetEndpoint>();
var updatedEndpoints = new HashSet<StaticWebAssetEndpoint>(StaticWebAssetEndpoint.RouteAndAssetComparer);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The combination of route + asset file is unique, when we have fingerprinting expressions more than one endpoint is associated with a given compressed asset, so we need to make sure we don't add it multiple times.

Using a HashSet is simpler and better than using a list, also, in this case we need to avoid comparing using the headers and properties because those will be different.

Comment on lines +96 to +99
if (!IsCompatible(compressedEndpoint, relatedEndpointCandidate))
{
continue;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were copying the related endpoint and applying a selector + the compressed endpoint properties. That caused for example that we created a fingerprinted endpoint using a non-fingerprinted compressed endpoint.

To address that, we check that both endpoints have or don't have fingerprint and in case of having it, they match.

This way the fingerprinted endpoint we create for content-negotiation has the same headers as the fingerprinted compressed endpoint.

@javiercn javiercn marked this pull request as ready for review June 10, 2024 21:59
@javiercn javiercn requested review from a team as code owners June 10, 2024 21:59
@javiercn javiercn enabled auto-merge (squash) June 11, 2024 07:38
@javiercn javiercn force-pushed the javiercn/static-web-asset-endpoints-fix-compression-headers branch from be84436 to 6c4f09e Compare June 11, 2024 07:39
javiercn added 3 commits June 11, 2024 11:39
Fixes the headers for the compressed endpoints during
compression negotiation to ensure that the correct
headers are set for the endpoints that perform content negotiation.
@javiercn javiercn force-pushed the javiercn/static-web-asset-endpoints-fix-compression-headers branch from 6c4f09e to 9975c20 Compare June 11, 2024 09:39
@javiercn javiercn merged commit ab2777f into main Jun 11, 2024
@javiercn javiercn deleted the javiercn/static-web-asset-endpoints-fix-compression-headers branch June 11, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch untriaged Request triage from a team member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants