Skip to content

fix(http): do reverse dns resolution only for IP addresses#10234

Merged
bryancall merged 1 commit intoapache:masterfrom
fatih-acar:fix_reverse_dns_origin
Oct 9, 2023
Merged

fix(http): do reverse dns resolution only for IP addresses#10234
bryancall merged 1 commit intoapache:masterfrom
fatih-acar:fix_reverse_dns_origin

Conversation

@fatih-acar
Copy link
Copy Markdown
Contributor

@fatih-acar fatih-acar commented Aug 17, 2023

We might use an origin fqdn starting with a number. Thus a reverse dns resolution is requested for a FQDN, which is failing and adding latency penalty in our test bed (around 20 ms per request). Refactor some IP checks and use the swoc::IPAddr to determine if the origin is an IP address.

@bneradt
Copy link
Copy Markdown
Contributor

bneradt commented Aug 20, 2023

[approve ci autest]

@bneradt bneradt added this to the 10.0.0 milestone Aug 20, 2023
@SolidWallOfCode
Copy link
Copy Markdown
Member

All of my comments are really "don't use ink_net if you can avoid it". The libswoc IP library should be used instead.

Comment thread proxy/http/HttpTransact.cc Outdated
Comment thread proxy/http/HttpTransact.h Outdated
Comment thread proxy/http/HttpTransact.cc Outdated
Comment thread proxy/http/HttpTransact.cc Outdated
Comment thread proxy/http/HttpTransact.cc Outdated
We might use an origin fqdn starting with a number.
Thus a reverse dns resolution is requested for a FQDN, which is failing
and adding latency penalty in our test bed (around 20 ms per request).
Refactor some IP checks and use the swoc::IPAddr to determine if the
origin is an IP address.

Signed-off-by: Fatih Acar <facar@scaleway.com>
@fatih-acar fatih-acar force-pushed the fix_reverse_dns_origin branch from 5126f61 to 49e3ea2 Compare August 22, 2023 07:02
@ezelkow1
Copy link
Copy Markdown
Member

[approve ci]

@SolidWallOfCode
Copy link
Copy Markdown
Member

Trying to catch up - approved. Thank you for all of the changes.

@bryancall bryancall merged commit 9e4f441 into apache:master Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants