Ensure external sources for styles only process CSS responses when inlining stylesheets #1
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
This fix relates to issues when a HTML template contains a
<link>node that points at a resource which returns something that isn't a CSS stylesheet. We had a user which created a HTML template (that we render and serve, hence the need to inline stylesheets), which managed to spike CPU usage on our Azure App Service Plan to 100%, with no end in sight. It simply just peaked there, and kept spinning, bringing several other apps on the same App Service Plan down and/or non-responsive.We ran a .NET trace on the Azure App Service to try and figure out where the issue was coming from, and we came across this issue which seemed to match up with what we were seeing, at least on the surface.
Once we dug into it a bit more, we realized the template had a
<link>node defined that once evaluated by the PreMailer.Net library, would result in a Content-Type return value oftext/html. It was returning plain HTML which seemed to be throwing the Regex parsers for a loop!The dodgy
<link>in question was pointing at a resource that would only return actual CSS (what PreMailer.Net would normally expect), if we set theAcceptheader totext/css, to force the server to return the CSS string. Otherwise it would render the "normal" HTML page, as if you'd navigated to it via a browser.So the fix for this is fairly straightforward, where we ensure the
Acceptheaders for theWebDownloader.DownloadStringmethod are added to specifically only request CSS content, and we check the responseContent-Typeand ensure that matches CSS content too, and throw if we detect something different.Open to feedback on any other/cleaner ways of fixing this :)
Changes
WebDownloader.DownloadStringto set theAcceptrequest header for CSS stylesheetsWebDownloader.DownloadStringto check the response and ensure theContent-Typematches the CSS Content-Type.Content-Typeresponse header, to make checking/comparing the response headers a bit easier.Benchmarks.Programclass to use a potentially safer method of benchmarking our code, where we create a config on the fly that usesInProcessNoEmitToolchainto prevent our Benchmarks from spawning too many threads/processes.Questions
IWebDownloaderimplementations tend to be mocked/stubbed out, so given most of my changes live in theWebDownloaderitself, I'd love some guidance on how you'd want me to test this.InProcessNoEmitToolchainconfiguration changes in the Benchmarker project?Exceptions I'm throwing? I wasn't entirely sure if throwing was the correct way forward for how we normally handle issues like this within PreMailer.Net, so I'm also open to feedback on changing this to something other than Exceptions being thrown everywhere :)