HttpPlatformHandler/Kestrel: HttpContext.Connection.RemoteIpAddress not working

[guardrex]

Following: #16

I'm using Google RECAPTCHA on this contact form, and I think I used to have ...

string gIp = Context.Connection.RemoteIpAddress.ToString();

... in the beta7 version of this app. For the beta8 upgrade, that Context went away, but it seemed cool with HttpContext ...

string gIp = HttpContext.Connection.RemoteIpAddress.ToString();

... but that's throwing a null reference exception. Is this a "Luke not handling the contexts correctly" problem or a "HttpPlatformHandler/Kestrel" problem?

[Tratcher]

app.UseIISPlatformHandler(); should set RemoteIpAddress.

[guardrex]

I have the app.UseIISPlatformHandler(); in the app with the package in place. Is there anything else I rundown on this for you?

[Tratcher]

You could show your startup class.

[guardrex]

namespace MyApp
{
    public class Startup
    {
        public IConfiguration Configuration { get; set; }

        public Startup(IHostingEnvironment env)
        {
            var configurationBuilder = new ConfigurationBuilder().AddEnvironmentVariables();
            Configuration = configurationBuilder.Build();
            string machineName = Environment.GetEnvironmentVariable("COMPUTERNAME");
            if(machineName.StartsWith("~~~~~~", StringComparison.OrdinalIgnoreCase))
            {
                Configuration["ASPNET_ENV"] = "Production";
                ... other config stuff here ...
            }
            else if (machineName.StartsWith("~~~~~~", StringComparison.OrdinalIgnoreCase))
            {
                Configuration["ASPNET_ENV"] = "Production";
                ... other config stuff here ...
            }
            else
            {
                Configuration["ASPNET_ENV"] = "Development";
            }
        }

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddAntiforgery();
            services.AddSingleton(_ => Configuration);
            services.AddMvc(options => 
            {
                string machineName = Environment.GetEnvironmentVariable("COMPUTERNAME");
                if (machineName.StartsWith("~~~~~~", StringComparison.OrdinalIgnoreCase))
                {
                    options.CacheProfiles.Add("PublicCache", new CacheProfile()
                    {
                        Duration = 2592000,
                        Location = ResponseCacheLocation.Any
                    });
                }
                else
                {
                    options.CacheProfiles.Add("PublicCache", new CacheProfile()
                    {
                        Duration = 0,
                        Location = ResponseCacheLocation.None
                    });
                }
            });
        }

        public void Configure(IApplicationBuilder app, IHostingEnvironment env)
        {
            app.UseIISPlatformHandler();
            if (env.IsDevelopment())
            {
                app.UseDeveloperExceptionPage();
                app.UseBrowserLink();
            }
            else
            {
                app.UseExceptionHandler("/error");
            }
            app.UseStaticFiles();
            app.Use(next => 
            {
                return ctx => 
                {
                    ctx.Response.Headers.Remove("Server");
                    return next(ctx);
                };
            });
            app.UseMvcWithDefaultRoute();
        }
    }
}

[Tratcher]

Looks fine. I recommend setting a break point here: ctx.Response.Headers.Remove("Server"); where it should be easy to verify the state of your request. E.g. dump the request headers and Connection properties to see what's available.

[guardrex]

Here's what I see stopping at that line and inspecting headers and connection:

Cache-Control:max-age=0
Connection:Keep-Alive Accept:text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
Accept-Encoding:gzip, deflate, sdch
Accept-Language:en-US,en;q=0.8
Cookie:_ga=GA1.2.162218639.1444775105; i4hHD23M7PY=CfDJ8OtjC ... 4TxFw
Host:www.XXXXXXX.com:8001
User-Agent:Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.71 Safari/537.36
Upgrade-Insecure-Requests:1
X-Forwarded-For:XXX.XXX.XXX.XXX:60982
X-Forwarded-Proto:http
X-Original-Proto:http

... but both ctx.Connection.LocalIpAddress and ctx.Connection.RemoteIpAddress were null.

On IIS Express, ctx.Connection.RemoteIpAddress correctly shows {::1}.

[Tratcher]

Ah, IPAddress.TryParse is probably choking on the port. https://github.com/aspnet/IISIntegration/blob/dev/src/Microsoft.AspNet.IISPlatformHandler/IISPlatformHandlerMiddleware.cs#L80-L96

It should probably split out the port and set it to Connection.RemotePort.

[guardrex]

Cool. I'll just drop the RECAPTCHA stuff for now and watch for an update here.

[gamoody]

Is the fix for this likely to make it into RC1? We have a site which we would like to take live when RC1 is released but it uses this for geo-location purposes.

[Tratcher]

No, this didn't make it into RC1. However it should be pretty easy to work around. Here's the existing code: https://github.com/aspnet/IISIntegration/blob/dev/src/Microsoft.AspNet.IISPlatformHandler/IISPlatformHandlerMiddleware.cs#L80-L96
The fix should only be a few lines. If you get to it before we do feel free to send a PR.

[guardrex]

@Tratcher Until the team gets to it (unless, as you say, someone jumps in with a PR), isn't the best practice when only having the headers to look for HTTP_X_FORWARDED_FOR. If that isn't there, then just take whatever REMOTE_ADDR has?

[EDIT] Nevermind ... I just looked at that code block, and I see what you mean about the port. No worries.

[guardrex]

@Tratcher btw - Maybe this changed when the move to Kestrel took place, but I'm not seeing ::1 running locally. Is that normal/expected?

[EDIT] ... and HttpContext.Connection.IsLocal is coming back false locally.