fix(mail): Keep content of Outlook downlevel-revealed conditional comments. Fixes #6167.

Documentation/SOGoInstallationGuide.asciidoc

@@ -841,14 +841,14 @@ Obiously, if your users can connect without specifying a domain, let this parame
 |Boolean enable password recovery with secret question or secondary e-mail. Default value is `NO`.
 
 |S |SOGoPasswordRecoveryBaseURLs
-| *Since 5.12.11*. Whitlist of base URL for password reset link.
+| *Since 5.12.11*. List of allowed base URL for password reset link.
 
 `SOGoPasswordRecoveryBaseURLs = ("https://demo.sogo.nu");`
 `SOGoPasswordRecoveryBaseURLs = ("https://platform1.sogo.nu", "https://platform2.sogo.nu);`
 
 Do not end the URLs by '/'.
 
-No defaults, will trhow an error if empty or unset.
+No defaults, will throw an error if empty or unset.
 
 |S |SOGoPasswordRecoveryDomains
 |List of domains where password recovery is enabled, for example `(example.org, example.net)`. If empty array `()`, disabled for all domains. If not set, enabled for all domains.

SoObjects/SOGo/NSString+Utilities.m

@@ -1089,14 +1089,75 @@ static int cssEscapingCount;
 }
 
 - (NSString *) cleanInvalidHTMLTags {
-  // Clean HTML invalid tags as reported in https://bugs.sogo.nu/view.php?id=5755
-  NSString *s;
-  
-  s = [NSString stringWithString: self];
-  s = [s stringByReplacingOccurrencesOfString:@"<!-->" withString:@""];
-  s = [s stringByReplacingOccurrencesOfString:@"<!--<!" withString:@"<!--"];
+  // Clean HTML invalid tags as reported in https://bugs.sogo.nu/view.php?id=5755 and https://bugs.sogo.nu/view.php?id=6167
+  // libxml2 (HTML4) sees a standalone empty comment (<!--> or <!--->) as an
+  // unterminated comment and drops the rest of the document, so remove it.
+  // Only do it outside of comments: the Outlook "downlevel-revealed" pattern
+  // <!--[if !mso]><!--> ... <!--<![endif]--> ends its first comment with
+  // "<!-->", and removing it would hide the content in between.
+  NSString *result;
+  unichar *in, *out;
+  NSUInteger len, i, j, o;
 
-  return s;
+  len = [self length];
+  if (len == 0)
+    return [NSString string];
+
+  // Work on raw buffers: linear time, bounded memory (no per-comment objects)
+  in = NSZoneMalloc (NULL, len * sizeof (unichar));
+  out = NSZoneMalloc (NULL, len * sizeof (unichar));
+  [self getCharacters: in];
+  i = o = 0;
+
+  while (i < len)
+    {
+      if (i + 3 < len
+          && in[i] == '<' && in[i+1] == '!' && in[i+2] == '-' && in[i+3] == '-')
+        {
+          j = i + 4;
+          if (j < len && in[j] == '>')
+            {
+              // <!-->
+              i = j + 1;
+              continue;
+            }
+          if (j + 1 < len && in[j] == '-' && in[j+1] == '>')
+            {
+              // <!--->
+              i = j + 2;
+              continue;
+            }
+
+          // Regular comment: keep it untouched up to its end ("-->" or "--!>",
+          // as libxml2 accepts both), or up to the end if unterminated
+          while (j < len)
+            {
+              if (j + 2 < len && in[j] == '-' && in[j+1] == '-' && in[j+2] == '>')
+                {
+                  j += 3;
+                  break;
+                }
+              if (j + 3 < len && in[j] == '-' && in[j+1] == '-'
+                  && in[j+2] == '!' && in[j+3] == '>')
+                {
+                  j += 4;
+                  break;
+                }
+              j++;
+            }
+          memcpy (out + o, in + i, (j - i) * sizeof (unichar));
+          o += j - i;
+          i = j;
+        }
+      else
+        out[o++] = in[i++];
+    }
+
+  result = [NSString stringWithCharacters: out length: o];
+  NSZoneFree (NULL, in);
+  NSZoneFree (NULL, out);
+
+  return result;
 }
 
 @end

Tests/Unit/TestNSString+Utilities.m

@@ -127,7 +127,15 @@
 - (void) test_stringCleanInvalidHTMLTags
 {
   testEquals([[NSString stringWithString:@"<div>Test<!--></div>"] cleanInvalidHTMLTags], @"<div>Test</div>");
-  testEquals([[NSString stringWithString:@"<div><!--[if !mso]><span>Test</span><!--<![endif]--></div>"] cleanInvalidHTMLTags], @"<div><!--[if !mso]><span>Test</span><!--[endif]--></div>");
+  testEquals([[NSString stringWithString:@"<div>Test<!---></div>"] cleanInvalidHTMLTags], @"<div>Test</div>");
+  testEquals([[NSString stringWithString:@"<div>A<!-->B<!-- c -->C<!-->D</div>"] cleanInvalidHTMLTags], @"<div>AB<!-- c -->CD</div>");
+  testEquals([[NSString stringWithString:@"<div><!--[if !mso]><span>Test</span><!--<![endif]--></div>"] cleanInvalidHTMLTags], @"<div><!--[if !mso]><span>Test</span><!--<![endif]--></div>");
+  // Outlook "downlevel-revealed" conditional comment: content must stay visible
+  testEquals([[NSString stringWithString:@"<a><!--[if !mso]><!--> <div><img src=\"x\"/></div> <!--<![endif]--><!--[if mso]><v:fill/><![endif]--></a>"] cleanInvalidHTMLTags], @"<a><!--[if !mso]><!--> <div><img src=\"x\"/></div> <!--<![endif]--><!--[if mso]><v:fill/><![endif]--></a>");
+  // libxml2 also ends a comment with "--!>"
+  testEquals([[NSString stringWithString:@"A<!-- x --!>B<!-->C"] cleanInvalidHTMLTags], @"A<!-- x --!>BC");
+  // unterminated comment is left as is
+  testEquals([[NSString stringWithString:@"<div>A<!-- B</div>"] cleanInvalidHTMLTags], @"<div>A<!-- B</div>");
 }
 
 - (void) test_stringRemoveHTMLTagsExceptAnchorTags

UI/MailerUI/UIxMailListActions.m

@@ -426,23 +426,23 @@
   moduleSettings = [us objectForKey: module];
 
   //Check sort value
-  sortValues = [NSArray arrayWithObjects: @"subject", @"from",
-                    @"date", @"size", @"arrival", nil];
+  sortValues = [NSArray arrayWithObjects: @"SUBJECT", @"FROM",
+                    @"DATE", @"SIZE", @"ARRIVAL", nil];
 
   if ([sort length] && [sortValues containsObject:sort])
-    {
-      if ([sort isEqualToString: [self defaultSortKey]] && !asc)
-	{
-	  if (moduleSettings && !dry)
+  {
+    if ([sort isEqualToString: [self defaultSortKey]] && !asc)
+	  {
+	    if (moduleSettings && !dry)
 	    {
 	      [moduleSettings removeObjectForKey: @"SortingState"];
 	      [us synchronize];
 	    }
-	}
-      else if (!dry)
-	{
-	  // Save the sorting state in the user settings
-	  if (!moduleSettings)
+	  }
+    else if (!dry)
+	  {
+	    // Save the sorting state in the user settings
+	    if (!moduleSettings)
 	    {
 	      moduleSettings = [NSMutableDictionary dictionary];
 	      [us setObject: moduleSettings forKey: module];
@@ -450,17 +450,18 @@
 	  [moduleSettings setObject: [NSArray arrayWithObjects: [sort lowercaseString], [NSString stringWithFormat: @"%d", (asc ? 1 : 0)], nil]
 			     forKey: @"SortingState"];
 	  [us synchronize];
-	}
-    }
+	  }
+  }
   else if (moduleSettings)
-    {
-      NSArray *sortState = [moduleSettings objectForKey: @"SortingState"];
-      if ([sortState count])
-	{
-	  sort = [[sortState objectAtIndex: 0] uppercaseString];
-	  asc = [[sortState objectAtIndex: 1] boolValue];
-	}
-    }
+  {
+    NSArray *sortState = [moduleSettings objectForKey: @"SortingState"];
+    if ([sortState count])
+	  {
+	    sort = [[sortState objectAtIndex: 0] uppercaseString];
+	    asc = [[sortState objectAtIndex: 1] boolValue];
+	  }
+  }
+
   if (![sort length])
     sort = [self defaultSortKey];