System.Net.Mail Validation improvements

src/libraries/System.Net.Mail/src/Resources/Strings.resx

@@ -155,7 +155,13 @@
     <value>The specified port is invalid. The port must be greater than 0.</value>
   </data>
   <data name="MailHeaderInvalidCID" xml:space="preserve">
-    <value>The ContentID cannot contain a '&lt;' or '&gt;' character.</value>
+    <value>The ContentID cannot contain a '&lt;', '&gt;', CR, or LF character.</value>
+  </data>
+  <data name="MailHeaderInvalid" xml:space="preserve">
+    <value>The value cannot contain a CR or LF character.</value>
+  </data>
+  <data name="DispositionTypeInvalid" xml:space="preserve">
+    <value>The value cannot contain disallowed characters.</value>
   </data>
   <data name="MailServerResponse" xml:space="preserve">
     <value>The server response was: {0}</value>

src/libraries/System.Net.Mail/src/System/Net/Mail/Attachment.cs

@@ -219,7 +219,7 @@ public string ContentId
                 }
                 else
                 {
-                    if (value.AsSpan().ContainsAny('<', '>')) // invalid chars
+                    if (value.AsSpan().ContainsAny("<>\r\n"))
                     {
                         throw new ArgumentException(SR.MailHeaderInvalidCID, nameof(value));
                     }
@@ -268,7 +268,20 @@ internal Uri? ContentLocation
 
             set
             {
-                _part.ContentLocation = value == null ? null : value.IsAbsoluteUri ? value.AbsoluteUri : value.OriginalString;
+                if (value == null)
+                {
+                    _part.ContentLocation = null;
+                    return;
+                }
+
+                string valueCandidate = value.IsAbsoluteUri ? value.AbsoluteUri : value.OriginalString;
+
+                if (MailBnfHelper.HasCROrLF(valueCandidate))
+                {
+                    throw new ArgumentException(SR.MailHeaderInvalid, nameof(value));
+                }
+
+                _part.ContentLocation = valueCandidate;
             }
         }
 

src/libraries/System.Net.Mail/src/System/Net/Mime/ContentDisposition.cs

@@ -79,6 +79,13 @@ public string DispositionType
             {
                 ArgumentException.ThrowIfNullOrEmpty(value);
 
+                // RFC allows IANA registered token or X- extension token. We do not validate extensively.
+                // For backward compatibility we restrict only chars which can interfere with other part of syntax.
+                if (!MimeBasePart.IsAscii(value, false) || value.AsSpan().ContainsAny(';', '"'))
+                {
+                    throw new ArgumentException(SR.DispositionTypeInvalid, nameof(value));
+                }
+
                 _isChanged = true;
                 _dispositionType = value;
             }

src/libraries/System.Net.Mail/src/System/Net/Mime/ContentType.cs

@@ -193,7 +193,7 @@ private static void EncodeToBuffer(string value, StringBuilder builder, bool all
         {
             if (MimeBasePart.IsFullyEncoded(value)) // Manually encoded elsewhere, pass through
             {
-                builder.Append('\"').Append(value).Append('"');
+                builder.Append('"').Append(value).Append('"');
             }
             else if ((allowUnicode && !MailBnfHelper.HasCROrLF(value)) // Unicode without CL or LF's
                 || MimeBasePart.IsAscii(value, false)) // Ascii

src/libraries/System.Net.Mail/tests/Functional/AlternateViewTest.cs

@@ -72,5 +72,38 @@ public void CreateAlternateViewFromStringEncodingNull()
             AlternateView av = AlternateView.CreateAlternateViewFromString("<p>test message</p>", null, "text/html");
             Assert.Equal(Text.Encoding.ASCII.BodyName, av.ContentType.CharSet);
         }
+
+        [Theory]
+        [InlineData("", UriKind.Relative)]
+        [InlineData("aaaa", UriKind.Relative)]
+        [InlineData("aaaň", UriKind.Relative)]
+        [InlineData("aa-aa", UriKind.Relative)]
+        [InlineData("aa_aa", UriKind.Relative)]
+        [InlineData("aa\"aa", UriKind.Relative)]
+        [InlineData("aa\\aa", UriKind.Relative)]
+        [InlineData("aa/aa", UriKind.Relative)]
+        [InlineData("aa@aa", UriKind.Relative)]
+        [InlineData("aa; bb=cc", UriKind.Relative)]
+        [InlineData("C:\\foo.txt", UriKind.Absolute)]
+        public void ContentLocation_Validation_Valid(string uri, UriKind kind)
+        {
+            AlternateView av = AlternateView.CreateAlternateViewFromString("foo");
+            av.BaseUri = new Uri(uri, kind);
+        }
+
+        [Theory]
+        [InlineData("aa\raa", UriKind.Relative)]
+        [InlineData("aa\naa", UriKind.Relative)]
+        [InlineData("aa\r\naa", UriKind.Relative)]
+        [InlineData("aa\r\n aa", UriKind.Relative)]
+        public void ContentLocation_Validation_Invalid(string uri, UriKind kind)
+        {
+            AlternateView av = AlternateView.CreateAlternateViewFromString("foo");
+            Uri u = new Uri(uri, kind);
+            Assert.Throws<ArgumentException>(() =>
+            {
+                av.BaseUri = u;
+            });
+        }
     }
 }

src/libraries/System.Net.Mail/tests/Functional/AttachmentTest.cs

@@ -103,14 +103,155 @@ public void ContentType()
         [Fact]
         public void NameEncoding()
         {
-            Attachment attach = Attachment.CreateAttachmentFromString("test", "attachment-name");
-            Assert.Null(attach.NameEncoding);
-            Attachment a = new Attachment(new MemoryStream(), "myname");
+            Attachment a;
+
+            a = Attachment.CreateAttachmentFromString("test", "attachment-name");
+            Assert.Null(a.NameEncoding);
+
+            a = new Attachment(new MemoryStream(), "attachmentname");
             Assert.Null(a.NameEncoding);
-            a = new Attachment(new MemoryStream(), "myname\u3067");
+
+            a = new Attachment(new MemoryStream(), "attachmentname\u3067");
+            Assert.Null(a.NameEncoding);
+        }
+
+        [Fact]
+        public void NameParsingAndEncodingDetection_Basics()
+        {
+            Attachment a;
+
+            // smoke test
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?Q?attachmentname?=");
+            Assert.Equal("attachmentname", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // lower case charset
+            a = new Attachment(new MemoryStream(), "=?iso-8859-1?Q?attachmentname?=");
+            Assert.Equal("attachmentname", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // Q encoding
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?Q?attachment=20name?=");
+            Assert.Equal("attachment name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // Q encoding (lowercase)
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?q?attachment=20name?=");
+            Assert.Equal("attachment name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // B encoding
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?B?YXR0YWNobWVudCBuYW1l?=");
+            Assert.Equal("attachment name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // B encoding (lowercase)
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?b?YXR0YWNobWVudCBuYW1l?=");
+            Assert.Equal("attachment name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // space alternate
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?q?attachment_name?=");
+            Assert.Equal("attachment name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // question mark alternate
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?q?attachment=3Fname?=");
+            Assert.Equal("attachment?name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // underscore alternate
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?q?attachment=5Fname?=");
+            Assert.Equal("attachment_name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // multiple encoded-words
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?Q?attachment=20?= =?ISO-8859-1?Q?name?=");
+            Assert.Equal("attachment name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // whitespace between encoded-words
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?Q?attachment=20?=    =?ISO-8859-1?Q?name?=");
+            Assert.Equal("attachment name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // tab between encoded-words
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?Q?attachment=20?= \t   =?ISO-8859-1?Q?name?=");
+            Assert.Equal("attachment name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // new-line whitespace between encoded-words
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?Q?attachment=20?=\r\n   =?ISO-8859-1?Q?name?=");
+            Assert.Equal("attachment name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+
+            // multiple different encodings
+            a = new Attachment(new MemoryStream(), "=?ISO-8859-1?Q?attachment=20?= =?UTF-8?Q?name?=");
+            Assert.Equal("attachment name", a.Name);
+            Assert.Equal(Encoding.Latin1, a.NameEncoding);
+        }
+
+        [Theory]
+        [InlineData("=?Q?foo?=")] // missing charset
+        [InlineData("=?ISO-8859-1?foo?=")] // missing encoding
+        [InlineData("=?ISO-8859-1?qb?foo?=")] // two letter encoding
+        [InlineData("=?ISO-8859-1?Q?foo?")] // missing end =
+        [InlineData("=?ISO-8859-1?Q?foo")] // missing whole end
+        [InlineData("=?ISO-8859-1?Q?foo_?A?=")] // broken Q encoding, ? in text
+        [InlineData("=?ISO-8859-1?Q?foo bar?=")] // broken Q encoding, space in text
+        [InlineData("=?ISO-8859-1?Q?foo\tbar?=")] // broken Q encoding, tab in text
+        [InlineData("=?ISO-8859-1?Q?foo\rbar?=")] // broken Q encoding, new line (CR) in text
+        [InlineData("=?ISO-8859-1?Q?foo\nbar?=")] // broken Q encoding, new line (LF) in text
+        [InlineData("=?ISO-8859-1?Q?foo\r\nbar?=")] // broken Q encoding, new line (CRLF) in text
+        [InlineData("=?ISO?8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO(8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO<8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO@8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO,8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO;8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO:8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO/8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO[8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO.8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO=8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO\"8859-1?Q?foo_bar?=")] // prohibited char in charset
+        [InlineData("=?ISO-8859-1?Q??foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q(?foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q<?foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q@?foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q,?foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q;?foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q:?foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q/?foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q[?foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q.?foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q=?foo_bar?=")] // prohibited char in encoding
+        [InlineData("=?ISO-8859-1?Q\"?foo_bar?=")] // prohibited char in encoding
+        public void NameParsingAndEncodingDetection_BadInputs(string attachmentName)
+        {
+            Attachment a = new Attachment(new MemoryStream(), attachmentName);
+            Assert.Equal(attachmentName, a.Name);
             Assert.Null(a.NameEncoding);
         }
 
+        [Fact]
+        public void NameParsingAndEncodingDetection_BadInputsThrowing()
+        {
+            // Bad Base64 encoding
+            Assert.Throws<FormatException>(() =>
+            {
+                using MemoryStream ms = new();
+                new Attachment(ms, "=?ISO-8859-1?B?YXR0YWNobWV@@@@@@@@udCBuYW1l?=");
+            });
+
+            // broken Q encoding, invalid hex value
+            Assert.Throws<FormatException>(() =>
+            {
+                using MemoryStream ms = new();
+                new Attachment(ms, "=?ISO-8859-1?Q?foo_=XY?=");
+            });
+        }
+
         [Fact]
         public void ContentStream()
         {
@@ -136,5 +277,61 @@ public void TransferEncodingTest()
             Attachment attach = Attachment.CreateAttachmentFromString("test", "attachment-name");
             Assert.Equal(TransferEncoding.QuotedPrintable, attach.TransferEncoding);
         }
+
+        // For backward compatibility, we allows some non-compliant with RFC2045
+        [Theory]
+        [InlineData("a")]
+        [InlineData("fň")]
+        [InlineData("foo-_:;()[]bar")]
+        [InlineData("foo\"bar")]
+        [InlineData("foo=bar")]
+        [InlineData("foo@bar")]
+        public void ContentId_Validation_Valid(string contentId)
+        {
+            Attachment attach = Attachment.CreateAttachmentFromString("test", "attachment-name");
+            attach.ContentId = contentId;
+            Assert.Equal(contentId, attach.ContentId);
+        }
+
+        [Theory]
+        [InlineData("foo<bar")]
+        [InlineData("foo>bar")]
+        [InlineData("<foobar>")]
+        [InlineData("foo\rbar")]
+        [InlineData("foo\nbar")]
+        [InlineData("foo\r\nbar")]
+        public void ContentId_Validation_Invalid(string contentId)
+        {
+            Attachment attach = Attachment.CreateAttachmentFromString("test", "attachment-name");
+            Assert.Throws<ArgumentException>(() =>
+            {
+                attach.ContentId = contentId;
+            });
+        }
+
+        [Fact]
+        public void ContentId_Validation_Empty()
+        {
+            // toggles between spcified and random generated content id
+
+            Attachment attach = Attachment.CreateAttachmentFromString("test", "attachment-name");
+
+            attach.ContentId = "foo";
+            Assert.Equal("foo", attach.ContentId);
+
+            attach.ContentId = "";
+            var guid1 = attach.ContentId;
+            Guid.ParseExact(guid1, "D");
+
+            attach.ContentId = "bar";
+            Assert.Equal("bar", attach.ContentId);
+
+            attach.ContentId = null;
+            var guid2 = attach.ContentId;
+            Guid.ParseExact(guid2, "D");
+
+            Assert.NotEqual(guid1, guid2);
+        }
+
     }
 }

src/libraries/System.Net.Mail/tests/Functional/ContentDispositionTest.cs

@@ -355,5 +355,59 @@ public void CultureSensitiveSetDateViaProperty_ShouldPersistToProprtyAndParamete
                 CultureInfo.CurrentCulture = origCulture;
             }
         }
+
+        [Theory]
+        [InlineData("=?utf-8?B?SGVsbG8=?=")]
+        [InlineData("=?utf-8?Q?Hello=20world?=")]
+        [InlineData("=?utf-8?B?SGVsbG8=?=\r\n =?utf-8?B?d29ybGQ=?=")]
+        [InlineData("=?utf-8?Q?foo?=?bar\"; filename=\"foo.txt")]
+        [InlineData("=?utf-8?B?Zm9v?=\"; filename=\"foo.txt")]
+        [InlineData("=?utf-8?Q?foo\"; filename=\"foo.txt?=")]
+        [InlineData("=?utf-8?Q?foo\"bar?=")]
+        [InlineData("=?utf-8?B?Zm9v?=trailing")]
+        public static void FileName_ParsingSerialization_Roundtrip(string encoded)
+        {
+            var cd = new ContentDisposition();
+            cd.FileName = encoded;
+
+            // Property roundtrip
+            Assert.Equal(encoded, cd.FileName);
+
+            // Serialize/deserialize roundtrip
+            string serialized = cd.ToString();
+            var reparsed = new ContentDisposition(serialized);
+            Assert.Equal(encoded, reparsed.FileName);
+
+            Assert.Equal(cd.Parameters.Count, reparsed.Parameters.Count);
+        }
+
+        [Theory]
+        [InlineData("inline")]
+        [InlineData("attachment")]
+        [InlineData("foo")]
+        [InlineData("foo_bar")]
+        [InlineData("foo@bar")]
+        public void ContentDisposition_Type_Validation_Valid(string dispositionType)
+        {
+            var cd = new ContentDisposition();
+            cd.DispositionType = dispositionType;
+            Assert.Equal(dispositionType, cd.DispositionType);
+        }
+
+        [Theory]
+        [InlineData("foo\rbar")]
+        [InlineData("foo\nbar")]
+        [InlineData("foo\r\nbar")]
+        [InlineData("foo; abc=bar")] // You can pass this to ctor, but not to property of already parsed disposition
+        [InlineData("foo\"bar")]
+        [InlineData("fň")]
+        public void ContentDisposition_Type_Validation_Invalid(string dispositionType)
+        {
+            var cd = new ContentDisposition();
+            Assert.Throws<ArgumentException>(() =>
+            {
+                cd.DispositionType = dispositionType;
+            });
+        }
     }
 }