agrande-ea · agrande-ea · Dec 27, 2024 · Dec 27, 2024 · Jan 23, 2025 · Jan 29, 2025
diff --git a/.github/workflows/dotnetcore.yml b/.github/workflows/dotnetcore.yml
@@ -16,7 +16,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4.2.2
     - name: Setup .NET Core
-      uses: actions/setup-dotnet@v4.1.0
+      uses: actions/setup-dotnet@v4.3.1
       with:
         dotnet-version: 3.1.101
     - name: Install dependencies

diff --git a/PreMailer.Net/Benchmarks/Benchmarks.csproj b/PreMailer.Net/Benchmarks/Benchmarks.csproj
@@ -2,12 +2,12 @@
 
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-	<TargetFramework>net7.0</TargetFramework>
+	<TargetFramework>net9.0</TargetFramework>
     <Nullable>enable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="BenchmarkDotNet" Version="0.13.3" />
+    <PackageReference Include="BenchmarkDotNet" Version="0.14.0" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/PreMailer.Net/Benchmarks/Program.cs b/PreMailer.Net/Benchmarks/Program.cs
@@ -1,17 +1,30 @@
 using AngleSharp;
 using AngleSharp.Html.Parser;
 using BenchmarkDotNet.Attributes;
+using BenchmarkDotNet.Configs;
+using BenchmarkDotNet.Jobs;
 using BenchmarkDotNet.Running;
+using BenchmarkDotNet.Toolchains.InProcess.NoEmit;
 
 public static class Program
 {
 	public static void Main()
 	{
-		BenchmarkRunner.Run<Realistic>();
+		// Some local environments may run into issues with Windows Defender or 
+		// SentinelOne (and others) when running a benchmark. This ensures we
+		// keep our toolchain within our process and stops the above apps from blocking
+		// our benchmark process, but can slow the execution time.
+		var avSafeConfig = DefaultConfig.Instance
+			.AddJob(
+				Job.ShortRun
+					.WithToolchain(InProcessNoEmitToolchain.Instance)
+					.WithIterationCount(100)
+			);
+
+		BenchmarkRunner.Run<Realistic>(avSafeConfig);
 	}
 }
 
-[SimpleJob(invocationCount: 100, iterationCount: 100)]
 [MemoryDiagnoser]
 public class Realistic
 {
@@ -47,7 +60,7 @@ public void MoveCssInline_AllFlags()
 
 <meta http-equiv=""Content-Type"" content=""text/html; charset=UTF-8"" />
 <title>PreMailer Benchmark</title>
-	
+
 </head>
 
 <body bgcolor=""#123"">

diff --git a/PreMailer.Net/PreMailer.Net.Tests/CssStyleEquivalenceTests.cs b/PreMailer.Net/PreMailer.Net.Tests/CssStyleEquivalenceTests.cs
@@ -17,7 +17,7 @@ public void FindEquivalentStyles()
 
             var result = CssStyleEquivalence.FindEquivalent(nodewithoutselector, clazz);
 
-            Assert.Equal(1, result.Count);
+            Assert.Single(result);
         }
 
         [Fact]
@@ -32,7 +32,7 @@ public void FindEquivalentStylesNoMatchingStyles()
 
             var result = CssStyleEquivalence.FindEquivalent(nodewithoutselector, clazz);
 
-            Assert.Equal(0, result.Count);
+            Assert.Empty(result);
         }
     }
 }
diff --git a/PreMailer.Net/PreMailer.Net.Tests/ImportRuleCssSourceTests.cs b/PreMailer.Net/PreMailer.Net.Tests/ImportRuleCssSourceTests.cs
@@ -0,0 +1,217 @@
+using Xunit;
+using Moq;
+using PreMailer.Net.Downloaders;
+using PreMailer.Net.Sources;
+using System;
+using System.Text;
+using System.Collections.Generic;
+using System.Linq;
+
+namespace PreMailer.Net.Tests
+{
+	public class ImportRuleCssSourceTests
+	{
+		private readonly Mock<IWebDownloader> _webDownloader = new Mock<IWebDownloader>();
+
+		public ImportRuleCssSourceTests()
+		{
+			WebDownloader.SharedDownloader = _webDownloader.Object;
+		}
+
+		[Fact]
+		public void ItShould_DownloadAllImportedUrls_WhenCssContainsImportRules()
+		{
+			var baseUri = new Uri("https://a.com");
+			var urls = new List<string>() { "variables.css?v234", "/fonts.css", "https://fonts.google.com/css/test-font" };
+
+			var css = CreateCss(urls);
+			var sut = new ImportRuleCssSource();
+
+			sut.GetCss(baseUri, css);
+
+			foreach (var url in urls)
+			{
+				_webDownloader.Verify(w => w.DownloadString(It.Is<Uri>(u => u.Equals(new Uri(baseUri, url)))));
+			}
+		}
+
+		[Fact]
+		public void ItShould_NotDownloadUrls_WhenLevelIsGreaterThanTwo()
+		{
+			var baseUri = new Uri("https://a.com");
+			var urls = new List<string>() { "variables.css?v234", "/fonts.css", "https://fonts.google.com/css/test-font" };
+
+			var css = CreateCss(urls);
+			var sut = new ImportRuleCssSource();
+
+			sut.GetCss(baseUri, css, 2);
+
+			_webDownloader.Verify(w => w.DownloadString(It.IsAny<Uri>()), Times.Never());
+		}
+
+		[Fact]
+		public void ItShould_NotDownloadUrls_WhenCssIsEmpty()
+		{
+			var baseUri = new Uri("https://a.com");
+			var urls = new List<string>() { "variables.css?v234", "/fonts.css", "https://fonts.google.com/css/test-font" };
+
+			var css = CreateCss(urls);
+			var sut = new ImportRuleCssSource();
+
+			sut.GetCss(baseUri, string.Empty);
+
+			_webDownloader.Verify(w => w.DownloadString(It.IsAny<Uri>()), Times.Never());
+		}
+
+		[Fact]
+		public void ItShould_NotDownloadUrls_WhenCssIsNull()
+		{
+			var baseUri = new Uri("https://a.com");
+			var urls = new List<string>() { "variables.css?v234", "/fonts.css", "https://fonts.google.com/css/test-font" };
+
+			var css = CreateCss(urls);
+			var sut = new ImportRuleCssSource();
+
+			sut.GetCss(baseUri, null);
+
+			_webDownloader.Verify(w => w.DownloadString(It.IsAny<Uri>()), Times.Never());
+		}
+
+		[Fact]
+		public void ItShould_NotDownloadUrls_WhenCssDoesNotContainImportRules()
+		{
+			var baseUri = new Uri("https://a.com");
+			var urls = new List<string>() { "variables.css?v234", "/fonts.css", "https://fonts.google.com/css/test-font" };
+
+			var css = string.Join(Environment.NewLine, urls);
+			var sut = new ImportRuleCssSource();
+
+			sut.GetCss(baseUri, css);
+
+			_webDownloader.Verify(w => w.DownloadString(It.IsAny<Uri>()), Times.Never());
+		}
+
+		[Fact]
+		public void ItShould_LoadImportsRecursively_UntilLevelTwo()
+		{
+			// Arrange
+			var baseUri = new Uri("https://a.com");
+			var level0Css = "@import \"level1.css\";";
+			var level1Css = "@import \"level2.css\";";
+			var level2Css = "@import \"level3.css\";";
+
+			_webDownloader
+				.Setup(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/level1.css")))
+				.Returns(level1Css);
+			_webDownloader
+				.Setup(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/level2.css")))
+				.Returns(level2Css);
+
+			var sut = new ImportRuleCssSource();
+
+			// Act
+			var result = sut.GetCss(baseUri, level0Css).ToList();
+
+			// Assert
+			_webDownloader.Verify(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/level1.css")), Times.Once);
+			_webDownloader.Verify(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/level2.css")), Times.Once);
+			_webDownloader.Verify(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/level3.css")), Times.Never);
+			Assert.Equal(2, result.Count); // Should only contain level1.css and level2.css contents
+		}
+
+		[Fact]
+		public void ItShould_CacheDownloadedImports_AndNotDownloadTwice()
+		{
+			// Arrange
+			var baseUri = new Uri("https://a.com");
+			var css = "@import \"shared.css\"; @import \"also-shared.css\";";
+			var secondCss = "@import \"shared.css\";"; // References same file
+
+			_webDownloader
+				.Setup(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/shared.css")))
+				.Returns("h1 { color: red; }");
+			_webDownloader
+				.Setup(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/also-shared.css")))
+				.Returns("h2 { color: blue; }");
+
+			var sut = new ImportRuleCssSource();
+
+			// Act
+			var firstResult = sut.GetCss(baseUri, css);
+			var secondResult = sut.GetCss(baseUri, secondCss);
+
+			// Assert
+			_webDownloader.Verify(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/shared.css")), Times.Once);
+		}
+
+		[Fact]
+		public void ItShould_PreserveImportOrder_WhenProcessingImports()
+		{
+			// Arrange
+			var baseUri = new Uri("https://a.com");
+			var css = "@import \"first.css\"; @import \"second.css\"; @import \"third.css\";";
+			var firstCss = "first { order: 1; }";
+			var secondCss = "second { order: 2; }";
+			var thirdCss = "third { order: 3; }";
+
+			_webDownloader
+				.Setup(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/first.css")))
+				.Returns(firstCss);
+			_webDownloader
+				.Setup(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/second.css")))
+				.Returns(secondCss);
+			_webDownloader
+				.Setup(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/third.css")))
+				.Returns(thirdCss);
+
+			var sut = new ImportRuleCssSource();
+
+			// Act
+			var result = sut.GetCss(baseUri, css).ToList();
+
+			// Assert
+			Assert.Equal(3, result.Count);
+			Assert.Equal(firstCss, result[0]);
+			Assert.Equal(secondCss, result[1]);
+			Assert.Equal(thirdCss, result[2]);
+		}
+
+		[Fact]
+		public void ItShould_HandleCircularImports_WithoutInfiniteLoop()
+		{
+			// Arrange
+			var baseUri = new Uri("https://a.com");
+			var css = "@import \"circular1.css\";";
+			var circular1Css = "@import \"circular2.css\";";
+			var circular2Css = "@import \"circular1.css\";"; // Creates a circle
+
+			_webDownloader
+				.Setup(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/circular1.css")))
+				.Returns(circular1Css);
+			_webDownloader
+				.Setup(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/circular2.css")))
+				.Returns(circular2Css);
+
+			var sut = new ImportRuleCssSource();
+
+			// Act
+			var result = sut.GetCss(baseUri, css).ToList();
+
+			// Assert
+			Assert.Equal(2, result.Count); // Should contain both files exactly once
+			_webDownloader.Verify(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/circular1.css")), Times.Once);
+			_webDownloader.Verify(w => w.DownloadString(It.Is<Uri>(u => u.ToString() == "https://a.com/circular2.css")), Times.Once);
+		}
+
+		private string CreateCss(IEnumerable<string> imports)
+		{
+			var builder = new StringBuilder();
+			foreach (var import in imports)
+			{
+				builder.AppendLine($"@import \"{import}\";");
+			}
+
+			return builder.ToString();
+		}
+	}
+}
diff --git a/PreMailer.Net/PreMailer.Net.Tests/PreMailer.Net.Tests.csproj b/PreMailer.Net/PreMailer.Net.Tests/PreMailer.Net.Tests.csproj
@@ -1,17 +1,17 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net7.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
 
     <IsPackable>false</IsPackable>
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
-    <PackageReference Include="Moq" Version="4.20.70" />
-    <PackageReference Include="xunit" Version="2.7.0" />
-    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.7" />
-    <PackageReference Include="coverlet.collector" Version="6.0.2" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.12.0" />
+    <PackageReference Include="Moq" Version="4.20.72" />
+    <PackageReference Include="xunit" Version="2.9.3" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="3.0.1" />
+    <PackageReference Include="coverlet.collector" Version="6.0.4" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/PreMailer.Net/PreMailer.Net.Tests/PreMailerTests.cs b/PreMailer.Net/PreMailer.Net.Tests/PreMailerTests.cs
@@ -104,7 +104,7 @@ public void MoveCssInline_CrazyCssSelector_DoesNotThrowError()
 			}
 			catch (Exception ex)
 			{
-				Assert.True(false, ex.Message);
+				Assert.Fail(ex.Message);
 			}
 		}
 

diff --git a/PreMailer.Net/PreMailer.Net/Downloaders/WebDownloader.cs b/PreMailer.Net/PreMailer.Net/Downloaders/WebDownloader.cs
@@ -1,3 +1,4 @@
+using PreMailer.Net.Extensions;
 using System;
 using System.IO;
 using System.Net;
@@ -8,7 +9,7 @@ namespace PreMailer.Net.Downloaders
 	public class WebDownloader : IWebDownloader
 	{
 		private static IWebDownloader _sharedDownloader;
-
+		private const string CssMimeType = "text/css";
 		public static IWebDownloader SharedDownloader
 		{
 			get
@@ -29,8 +30,16 @@ public static IWebDownloader SharedDownloader
 		public string DownloadString(Uri uri)
 		{
 			var request = WebRequest.Create(uri);
+			request.Headers.Add(HttpRequestHeader.Accept, CssMimeType);
+
 			using (var response = request.GetResponse())
 			{
+				// We only support this operation for CSS file/content types coming back
+				// from the response. If we get something different, throw with the unsupported
+				// content type in the message
+				if(response.ParseContentType() != CssMimeType)
+					throw new NotSupportedException($"The Uri type is giving a response in unsupported content type '{response.ContentType}'.");
+
 				switch (response)
 				{
 					case HttpWebResponse httpWebResponse:

diff --git a/PreMailer.Net/PreMailer.Net/Extensions/WebResponseExtensions.cs b/PreMailer.Net/PreMailer.Net/Extensions/WebResponseExtensions.cs
@@ -0,0 +1,24 @@
+using System;
+using System.Net;
+
+namespace PreMailer.Net.Extensions
+{
+	public static class WebResponseExtensions
+	{
+		public static string ParseContentType(this WebResponse response)
+		{
+			if(response == null)
+				throw new NullReferenceException("Malformed response detected when parsing WebResponse Content-Type");
+
+			if(string.IsNullOrEmpty(response.ContentType))
+				throw new NullReferenceException("Malformed Content-Type response detected when parsing WebResponse");
+
+			var results = response.ContentType.Split(';');
+
+			if(results.Length == 0)
+				throw new FormatException("Malformed Content-Type response detected when parsing WebResponse");
+
+			return results[0];
+		}
+	}
+}
-Original file line number
+Diff line change
@@ Expand Up @@
     			}
     			catch (Exception ex)
     			{
-    				Assert.True(false, ex.Message);
+    				Assert.Fail(ex.Message);
     			}
     		}
@@ Expand Down @@