1
\$\begingroup\$

I wrote a small middleware for redirecting between WWW URLs.

The most important thing that I want to change is to make it super fast.

Here's what it is doing:

If the host is localhost or mywebsite.com without any subdomains and without www, it will redirect to www.localhost or www.mywebsite.com.

If the host starts with www. AND has subdomains, it will redirect to a url without www.

using System;
using System.Threading.Tasks;
using System.Text.RegularExpressions;
 
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Builder;
 
using Microsoft.Extensions.Logging;
 
 
namespace Website.Middlewares
{
 
    public class WWWRedirectionMiddleware
    {
        private readonly RequestDelegate _next;
 
        private string _domain;
        private int _https_port;
 
        public WWWRedirectionMiddleware(RequestDelegate next)
        {
            _next = next;
        }
 
        public async Task InvokeAsync(HttpContext context)
        {
 
            var request = context.Request;
            var response = context.Response;
 
            var host = request.Host;
 
            if (Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") == "Development")
            {
                _domain = "localhost";
                _https_port = 5001;
            } 
            else
            {
                _domain = "seaoftools.com";
                _https_port = 80;
            }
 
            // Redirect to www domain if url is home page
            if (host.Host.Equals(_domain, StringComparison.OrdinalIgnoreCase))
            {
                host = new HostString("www." + host.Host, _https_port);
 
                var redirectUrl = UriHelper.BuildAbsolute(
                    "https", 
                    host,
                    request.PathBase,
                    request.Path,
                    request.QueryString);
 
                response.Redirect(redirectUrl, true);
                return;
            } // Redirect to non-www domain if url is with a subdomain 
            else if (host.Value.StartsWith("www.", StringComparison.OrdinalIgnoreCase)
                     && Regex.Replace(host.Host, @"www.|" + _domain, String.Empty) != String.Empty)
            { 
                host = new HostString(host.Host.Replace("www.", String.Empty), _https_port);
 
                var redirectUrl = UriHelper.BuildAbsolute(
                    "https", 
                    host,
                    request.PathBase,
                    request.Path,
                    request.QueryString);
 
                response.Redirect(redirectUrl, true);
                return;
            }
 
            await _next(context);
        }
    }
 
}

What should I change to make the middleware run faster?

\$\endgroup\$
5
  • \$\begingroup\$ Welcome to the Code Review Community. Reviews generally require more code than debugging, please include the entire class in the question. \$\endgroup\$
    – pacmaninbw
    Dec 24 '20 at 20:52
  • \$\begingroup\$ @pacmaninbw stack exchange shows me an error when I add the full code so I posted only the main part of the code and the full code I posted in a link to paste bin \$\endgroup\$ Dec 25 '20 at 8:01
  • \$\begingroup\$ stack exchange shows me an error when I add the full code” - what do you mean by “stack exchange”? is that on code review or another site like stack overflow?? what is the error? \$\endgroup\$ Dec 25 '20 at 10:43
  • \$\begingroup\$ the error was “Your post appears to contain code that is not properly formatted” but now it's fixed I added the whole code @SᴀᴍOnᴇᴌᴀ \$\endgroup\$ Dec 25 '20 at 10:58
  • \$\begingroup\$ To anyone in the close vote queue, @SᴀᴍOnᴇᴌᴀ fixed the problem. \$\endgroup\$
    – pacmaninbw
    Dec 25 '20 at 12:11
2
\$\begingroup\$

Quick remarks:

  • _https_port: do not use underscores in variable names etc., except at the start.

  • _https_port = 80; The HTTPS port is most definitely not port 80.

  • The logic to fill redirectUrl and then redirect is duplicated. It should be a method that gets called. I'd move the "compile new host value in these specific cases" logic to a method and return the resulting HostString, then compare that HostString to the existing host and execute the redirect if they're different.

  • The string "www." is repeatedly used. It would be better to store it in a const string. Same for "https".

  • In one if you say host.Host, yet in the other you say host.Value.

  • Microsoft has guidelines against using more than three capital letters in a row. So "WWW" should become "Www" when it is part of a class name or method name etc.

\$\endgroup\$
1
3
\$\begingroup\$

If you add IWebHostEnvironment as a dependency to the middleware class, you can simplify the environment check by this:

if (webHostEnvironment.IsDevelopment())
{
        
}
\$\endgroup\$
0
\$\begingroup\$

My final code after all of the optimization:

using System;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Hosting;
using Microsoft.Extensions.Hosting;

namespace Website.Middlewares
{
    public class WwwRedirectionMiddleware
    {
        private readonly RequestDelegate _next;

        private string _domain;
        private int _httpsPort;

        public WwwRedirectionMiddleware(RequestDelegate next, IWebHostEnvironment webHostEnvironment)
        {
            _next = next;

            _domain = webHostEnvironment.IsDevelopment() ? "localhost" : "seaoftools.com";
            _httpsPort = webHostEnvironment.IsDevelopment() ? 5001 : 443;
        }

        public async Task InvokeAsync(HttpContext context)
        {
            bool IsRedirectNeeded = false;

            var request = context.Request;
            var response = context.Response;

            var host = request.Host;

            string WwwString = "www.";

            string HostWithoutWww = host.Host.Replace(WwwString, String.Empty);

            string HostWithWww = WwwString + host.Host;

            // Redirect to www domain if url is home page
            if (host.Host.Equals(_domain, StringComparison.OrdinalIgnoreCase))
            {
                host = new HostString(HostWithWww, _httpsPort);

                IsRedirectNeeded = true;
            } // Redirect to non-www domain if url is with a subdomain
            else if (host.Host.StartsWith(WwwString, StringComparison.OrdinalIgnoreCase) && host.Host.Split(".").Length > 2)
            {
                host = new HostString(HostWithoutWww, _httpsPort);

                IsRedirectNeeded = true;
            }

            if (IsRedirectNeeded)
            {
                var redirectUrl =
                    UriHelper
                        .BuildAbsolute("https",
                        host,
                        request.PathBase,
                        request.Path,
                        request.QueryString);

                response.Redirect(redirectUrl, true);
                return;
            }

            await _next(context);
        }
    }
}
```
\$\endgroup\$
1
  • \$\begingroup\$ If this is for general purpose use, consider that hosts can contain "www." in the name itself, for example: hostwww.com is a valid host. Will your middleware do the right thing in that case? \$\endgroup\$ Dec 27 '20 at 14:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.