Improve HTML processing for placeholders that are using formatting ar…

…guments (#46)

* Improve HTML processing for placeholders that are using formatting arguments

- Introduced new test methods `SimpleHtmlContent` and `ConditionalHtmlContent` in `Message_Html.cs`.
- Modified `HtmlBodyBuilder` constructor to replace placeholders before parsing HTML to avoid entity encoding issues.
- Simplified `ReplaceImgSrcByCid` method in `HtmlBodyBuilder.cs`.
- Updated `GetConfiguredMailSmartFormatter` in `MailMergeMessage.cs` to set `ParseInputAsHtml` to `true`.
- Corrected typos in comments
- Refactor some unit tests

* Bump version to v5.12.2

Src/Directory.Build.props

@@ -8,8 +8,8 @@
         <Copyright>Copyright 2011-$(CurrentYear) axuno, MailMergeLib Project maintainers and contributers</Copyright>
         <RepositoryUrl>https://github.com/axuno/MailMergeLib.git</RepositoryUrl>
         <PublishRepositoryUrl>true</PublishRepositoryUrl>
-        <Version>5.12.0</Version>
-        <FileVersion>5.12.0</FileVersion>
+        <Version>5.12.2</Version>
+        <FileVersion>5.12.2</FileVersion>
         <AssemblyVersion>5.0.0.0</AssemblyVersion> <!--only update AssemblyVersion with major releases -->
         <LangVersion>latest</LangVersion>
         <EnableNETAnalyzers>true</EnableNETAnalyzers>

Src/MailMergeLib.Tests/MailMergeLib.Tests.csproj

@@ -8,7 +8,6 @@
         <AssemblyName>MailMergeLib.Tests</AssemblyName>
         <AssemblyOriginatorKeyFile>../MailMergeLib/MailMergeLib.snk</AssemblyOriginatorKeyFile>
         <SignAssembly>true</SignAssembly>
-        <PublicSign Condition=" '$(OS)' != 'Windows_NT' ">true</PublicSign>
         <IsTestProject>true</IsTestProject>
         <Nullable>enable</Nullable>
         <LangVersion>latest</LangVersion>

Src/MailMergeLib.Tests/Message_Html.cs

@@ -31,10 +31,10 @@ public void MimeMessageSize()
             var mimeMessage = mmm.GetMimeMessage(null);
 
             var size = MailMergeLib.Tools.CalcMessageSize(mimeMessage);
-            Assert.That(size > 0, Is.True);
+            Assert.That(size, Is.GreaterThan(0));
         }
 
-        Assert.That(MailMergeLib.Tools.CalcMessageSize(null) == 0, Is.True);
+        Assert.That(MailMergeLib.Tools.CalcMessageSize(null), Is.EqualTo(0));
     }
 
     [Test]
@@ -105,16 +105,24 @@ public void HtmlMailMergeWithInlineAndAtt()
 
         Assert.Multiple(() =>
         {
-            Assert.That(((MailboxAddress) msg.From.First()).Address == dataItem.SenderAddr, Is.True);
-            Assert.That(((MailboxAddress) msg.To.First()).Address == dataItem.MailboxAddr, Is.True);
-            Assert.That(((MailboxAddress) msg.To.First()).Name == dataItem.Name, Is.True);
-            Assert.That(msg.Headers[HeaderId.Organization] == mmm.Config.Organization, Is.True);
-            Assert.That(msg.Priority == mmm.Config.Priority, Is.True);
-            Assert.That(msg.Attachments.FirstOrDefault(a => ((MimePart) a).FileName == "Log file from {Date:yyyy-MM-dd}.log".Replace("{Date:yyyy-MM-dd}", dataItem.Date.ToString("yyyy-MM-dd"))) != null, Is.True);
-            Assert.That(msg.Subject == mmm.Subject.Replace("{Date:yyyy-MM-dd}", dataItem.Date.ToString("yyyy-MM-dd")), Is.True);
-            Assert.That(msg.HtmlBody.Contains(dataItem.Success ? "succeeded" : "failed"), Is.True);
-            Assert.That(msg.TextBody.Contains(dataItem.Success ? "succeeded" : "failed"), Is.True);
-            Assert.That(msg.BodyParts.Any(bp => bp.ContentDisposition?.Disposition == ContentDisposition.Inline && bp.ContentType.IsMimeType("image", "jpeg")), Is.True);
+            Assert.That(((MailboxAddress) msg.From.First()).Address, Is.EqualTo(dataItem.SenderAddr));
+            Assert.That(((MailboxAddress) msg.To.First()).Address, Is.EqualTo(dataItem.MailboxAddr));
+            Assert.That(((MailboxAddress) msg.To.First()).Name, Is.EqualTo(dataItem.Name));
+            Assert.That(msg.Headers[HeaderId.Organization], Is.EqualTo(mmm.Config.Organization));
+            Assert.That(msg.Priority, Is.EqualTo(mmm.Config.Priority));
+            Assert.That(
+                msg.Attachments.FirstOrDefault(a =>
+                    ((MimePart) a).FileName ==
+                    "Log file from {Date:yyyy-MM-dd}.log".Replace("{Date:yyyy-MM-dd}",
+                        dataItem.Date.ToString("yyyy-MM-dd"))), Is.Not.EqualTo(null));
+            Assert.That(msg.Subject,
+                Is.EqualTo(mmm.Subject.Replace("{Date:yyyy-MM-dd}", dataItem.Date.ToString("yyyy-MM-dd"))));
+            Assert.That(msg.HtmlBody, Does.Contain(dataItem.Success ? "succeeded" : "failed"));
+            Assert.That(msg.TextBody, Does.Contain(dataItem.Success ? "succeeded" : "failed"));
+            Assert.That(
+                msg.BodyParts.Any(bp =>
+                    bp.ContentDisposition?.Disposition == ContentDisposition.Inline &&
+                    bp.ContentType.IsMimeType("image", "jpeg")), Is.True);
         });
 
         MailMergeMessage.DisposeFileStreams(msg);
@@ -134,12 +142,12 @@ public void HtmlStreamAttachments()
             mmm.StreamAttachments.Add(new StreamAttachment(stream, streamAttFilename, "text/plain"));
         }
 
-        Assert.That(mmm.StreamAttachments.Count == 1, Is.True);
+        Assert.That(mmm.StreamAttachments.Count, Is.EqualTo(1));
         mmm.StreamAttachments.Clear();
-        Assert.That(mmm.StreamAttachments.Count == 0, Is.True);
+        Assert.That(mmm.StreamAttachments.Count, Is.EqualTo(0));
 
         mmm.StreamAttachments = streamAttachments;
-        Assert.That(mmm.StreamAttachments.Count == 2, Is.True);
+        Assert.That(mmm.StreamAttachments.Count, Is.EqualTo(2));
     }
 
 
@@ -199,8 +207,8 @@ public void HtmlMailMergeWithMoreEqualInlineAtt()
 
         Assert.Multiple(() =>
         {
-            Assert.That(new HtmlParser().ParseDocument((string) msg.HtmlBody).All.Count(m => m is IHtmlImageElement) == 3, Is.True);
-            Assert.That(msg.BodyParts.Count(bp => bp.ContentDisposition?.Disposition == ContentDisposition.Inline && bp.ContentType.IsMimeType("image", "jpeg")) == 1, Is.True);
+            Assert.That(new HtmlParser().ParseDocument((string) msg.HtmlBody).All.Count(m => m is IHtmlImageElement), Is.EqualTo(3));
+            Assert.That(msg.BodyParts.Count(bp => bp.ContentDisposition?.Disposition == ContentDisposition.Inline && bp.ContentType.IsMimeType("image", "jpeg")), Is.EqualTo(1));
         });
 
         MailMergeMessage.DisposeFileStreams(msg);
@@ -341,4 +349,42 @@ public void SearchAndReplaceFilename(string text, string expected)
         var result = mmm.SearchAndReplaceVarsInFilename(text, dataItem);
         Assert.That(result, Is.EqualTo(expected));
     }
+
+    [Test]
+    public void SimpleHtmlContent()
+    {
+        using var mmm = new MailMergeMessage("subject", "plain text", "<html><head></head><body>{Name}{Value:0.00}</body></html>");
+        mmm.MailMergeAddresses.Add(new MailMergeAddress(MailAddressType.To, "john@specimen.com"));
+        mmm.MailMergeAddresses.Add(new MailMergeAddress(MailAddressType.From, "no-reply@specimen.com"));
+        var dataItem = new { Name = "John", Value = 2 };
+        var msg = mmm.GetMimeMessage(dataItem);
+        Assert.That(msg.HtmlBody, Does.Contain(dataItem.Name));
+    }
+
+    [Test]
+    public void HtmlBodyBuilder()
+    {
+        using var mmm = new MailMergeMessage("subject", "plain text", "<html><head></head><body>{Name}{Value:0.00}</body></html>");
+        mmm.MailMergeAddresses.Add(new MailMergeAddress(MailAddressType.To, "john@specimen.com"));
+        mmm.MailMergeAddresses.Add(new MailMergeAddress(MailAddressType.From, "no-reply@specimen.com"));
+        var dataItem = new { Name = "John", Value = 2 };
+        var bb = new HtmlBodyBuilder(mmm, dataItem);
+
+        Assert.That(bb.DocHtml, Does.Contain(dataItem.Name));
+    }
+
+    [TestCase("John", 0, "John: Nothing")]
+    [TestCase("John", 2, "John: Double")]
+    [TestCase("John", 3, "John: More")]
+    public void ConditionalHtmlContent(string name, int value, string expected)
+    {
+        // Note: The ConditionalFormatter makes use of characters <, >, =, &, ? and :
+        //       which must not be encoded to &lt;, &gt;, &amp; etc.
+        using var mmm = new MailMergeMessage("subject", "plain text", "<html><head></head><body>{Name}: {Value:cond:<1?Nothing|=2?Double|More}</body></html>");
+        mmm.MailMergeAddresses.Add(new MailMergeAddress(MailAddressType.To, "john@specimen.com"));
+        mmm.MailMergeAddresses.Add(new MailMergeAddress(MailAddressType.From, "no-reply@specimen.com"));
+        var dataItem = new { Name = name, Value = value };
+        var msg = mmm.GetMimeMessage(dataItem);
+        Assert.That(msg.HtmlBody, Does.Contain(expected));
+    }
 }

Src/MailMergeLib/BodyBuilderBase.cs

@@ -25,7 +25,7 @@ protected BodyBuilderBase()
     public ContentEncoding TextTransferEncoding { get; set; }
 
     /// <summary>
-    /// Gets the ready made body part for a mail message.
+    /// Gets the ready-made body part for a mail message.
     /// </summary>
     public abstract MimeEntity GetBodyPart();
-}
+}

Src/MailMergeLib/HtmlBodyBuilder.cs

@@ -38,10 +38,15 @@ public HtmlBodyBuilder(MailMergeMessage mailMergeMessage, object? dataItem)
         _dataItem = dataItem;
         BinaryTransferEncoding = mailMergeMessage.Config.BinaryTransferEncoding;
 
+        // We need to replace placeholders in the HTML text before parsing as HTML
+        // because SmartFormat extensions may use characters like '<', '&' or '>' in placeholders.
+        // These characters would be encoded to HTML entities by AngleSharp
+        // and thus not be interpreted correctly in SmartFormat.
+        var htmlFormatted = mailMergeMessage.SearchAndReplaceVars(mailMergeMessage.HtmlText, dataItem);
         // Create a new parser front-end (can be re-used)
         var parser = new HtmlParser();
-        //Just get the DOM representation
-        _htmlDocument = parser.ParseDocument(mailMergeMessage.HtmlText);
+        // Just get the DOM representation
+        _htmlDocument = parser.ParseDocument(htmlFormatted);
     }
 
     /// <summary>
@@ -55,7 +60,7 @@ public HtmlBodyBuilder(MailMergeMessage mailMergeMessage, object? dataItem)
     public string DocHtml => _htmlDocument.ToHtml();
 
     /// <summary>
-    /// Gets the ready made body part for a mail message either 
+    /// Gets the ready-made body part for a mail message either 
     /// - as TextPart, if there are no inline attachments
     /// - as MultipartRelated with a TextPart and one or more MimeParts of type inline attachments
     /// </summary>
@@ -89,17 +94,12 @@ public override MimeEntity GetBodyPart()
 
         ReplaceImgSrcByCid();
 
-        // replace placeholders only in the HTML Body, because e.g. 
-        // in the header there may be CSS definitions with curly brace which collide with SmartFormat {placeholders}
-        if (_htmlDocument.Body != null)
-            _htmlDocument.Body.InnerHtml =
-                _mailMergeMessage.SearchAndReplaceVars(_htmlDocument.Body.InnerHtml, _dataItem) ?? string.Empty;
-
         var htmlTextPart = new TextPart("html")
         {
             ContentTransferEncoding = TextTransferEncoding
         };
-        htmlTextPart.SetText(CharacterEncoding, DocHtml);  // MimeKit.ContentType.Charset is set using CharacterEncoding
+        // MimeKit.ContentType.Charset is set using CharacterEncoding
+        htmlTextPart.SetText(CharacterEncoding, _htmlDocument.ToHtml());
         htmlTextPart.ContentId = MimeUtils.GenerateMessageId();
 
         if (!InlineAtt.Any())
@@ -214,30 +214,25 @@ private void ReplaceImgSrcByCid()
             var filename = _mailMergeMessage.SearchAndReplaceVarsInFilename(srcUri.LocalPath, _dataItem);
             try
             {
-                if (filename != null)
+                if (!fileList.TryGetValue(filename, out var cidForExistingFile))
+                {
+                    var fileInfo = new FileInfo(filename);
+                    var contentType = MimeTypes.GetMimeType(filename);
+                    var cid = MimeUtils.GenerateMessageId();
+                    InlineAtt.Add(new FileAttachment(fileInfo.FullName,
+                        MakeCid(string.Empty, cid, fileInfo.Extension), contentType));
+                    srcAttr.Value = MakeCid("cid:", cid, fileInfo.Extension);
+                    fileList.Add(fileInfo.FullName, cid);
+                }
+                else
                 {
-                    if (!fileList.ContainsKey(filename))
-                    {
-                        var fileInfo = new FileInfo(filename);
-                        var contentType = MimeTypes.GetMimeType(filename);
-                        var cid = MimeUtils.GenerateMessageId();
-                        InlineAtt.Add(new FileAttachment(fileInfo.FullName,
-                            MakeCid(string.Empty, cid, fileInfo.Extension), contentType));
-                        srcAttr.Value = MakeCid("cid:", cid, fileInfo.Extension);
-                        fileList.Add(fileInfo.FullName, cid);
-                    }
-                    else
-                    {
-                        var cidForExistingFile = fileList[filename];
-                        var fileInfo = new FileInfo(filename);
-                        srcAttr.Value = MakeCid("cid:", cidForExistingFile, fileInfo.Extension);
-                    }
+                    var fileInfo = new FileInfo(filename);
+                    srcAttr.Value = MakeCid("cid:", cidForExistingFile, fileInfo.Extension);
                 }
             }
             catch
             {
-                BadInlineFiles.Add(filename ?? "(null)");
-                continue;
+                BadInlineFiles.Add(filename);
             }
         }
     }
@@ -246,11 +241,11 @@ private void ReplaceImgSrcByCid()
     /// Makes the content identifier (CID)
     /// </summary>
     /// <param name="prefix">i.e. normally "cid:"</param>
-    /// <param name="contentId">unique indentifier</param>
+    /// <param name="contentId">unique identifier</param>
     /// <param name="fileExt">file extension, so that content type can be easily identified. May be string.empty</param>
     /// <returns></returns>
     private static string MakeCid(string prefix, string contentId, string fileExt)
     {
         return prefix + contentId + fileExt.Replace('.', '-');
     }
-}
+}

Src/MailMergeLib/MailMergeMessage.cs

@@ -340,6 +340,10 @@ private MailSmartFormatter GetConfiguredMailSmartFormatter(bool invokedFromConst
         var currentSmartSettings = invokedFromConstructor
             ? new SmartSettings()
             : SmartFormatter.Settings;
+        // Parse content of <script> and <style> tags as literal text, 
+        // i.e. placeholder parsing is disabled for these tags
+        currentSmartSettings.Parser.ParseInputAsHtml = true;
+
         var smartFormatter = new MailSmartFormatter(Config.SmartFormatterConfig, currentSmartSettings);
         smartFormatter.OnFormattingFailure += (sender, args) => { _badVariableNames.Add(args.Placeholder); };
         smartFormatter.Parser.OnParsingFailure += (sender, args) => { _parseExceptions.Add(new ParseException(args.Errors.MessageShort, args.Errors)); };
@@ -356,7 +360,7 @@ private MailSmartFormatter GetConfiguredMailSmartFormatter(bool invokedFromConst
     /// <remarks>
     /// In case <see cref="SmartFormat.Core.Settings.SmartSettings.FormatErrorAction"/> == ErrorAction.ThrowError
     /// or <see cref="SmartFormat.Core.Settings.SmartSettings.ParseErrorAction"/> == ErrorAction.ThrowError
-    /// we simple catch the exception and simulate setting ErrorAction.MaintainTokens.
+    /// we simply catch the exception and simulate setting ErrorAction.MaintainTokens.
     /// Note: We track such errors by subscribing to Parser.OnParsingFailure and Formatter.OnFormattingFailure.
     /// </remarks>
     internal string SearchAndReplaceVars(string text, object? dataItem)
@@ -390,7 +394,7 @@ internal string SearchAndReplaceVars(string text, object? dataItem)
     /// <remarks>
     /// In case <see cref="SmartFormat.Core.Settings.SmartSettings.FormatErrorAction"/> == ErrorAction.ThrowError
     /// or <see cref="SmartFormat.Core.Settings.SmartSettings.ParseErrorAction"/> == ErrorAction.ThrowError
-    /// we simple catch the exception and simulate setting ErrorAction.MaintainTokens.
+    /// we simply catch the exception and simulate setting ErrorAction.MaintainTokens.
     /// Note: We track such errors by subscribing to Parser.OnParsingFailure and Formatter.OnFormattingFailure.
     /// </remarks>
     internal string SearchAndReplaceVarsInFilename(string text, object? dataItem)

appveyor.yml

@@ -1,4 +1,4 @@
-version: 5.12.1.{build} # Only change for mayor versions (e.g. 6.0.0)
+version: 5.12.2.{build} # Only change for mayor versions (e.g. 6.0.0)
 environment:
   DOTNET_SKIP_FIRST_TIME_EXPERIENCE: true 
   matrix:
@@ -27,7 +27,7 @@ for:
       - ps: dotnet restore --verbosity quiet
       - ps: dotnet add .\MailMergeLib.Tests\MailMergeLib.Tests.csproj package AltCover
       - ps: |
-          $version = "5.12.1"
+          $version = "5.12.2"
           $versionFile = $version + "." + ${env:APPVEYOR_BUILD_NUMBER}
 
           if ($env:APPVEYOR_PULL_REQUEST_NUMBER) {