Sitelet https://github.com/iputils/iputils/pull/553
Skip to content

ping: Allow to disable with environment variable - #553

Merged
pevik merged 1 commit into
iputils:masterfrom
pevik:IPUTILS_PING_PTR_LOOKUP
Sep 2, 2024
Merged

ping: Allow to disable with environment variable#553
pevik merged 1 commit into
iputils:masterfrom
pevik:IPUTILS_PING_PTR_LOOKUP

Conversation

@pevik

@pevik pevik commented Aug 30, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Allow to disable reverse DNS resolution (PTR lookup) with IPUTILS_PING_PTR_LOOKUP environment variable set to 0:

$ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 google.com
PING google.com (142.251.37.110) 56(84) bytes of data.
64 bytes from 142.251.37.110: icmp_seq=1 ttl=116 time=11.1 ms

It's off by default:

$ ./builddir/ping/ping -c1 google.com
PING google.com (142.251.37.110) 56(84) bytes of data.
64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=18.6 ms

-H/-n override the variable:

$ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 -H google.com
PING google.com (142.251.37.110) 56(84) bytes of data.
64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=17.1 ms

$ IPUTILS_PING_PTR_LOOKUP= ./builddir/ping/ping -c1 -n google.com
PING google.com (142.251.37.110) 56(84) bytes of data.
64 bytes from 142.251.36.142: icmp_seq=1 ttl=116 time=15.8 ms

Update man page.

NOTE: variable needs to be parsed before getopts, therefore the optional warning is printed afterwards (only if the lookup disabled due environment variable and if not -q).

Implements: #531

@pevik pevik added this to the Next release summer 2024 milestone Aug 30, 2024
pevik added a commit to pevik/iputils that referenced this pull request Aug 30, 2024
@pevik
ping: Allow to disable with environment variable
7c18b97 
Allow to disable reverse DNS resolution (PTR lookup) with
IPUTILS_PING_PTR_LOOKUP environment variable set to 0:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.37.110: icmp_seq=1 ttl=116 time=11.1 ms

It's off by default:

    $ ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=18.6 ms

-H/-n override the variable:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 -H google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=17.1 ms

    $ IPUTILS_PING_PTR_LOOKUP= ./builddir/ping/ping -c1 -n google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.36.142: icmp_seq=1 ttl=116 time=15.8 ms

Update man page.

Implements: iputils#531
Closes: iputils#553
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@pevik
pevik force-pushed the IPUTILS_PING_PTR_LOOKUP branch from e77e040 to 7c18b97 Compare August 30, 2024 14:41
@pevik
pevik requested review from a team, kerolasa, nmeyerhans and okias August 30, 2024 14:41
pevik added a commit to pevik/iputils that referenced this pull request Aug 30, 2024
@pevik
ping: Allow to disable with environment variable
adc753a 
Allow to disable reverse DNS resolution (PTR lookup) with
IPUTILS_PING_PTR_LOOKUP environment variable set to 0:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.37.110: icmp_seq=1 ttl=116 time=11.1 ms

It's off by default (we are conservative, most of the users does not
have problem thus why to loose the functionality):

    $ ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=18.6 ms

-H/-n override the variable:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 -H google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=17.1 ms

    $ IPUTILS_PING_PTR_LOOKUP= ./builddir/ping/ping -c1 -n google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.36.142: icmp_seq=1 ttl=116 time=15.8 ms

This help users to easier disable reverse DNS resolution than alias ping='ping -H'
which would not work in scripts.

Update man page.

Implements: iputils#531
Closes: iputils#553
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@pevik
pevik force-pushed the IPUTILS_PING_PTR_LOOKUP branch from 7c18b97 to adc753a Compare August 30, 2024 14:45
pevik added a commit to pevik/iputils that referenced this pull request Aug 30, 2024
@pevik
ping: Allow to disable with environment variable
57acd64 
Allow to disable reverse DNS resolution (PTR lookup) with
IPUTILS_PING_PTR_LOOKUP environment variable set to 0:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.37.110: icmp_seq=1 ttl=116 time=11.1 ms

It's off by default (we are conservative, most of the users does not
have problem thus why to loose the functionality):

    $ ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=18.6 ms

-H/-n override the variable:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 -H google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=17.1 ms

    $ IPUTILS_PING_PTR_LOOKUP= ./builddir/ping/ping -c1 -n google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.36.142: icmp_seq=1 ttl=116 time=15.8 ms

This help users to easier disable reverse DNS resolution than alias ping='ping -H'
which would not work in scripts.

Update man page.

Implements: iputils#531
Closes: iputils#553
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@pevik
pevik force-pushed the IPUTILS_PING_PTR_LOOKUP branch from adc753a to 57acd64 Compare August 30, 2024 15:41
@pevik pevik mentioned this pull request Aug 30, 2024
Open
@okias

okias commented Aug 31, 2024

Copy link
Copy Markdown
Member

IPUTILS_PING_PTR_LOOKUP, I would pick shorter PING_PTR or PING_PTR_LOOKUP chance this will interfere with something else is 0.00000001 IMHO.

Thank you for this PR!

@pevik

pevik commented Aug 31, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for your review!

IPUTILS_PING_PTR_LOOKUP, I would pick shorter PING_PTR or PING_PTR_LOOKUP chance this will interfere with something else is 0.00000001 IMHO.

I wanted to be obvious that change is not for some other implementation (e.g. fping, busybox, inetutils). Most of the people will copy paste variable from man page anyway.

metan-ucw
metan-ucw reviewed Sep 2, 2024
View reviewed changes
Comment thread ping/ping.c Outdated
if (env && !strcmp(env, "0")) {
rts.opt_numeric = 1;
if (rts.opt_verbose)
error(0, 0, _("WARNING: reverse DNS resolution (PTR lookup) disabled, enforce with -H"));

@metan-ucw metan-ucw Sep 2, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we are checking the opt_verbose before the getopt() loop that sets it?

@pevik pevik Sep 2, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I'll move it below.

@pevik pevik Sep 2, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning moved below, don't print on -q (enabled by default, no need to put -v).

pevik added a commit to pevik/iputils that referenced this pull request Sep 2, 2024
@pevik
ping: Allow to disable with environment variable
e0b31a5 
Allow to disable reverse DNS resolution (PTR lookup) with
IPUTILS_PING_PTR_LOOKUP environment variable set to 0:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.37.110: icmp_seq=1 ttl=116 time=11.1 ms

It's off by default (we are conservative, most of the users does not
have problem thus why to loose the functionality):

    $ ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=18.6 ms

-H/-n override the variable:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 -H google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=17.1 ms

    $ IPUTILS_PING_PTR_LOOKUP= ./builddir/ping/ping -c1 -n google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.36.142: icmp_seq=1 ttl=116 time=15.8 ms

This help users to easier disable reverse DNS resolution than alias ping='ping -H'
which would not work in scripts.

Update man page.

Implements: iputils#531
Closes: iputils#553
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@pevik
pevik force-pushed the IPUTILS_PING_PTR_LOOKUP branch from 57acd64 to e0b31a5 Compare September 2, 2024 14:02
pevik added a commit to pevik/iputils that referenced this pull request Sep 2, 2024
@pevik
ping: Allow to disable with environment variable
5ffec05 
Allow to disable reverse DNS resolution (PTR lookup) with
IPUTILS_PING_PTR_LOOKUP environment variable set to 0:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.37.110: icmp_seq=1 ttl=116 time=11.1 ms

It's off by default (we are conservative, most of the users does not
have problem thus why to loose the functionality):

    $ ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=18.6 ms

-H/-n override the variable:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 -H google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=17.1 ms

    $ IPUTILS_PING_PTR_LOOKUP= ./builddir/ping/ping -c1 -n google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.36.142: icmp_seq=1 ttl=116 time=15.8 ms

This help users to easier disable reverse DNS resolution than alias ping='ping -H'
which would not work in scripts.

NOTE: variable needs to be parsed before getopts, therefore the
optional warning is printed afterwards (only if the lookup disabled due
environment variable and if not -q).

Update man page.

Implements: iputils#531
Closes: iputils#553
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@pevik
pevik force-pushed the IPUTILS_PING_PTR_LOOKUP branch from e0b31a5 to 5ffec05 Compare September 2, 2024 14:08
metan-ucw
metan-ucw approved these changes Sep 2, 2024
View reviewed changes

@metan-ucw metan-ucw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it looks good.

Reviewed-by: Cyril Hrubis chrubis@suse.cz

pevik added a commit to pevik/iputils that referenced this pull request Sep 2, 2024
@pevik
ping: Allow to disable with environment variable
ff14b5d 
Allow to disable reverse DNS resolution (PTR lookup) with
IPUTILS_PING_PTR_LOOKUP environment variable set to 0:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.37.110: icmp_seq=1 ttl=116 time=11.1 ms

It's off by default (we are conservative, most of the users does not
have problem thus why to loose the functionality):

    $ ./builddir/ping/ping -c1 google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=18.6 ms

-H/-n override the variable:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 -H google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from prg03s13-in-f14.1e100.net (142.251.37.110): icmp_seq=1 ttl=116 time=17.1 ms

    $ IPUTILS_PING_PTR_LOOKUP= ./builddir/ping/ping -c1 -n google.com
    PING google.com (142.251.37.110) 56(84) bytes of data.
    64 bytes from 142.251.36.142: icmp_seq=1 ttl=116 time=15.8 ms

This help users to easier disable reverse DNS resolution than alias ping='ping -H'
which would not work in scripts.

NOTE: variable needs to be parsed before getopts, therefore the
optional warning is printed afterwards (only if the lookup disabled due
environment variable and if not -q).

Update man page.

Implements: iputils#531
Closes: iputils#553
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@pevik
pevik force-pushed the IPUTILS_PING_PTR_LOOKUP branch from 5ffec05 to ff14b5d Compare September 2, 2024 16:10
@pevik
ping: Allow to disable with environment variable
6fc68b1 
Allow to disable reverse DNS resolution (PTR lookup) with
IPUTILS_PING_PTR_LOOKUP environment variable set to 0:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 google.com
    ./builddir/ping/ping: WARNING: reverse DNS resolution (PTR lookup) disabled, enforce with -H
    PING google.com (172.217.17.110) 56(84) bytes of data.
    64 bytes from 172.217.17.110: icmp_seq=1 ttl=113 time=45.5 ms

It's off by default (we are conservative, most of the users does not
have problem thus why to loose the functionality):

    $ ./builddir/ping/ping -c1 google.com
    PING google.com (172.217.17.110) 56(84) bytes of data.
    64 bytes from ams15s29-in-f110.1e100.net (172.217.17.110): icmp_seq=1 ttl=113 time=46.1 ms

-H/-n override the variable:

    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 -H google.com
    ./builddir/ping/ping: WARNING: reverse DNS resolution (PTR lookup) disabled, enforce with -H
    PING google.com (172.217.17.110) 56(84) bytes of data.
    64 bytes from ams15s29-in-f14.1e100.net (172.217.17.110): icmp_seq=1 ttl=113 time=46.3 ms

    $ IPUTILS_PING_PTR_LOOKUP= ./builddir/ping/ping -c1 -n google.com
    PING google.com (172.217.17.110) 56(84) bytes of data.
    64 bytes from 172.217.17.110: icmp_seq=1 ttl=113 time=47.1 ms

-q suppresses the warning:
    $ IPUTILS_PING_PTR_LOOKUP=0 ./builddir/ping/ping -c1 -q google.com
    PING google.com (172.217.17.110) 56(84) bytes of data.

This help users to easier disable reverse DNS resolution than alias ping='ping -H'
which would not work in scripts.

NOTE: variable needs to be parsed before getopts, therefore the
optional warning is printed afterwards (only if the lookup disabled due
environment variable and if not -q).

Update man page.

Implements: iputils#531
Closes: iputils#553
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@pevik
pevik force-pushed the IPUTILS_PING_PTR_LOOKUP branch from ff14b5d to 6fc68b1 Compare September 2, 2024 16:16
@pevik
pevik merged commit 6fc68b1 into iputils:master Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@metan-ucw metan-ucw metan-ucw approved these changes
@kerolasa kerolasa Awaiting requested review from kerolasa
@nmeyerhans nmeyerhans Awaiting requested review from nmeyerhans
@okias okias Awaiting requested review from okias

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Next release summer 2024

Development

Successfully merging this pull request may close these issues.

3 participants

@pevik @okias @metan-ucw