Skip to content

[Static Web Assets] Fix compressed endpoint headers #41511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 11, 2024
Merged

[Static Web Assets] Fix compressed endpoint headers #41511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cf44f79
[Static Web Assets] Fix compressed endpoint headers
javiercn Jun 10, 2024
4ed3846
Fix test
javiercn Jun 10, 2024
9975c20
Fix test
javiercn Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions src/StaticWebAssetsSdk/Tasks/ApplyCompressionNegotiation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public override bool Execute()
.ToDictionary(g => g.Key, g => g.ToList());

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.


var preservedEndpoints = new Dictionary<(string, string), StaticWebAssetEndpoint>();

Expand Down Expand Up @@ -89,13 +89,14 @@ public override bool Execute()
}

Log.LogMessage(MessageImportance.Low, " Updated endpoint '{0}' with Content-Encoding and Vary headers", compressedEndpoint.Route);
if (!updatedEndpoints.Contains(compressedEndpoint))
{
updatedEndpoints.Add(compressedEndpoint);
}
updatedEndpoints.Add(compressedEndpoint);

foreach (var relatedEndpointCandidate in relatedAssetEndpoints)
{
if (!IsCompatible(compressedEndpoint, relatedEndpointCandidate))
{
continue;
}
Comment on lines +96 to +99
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.

Log.LogMessage(MessageImportance.Low, "Processing related endpoint '{0}'", relatedEndpointCandidate.Route);
var encodingSelector = new StaticWebAssetEndpointSelector
{
Expand Down Expand Up @@ -188,7 +189,10 @@ public override bool Execute()
{
Log.LogMessage(MessageImportance.Low, " Adding endpoint '{0}'", endpoint.AssetFile);
}
updatedEndpoints.AddRange(endpoints);
foreach (var endpoint in endpoints)
{
updatedEndpoints.Add(endpoint);
}
}
}

Expand All @@ -197,6 +201,13 @@ public override bool Execute()
return true;
}

private static bool IsCompatible(StaticWebAssetEndpoint compressedEndpoint, StaticWebAssetEndpoint relatedEndpointCandidate)
{
var compressedFingerprint = compressedEndpoint.EndpointProperties.FirstOrDefault(ep => ep.Name == "fingerprint");
var relatedFingerprint = relatedEndpointCandidate.EndpointProperties.FirstOrDefault(ep => ep.Name == "fingerprint");
return string.Equals(compressedFingerprint?.Value, relatedFingerprint?.Value, StringComparison.Ordinal);
}

private void ApplyCompressedEndpointHeaders(List<StaticWebAssetEndpointResponseHeader> headers, StaticWebAssetEndpoint compressedEndpoint, string relatedEndpointCandidateRoute)
{
foreach (var header in compressedEndpoint.ResponseHeaders)
Expand Down
2 changes: 2 additions & 0 deletions src/StaticWebAssetsSdk/Tasks/Data/StaticWebAssetPathPattern.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ public static StaticWebAssetPathPattern Parse(string rawPath, string assetIdenti
}
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.

result.Append(part.Value);
}
else
Expand Down
24 changes: 20 additions & 4 deletions ...ts/StaticWebAssetsBaselines/StaticWebAssets_BuildMinimal_Works.Build.staticwebassets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36900,7 +36900,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -36928,9 +36928,17 @@
}
],
"EndpointProperties": [
{
"Name": "fingerprint",
"Value": "__fingerprint__"
},
{
"Name": "integrity",
"Value": "__integrity__"
},
{
"Name": "label",
"Value": "blazorwasm-minimal.bundle.scp.css.gz"
}
]
},
Expand Down Expand Up @@ -37006,7 +37014,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -37108,7 +37116,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -37136,9 +37144,17 @@
}
],
"EndpointProperties": [
{
"Name": "fingerprint",
"Value": "__fingerprint__"
},
{
"Name": "integrity",
"Value": "__integrity__"
},
{
"Name": "label",
"Value": "blazorwasm-minimal.styles.css.gz"
}
]
},
Expand Down Expand Up @@ -37214,7 +37230,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down
52 changes: 42 additions & 10 deletions ...taticWebAssetsBaselines/StaticWebAssets_PublishMinimal_Works.Publish.staticwebassets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5858,7 +5858,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -5886,9 +5886,17 @@
}
],
"EndpointProperties": [
{
"Name": "fingerprint",
"Value": "__fingerprint__"
},
{
"Name": "integrity",
"Value": "__integrity__"
},
{
"Name": "label",
"Value": "blazorwasm-minimal.bundle.scp.css.gz"
}
]
},
Expand Down Expand Up @@ -5964,7 +5972,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -6011,7 +6019,7 @@
]
},
{
"Route": "blazorwasm-minimal.__fingerprint__.styles.css.gz",
"Route": "blazorwasm-minimal.styles.css.gz",
"AssetFile": "${ProjectPath}\\obj\\Debug\\${Tfm}\\compressed\\blazorwasm-minimal#[.{fingerprint=__fingerprint__}]?.styles.css.gz",
"Selectors": [],
"ResponseHeaders": [
Expand Down Expand Up @@ -6056,7 +6064,7 @@
]
},
{
"Route": "blazorwasm-minimal.styles.css.gz",
"Route": "blazorwasm-minimal.__fingerprint__.styles.css.gz",
"AssetFile": "${ProjectPath}\\obj\\Debug\\${Tfm}\\compressed\\blazorwasm-minimal#[.{fingerprint=__fingerprint__}]?.styles.css.gz",
"Selectors": [],
"ResponseHeaders": [
Expand All @@ -6066,7 +6074,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -6094,9 +6102,17 @@
}
],
"EndpointProperties": [
{
"Name": "fingerprint",
"Value": "__fingerprint__"
},
{
"Name": "integrity",
"Value": "__integrity__"
},
{
"Name": "label",
"Value": "blazorwasm-minimal.styles.css.gz"
}
]
},
Expand Down Expand Up @@ -6172,7 +6188,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -20474,7 +20490,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -20502,9 +20518,17 @@
}
],
"EndpointProperties": [
{
"Name": "fingerprint",
"Value": "__fingerprint__"
},
{
"Name": "integrity",
"Value": "__integrity__"
},
{
"Name": "label",
"Value": "blazorwasm-minimal.bundle.scp.css.br"
}
]
},
Expand Down Expand Up @@ -20580,7 +20604,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -20682,7 +20706,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -20710,9 +20734,17 @@
}
],
"EndpointProperties": [
{
"Name": "fingerprint",
"Value": "__fingerprint__"
},
{
"Name": "integrity",
"Value": "__integrity__"
},
{
"Name": "label",
"Value": "blazorwasm-minimal.styles.css.br"
}
]
},
Expand Down Expand Up @@ -20788,7 +20820,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down
28 changes: 22 additions & 6 deletions ...bAssets_Publish_DoesNotIncludeXmlDocumentationFiles_AsAssets.Publish.staticwebassets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6122,7 +6122,7 @@
]
},
{
"Route": "blazorwasm.__fingerprint__.styles.css.gz",
"Route": "blazorwasm.styles.css.gz",
"AssetFile": "${ProjectPath}\\blazorwasm\\obj\\Debug\\${Tfm}\\compressed\\blazorwasm#[.{fingerprint=__fingerprint__}]?.styles.css.gz",
"Selectors": [],
"ResponseHeaders": [
Expand Down Expand Up @@ -6167,7 +6167,7 @@
]
},
{
"Route": "blazorwasm.styles.css.gz",
"Route": "blazorwasm.__fingerprint__.styles.css.gz",
"AssetFile": "${ProjectPath}\\blazorwasm\\obj\\Debug\\${Tfm}\\compressed\\blazorwasm#[.{fingerprint=__fingerprint__}]?.styles.css.gz",
"Selectors": [],
"ResponseHeaders": [
Expand All @@ -6177,7 +6177,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -6205,9 +6205,17 @@
}
],
"EndpointProperties": [
{
"Name": "fingerprint",
"Value": "__fingerprint__"
},
{
"Name": "integrity",
"Value": "__integrity__"
},
{
"Name": "label",
"Value": "blazorwasm.styles.css.gz"
}
]
},
Expand Down Expand Up @@ -6283,7 +6291,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -20785,7 +20793,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down Expand Up @@ -20813,9 +20821,17 @@
}
],
"EndpointProperties": [
{
"Name": "fingerprint",
"Value": "__fingerprint__"
},
{
"Name": "integrity",
"Value": "__integrity__"
},
{
"Name": "label",
"Value": "blazorwasm.styles.css.br"
}
]
},
Expand Down Expand Up @@ -20891,7 +20907,7 @@
},
{
"Name": "Cache-Control",
"Value": "no-cache"
"Value": "max-age=31536000, immutable"
},
{
"Name": "Content-Encoding",
Expand Down
Loading