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

Added null check for remoteIpAdress in HttpContextClientInfoProvider.GetComputerName #6469

Merged
merged 1 commit into from Jun 10, 2022

Conversation

Urtgard
Copy link
Contributor

@Urtgard Urtgard commented Jun 9, 2022

After an upgrade to 7.2 my app was really slow. The reason is the implementation in #6400.
remoteIpAdress can be null, but null isn't a valid input for Dns.GetHostEntry.
My log was full with this error message:

INFO  2022-06-09 15:46:20,519 [63   ] osoft.EntityFrameworkCore.Infrastructure - Entity Framework Core 6.0.5 initialized 'DbContext' using provider 'Microsoft.EntityFrameworkCore.SqlServer:6.0.5' with options: None
WARN  2022-06-09 15:46:20,524 [63   ] c.Auditing.HttpContextClientInfoProvider - System.ArgumentNullException: Value cannot be null. (Parameter 'address')
   at System.Net.Dns.GetHostEntry(IPAddress address)
   at Abp.AspNetCore.Mvc.Auditing.HttpContextClientInfoProvider.GetComputerName()

@ismcagdas ismcagdas self-requested a review June 10, 2022 05:28
@ismcagdas ismcagdas added this to the v7.3 milestone Jun 10, 2022
@ismcagdas
Copy link
Member

thanks a lot @Urtgard 👍

@ismcagdas ismcagdas merged commit 098e71c into aspnetboilerplate:dev Jun 10, 2022
@hzicong
Copy link

hzicong commented Jun 16, 2022

I use docker in the virtual machine,
remoteIpAddress is ::ffff:192.168.0.3
Dns.GetHostEntry(remoteIpAddress).HostName was wrong:

System.Net.Internals.SocketExceptionFactory+ExtendedSocketException (00000005, 0xFFFDFFFF): Name or service not known
at System.Net.Dns.GetHostEntryOrAddressesCore(IPAddress address, Boolean justAddresses, AddressFamily addressFamily, ValueStopwatch stopwatch)
at System.Net.Dns.GetHostEntry(IPAddress address)

@Urtgard
Copy link
Contributor Author

Urtgard commented Jun 16, 2022

Do you have performance problems as well? With errors average request time is about 60 seconds.

In one environment without a dns reverse zone I got the same error. And the same performance hit.

Maybe this shouldn't be enabled by default?

@hzicong
You could fix the dns issue. In my case a reverse zone was missing.
Or you could replace the service with your own implementation. https://aspnetboilerplate.com/Pages/Documents/Startup-Configuration#replacing-built-in-services

@hzicong
Copy link

hzicong commented Jun 17, 2022

Yes ,After upgrading to 7.2, my app takes an average of 5 seconds

@ismcagdas
Copy link
Member

We will release the 7.3 version sooner (probably today) so you can get the fix.

@Urtgard
Copy link
Contributor Author

Urtgard commented Jun 17, 2022

@ismcagdas The update doesn't help.

I fixed just the case when remoteIpAdress is null.

In his case remoteIpAdress is not null.
But GetHostEntry cant resolve the adress and throws a SocketException.

@ismcagdas
Copy link
Member

Maybe we should convert it back to its original verison. I will think about this, thanks.

@hzicong
Copy link

hzicong commented Jun 23, 2022

@Urtgard thanks,I replaced the service and fixed the bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants