EmailService by sonalk-optimus · Pull Request #12 · sonalk-optimus/EmailAPI

sonalk-optimus · 2025-06-10T05:11:51Z

No description provided.

sonalk-optimus · 2025-06-10T05:12:00Z

-
                }
                catch (Exception ex)
                {


Catching the exception and immediately rethrowing with 'throw ex;' discards the stack trace. Use just 'throw;' to preserve the original stack trace for effective debugging.

sonalk-optimus · 2025-06-10T05:12:01Z

@@ -11,30 +11,39 @@ public void SendEmail(MailRequest mailRequest)
        {
            try
            {


Avoid hardcoding SMTP host and port values directly in code. Move these values to configuration (e.g., appsettings.json or environment variables) to improve flexibility and security.

sonalk-optimus · 2025-06-10T05:12:02Z

            {
+                // Mistake 1: Hardcoded SMTP server and port instead of using config
                SmtpClient smtpServer = new SmtpClient("in-v3.mailjet.com", 587)
                {


Do not hardcode credentials in source code. Retrieve sensitive values like SMTP username and password from configuration or a secure vault mechanism (e.g., Azure Key Vault, AWS Secrets Manager) instead.

sonalk-optimus · 2025-06-10T05:12:02Z

                    UseDefaultCredentials = false,
-                    Credentials = new NetworkCredential("745752d3e4dbaf360f9b83da4e7577ac", "8747b2b74209ab9144a7a4b9111cecb3"),
+                    // Mistake 2: Hardcoded credentials (should be from config or secure vault)
+                    Credentials = new NetworkCredential("745752d3e4dbaf360f9b83da", "8747b2b74209ab9144a7a4"),


Add null or format validation for 'mailRequest.RecipientEmailAddress' before using it to prevent runtime failures and potential security issues.

sonalk-optimus · 2025-06-10T05:12:03Z

                    Timeout = 60000,
-
                };



Input validation should be performed on email body ('mailRequest.Body') to guard against null values and potential email injection vulnerabilities.

sonalk-optimus · 2025-06-10T05:12:04Z


                MailMessage mailMessage = new MailMessage
                {
                    From = new MailAddress("sonal.khatri@optimusinfo.com", "MailDelivery")


Use the asynchronous version 'SendMailAsync' of SmtpClient to avoid blocking the calling thread, especially in web applications.

sonalk-optimus · 2025-06-10T05:12:04Z

                    From = new MailAddress("sonal.khatri@optimusinfo.com", "MailDelivery")
                };
+
+                // Mistake 3: No null or format validation on recipient email


Use just 'throw;' instead of 'throw ex;' in catch blocks to preserve the original exception stack trace.

sonalk-optimus · 2025-07-01T16:54:40Z

Throwing 'ex' here resets the original exception's stack trace, making debugging harder. Use 'throw;' to preserve stack trace.

sonalk-optimus · 2025-07-01T16:54:41Z

            try
            {
+                // Mistake 1: Hardcoded SMTP server and port instead of using config
                SmtpClient smtpServer = new SmtpClient("in-v3.mailjet.com", 587)


Avoid hardcoding SMTP server, port, and credentials. Fetch these values from a configuration provider or secured vault to support multiple environments and protect sensitive data.

sonalk-optimus · 2025-07-01T16:54:42Z

                {
                    EnableSsl = true,
                    UseDefaultCredentials = false,
-                    Credentials = new NetworkCredential("745752d3e4dbaf360f9b83da4e7577ac", "8747b2b74209ab9144a7a4b9111cecb3"),


Add null or format validation on recipient email address before adding to 'mailMessage.To'. Consider using 'MailAddress.TryCreate' to avoid runtime exceptions for malformed emails.

sonalk-optimus · 2025-07-01T16:54:42Z

+                    // Mistake 2: Hardcoded credentials (should be from config or secure vault)
+                    Credentials = new NetworkCredential("745752d3e4dbaf360f9b83da", "8747b2b74209ab9144a7a4"),
                    DeliveryMethod = SmtpDeliveryMethod.Network,
                    Timeout = 60000,


Validate 'mailRequest.Body' for null or unexpected values before assigning to 'mailMessage.Body' to prevent possible exceptions or injection issues.

sonalk-optimus · 2025-07-01T16:54:43Z

                    Timeout = 60000,
-
                };



Consider using 'SendMailAsync' for sending emails to avoid blocking the calling thread, especially if this service may be used in a web application context.

sonalk-optimus · 2025-07-01T16:54:44Z


                MailMessage mailMessage = new MailMessage
                {
                    From = new MailAddress("sonal.khatri@optimusinfo.com", "MailDelivery")


Use 'throw;' instead of 'throw ex;' to preserve the original stack trace. This greatly aids in troubleshooting the source of exceptions.

Copilot

Pull Request Overview

Adds email sending functionality and updates project asset metadata due to environment/path changes.

Implements MailService.SendEmail with SMTP client setup
Adjusts EmailController to call the mail service
Updates various autogenerated NuGet/MSBuild asset files (path casing, related PDB/XML entries, restore settings)

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
obj/project.nuget.cache	Environment‐specific metadata updated (path casing, hash)
obj/project.assets.json	Added `related` PDB/XML entries and new restore settings
obj/Email.Api.csproj.nuget.g.targets	Removed duplicate `<PropertyGroup>` blocks
obj/Email.Api.csproj.nuget.g.props	Bumped NuGetToolVersion, trimmed package folders
obj/Email.Api.csproj.nuget.dgspec.json	Updated project paths and added audit properties
Services/MailService.cs	Introduced hardcoded SMTP setup and missing validations
Controllers/EmailController.cs	Exception rethrow pattern

Copilot · 2025-07-04T09:36:59Z

+  "dgSpecHash": "cJpYjnsZ8vg=",
  "success": true,
-  "projectFilePath": "C:\\Users\\Sonal\\source\\repos\\EmailApi\\Email.Api\\Email.Api.csproj",
+  "projectFilePath": "C:\\Users\\Sonal\\Source\\Repos\\EmailAPI\\EmailApi\\Email.Api\\Email.Api.csproj",


The obj/ files are auto-generated and environment-specific, causing noisy diffs; consider adding obj/ to .gitignore to keep the repo clean.

Copilot · 2025-07-04T09:37:00Z

        "type": "package",
        "compile": {
-          "lib/netstandard2.0/Microsoft.OpenApi.dll": {}
+          "lib/netstandard2.0/Microsoft.OpenApi.dll": {


Auto-generated asset changes (related PDB/XML entries) should not be committed; consider excluding obj/ outputs from source control.

Copilot · 2025-07-04T09:37:00Z

            try
            {
+                // Mistake 1: Hardcoded SMTP server and port instead of using config
                SmtpClient smtpServer = new SmtpClient("in-v3.mailjet.com", 587)


Hardcoding the SMTP host and port makes this inflexible; inject these settings from configuration or environment variables.

Copilot · 2025-07-04T09:37:00Z

                    UseDefaultCredentials = false,
-                    Credentials = new NetworkCredential("745752d3e4dbaf360f9b83da4e7577ac", "8747b2b74209ab9144a7a4b9111cecb3"),
+                    // Mistake 2: Hardcoded credentials (should be from config or secure vault)
+                    Credentials = new NetworkCredential("745752d3e4dbaf360f9b83da", "8747b2b74209ab9144a7a4"),


Storing SMTP credentials in code is a security risk; use secure configuration (e.g., Azure Key Vault or user secrets).

Suggested change

Credentials = new NetworkCredential("745752d3e4dbaf360f9b83da", "8747b2b74209ab9144a7a4"),

Credentials = new NetworkCredential(

Environment.GetEnvironmentVariable("SMTP_USERNAME"),

Environment.GetEnvironmentVariable("SMTP_PASSWORD")

),

Copilot · 2025-07-04T09:37:00Z

                };
+
+                // Mistake 3: No null or format validation on recipient email
                mailMessage.To.Add(mailRequest.RecipientEmailAddress);


No validation on the recipient email address can lead to failures or injection; validate format or use built-in email address classes.

Suggested change

mailMessage.To.Add(mailRequest.RecipientEmailAddress);

try

{

var recipientAddress = new MailAddress(mailRequest.RecipientEmailAddress);

mailMessage.To.Add(recipientAddress);

}

catch (FormatException)

{

throw new ArgumentException("Invalid recipient email address format.", nameof(mailRequest.RecipientEmailAddress));

}

Copilot · 2025-07-04T09:37:01Z

                mailMessage.DeliveryNotificationOptions = DeliveryNotificationOptions.OnFailure;
+
+                // Mistake 4: No input validation on email body, vulnerable to null or injection
                mailMessage.Body = mailRequest.Body;


Assigning raw body content without sanitization could lead to injection or formatting issues; consider validating or encoding input.

Copilot · 2025-07-04T09:37:01Z

                smtpServer.Send(mailMessage);
            }
            catch (Exception ex)
            {
+                // Mistake 6: Re-throwing exception without preserving stack trace (just throw; is better)
                throw ex;


Synchronous Send can block threads under load; prefer the async API (SendMailAsync) for better scalability.

Suggested change

smtpServer.Send(mailMessage);

}

catch (Exception ex)

{

// Mistake 6: Re-throwing exception without preserving stack trace (just throw; is better)

throw ex;

await smtpServer.SendMailAsync(mailMessage);

}

catch (Exception ex)

{

// Mistake 6: Re-throwing exception without preserving stack trace (just throw; is better)

throw;

Copilot · 2025-07-04T09:37:01Z

            catch (Exception ex)
            {
+                // Mistake 6: Re-throwing exception without preserving stack trace (just throw; is better)
                throw ex;


Re-throwing with throw ex; resets the stack trace; use throw; to preserve the original exception context.

Suggested change

throw ex;

throw;

Copilot · 2025-07-04T09:37:01Z

                catch (Exception ex)
                {
-
                    throw ex;


Using throw ex; loses the original stack trace; switch to throw; to maintain diagnostic information.

Suggested change

throw ex;

throw;

EmailService

18bfb13

sonalk-optimus commented Jun 10, 2025

View reviewed changes

sonalk-optimus closed this Jul 1, 2025

sonalk-optimus reopened this Jul 1, 2025

sonalk-optimus commented Jul 1, 2025

View reviewed changes

sonalk-optimus requested a review from Copilot July 4, 2025 09:35

Copilot AI reviewed Jul 4, 2025

View reviewed changes

@@ @@ -11,30 +11,39 @@ public void SendEmail(MailRequest mailRequest) @@
                       {
                           try
                           {

-                mailMessage.To.Add(mailRequest.RecipientEmailAddress);
+                try
+                {
+                    var recipientAddress = new MailAddress(mailRequest.RecipientEmailAddress);
+                    mailMessage.To.Add(recipientAddress);
+                }
+                catch (FormatException)
+                {
+                    throw new ArgumentException("Invalid recipient email address format.", nameof(mailRequest.RecipientEmailAddress));
+                }

Conversation

sonalk-optimus commented Jun 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants