Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wasm][net] System.Net.Mail should not throw PNSE for full assembly. #42974

Merged
merged 21 commits into from
Oct 9, 2020
Merged

[wasm][net] System.Net.Mail should not throw PNSE for full assembly. #42974

merged 21 commits into from
Oct 9, 2020

Conversation

kjpou1
Copy link
Contributor

@kjpou1 kjpou1 commented Oct 2, 2020

Add back certain functionality for System.Net.Mail.

Right now SmtpClient throws PNSE by providing a browser specific implementation.

Supported from the issue:

MailAddress passes:

            var emailAdddress = "[email protected]";
            try
            {
                new MailAddress(emailAdddress);
            }
            catch (Exception exc)
            {
                Console.WriteLine(exc);
            }

Invalid email address parse:

System.FormatException: The specified string is not in the form required for an e-mail address.
   at System.Net.Mail.MailAddressParser.TryReadCfwsAndThrowIfIncomplete(String data, Int32 index, Int32& outIndex, Boolean throwExceptionIfFail)
   at System.Net.Mail.MailAddressParser.TryParseDomain(String data, Int32& index, String& domain, Boolean throwExceptionIfFail)
   at System.Net.Mail.MailAddressParser.TryParseAddress(String data, Boolean expectMultipleAddresses, Int32& index, ParseAddressInfo& parseAddressInfo, Boolean throwExceptionIfFail)
   at System.Net.Mail.MailAddressParser.TryParseAddress(String data, ParseAddressInfo& parsedAddress, Boolean throwExceptionIfFail)
   at System.Net.Mail.MailAddress.TryParse(String address, String displayName, Encoding displayNameEncoding, ValueTuple`4& parsedData, Boolean throwExceptionIfFail)
   at System.Net.Mail.MailAddress..ctor(String address, String displayName, Encoding displayNameEncoding)
   at System.Net.Mail.MailAddress..ctor(String address)
   at Sample.Test.Main(String[] args)

MailMessage with ContentDisposition passes:

    string file = "data.xls";
    // Create a message and set up the recipients.
    MailMessage message = new MailMessage(
        "[email protected]",
        "[email protected]",
        "Quarterly data report.",
        "See the attached spreadsheet.");

    // Create  the file attachment for this email message.
    Attachment data = new Attachment(file, MediaTypeNames.Application.Octet);
    // Add time stamp information for the file.
    ContentDisposition disposition = data.ContentDisposition;
    disposition.CreationDate = System.IO.File.GetCreationTime(file);
    disposition.ModificationDate = System.IO.File.GetLastWriteTime(file);
    disposition.ReadDate = System.IO.File.GetLastAccessTime(file);
    // Add the file attachment to this email message.
    message.Attachments.Add(data);

ContentDisposition constructor passes:

ContentDisposition c1 = new ContentDisposition();
Console.WriteLine(c1.ToString());

ContentDisposition c2 = new ContentDisposition("attachment");
Console.WriteLine(c2.ToString());

SmtpClient throws PNSE:

            try
            {
                new SmtpClient();
            }
            catch (Exception exc)
            {
                Console.WriteLine(exc);
            }

error:

System.PlatformNotSupportedException: Operation is not supported on this platform.
   at System.Net.Mail.SmtpClient.Initialize()
   at System.Net.Mail.SmtpClient..ctor()
   at Sample.Test.Main(String[] args)

CLI Tests ARE reactivated

Issue: #42576

@ghost
Copy link

ghost commented Oct 2, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you using as the bar to pull things back in?

@kjpou1
Copy link
Contributor Author

kjpou1 commented Oct 5, 2020

Making sure the scenarios in the issue #42576 work and the tests pass.

@kjpou1 kjpou1 marked this pull request as ready for review October 5, 2020 05:37
@lewing lewing added the arch-wasm WebAssembly architecture label Oct 5, 2020
@kjpou1
Copy link
Contributor Author

kjpou1 commented Oct 7, 2020

Are we sure that the tests we have cover all the methods on all the classes that we are preserving?

The only thing we removed was the SmtpClient and left all the remaining Functional and Unit tests case coverage. That should catch any fallout.

@mdh1418
Copy link
Member

mdh1418 commented Oct 7, 2020

I think we need to modify src/libraries/System.Net.Mail/src/Resources/Strings.resx since it was added when adding PNSE for System.Net.Mail in 80fe5bb.

@kjpou1
Copy link
Contributor Author

kjpou1 commented Oct 8, 2020

I think we need to modify src/libraries/System.Net.Mail/src/Resources/Strings.resx since it was added when adding PNSE for System.Net.Mail in 80fe5bb.

Removed in recent round of commits

Copy link
Member

@mdh1418 mdh1418 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good other than the comment

Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@lewing lewing requested a review from safern October 9, 2020 04:14
@kjpou1 kjpou1 merged commit 59bb74c into dotnet:master Oct 9, 2020
@kjpou1
Copy link
Contributor Author

kjpou1 commented Oct 9, 2020

/backport to release/5.0

@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2020

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/297521067

@kjpou1 kjpou1 deleted the wasm-net-mail branch October 13, 2020 04:30
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
@karelz karelz added this to the 6.0.0 milestone Jan 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Net
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants