[3/4] resolv: Support clearing option flags with a “-” prefix (bug 14799)

Message ID 53ec8f1ff51463adfee5b761f0c698b8064aa263.1718345824.git.fweimer@redhat.com
State Accepted
Headers
Series Some DNS stub resolver fixes/enhancements |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed

Commit Message

Florian Weimer June 14, 2024, 6:20 a.m. UTC
  I think using a “-” prefix is less confusing than introducing
double-negation construct (“no-no-tld-query”).
---
 NEWS                                  |  6 ++++++
 resolv/res_init.c                     | 28 ++++++++++++++-------------
 resolv/tst-resolv-res_init-skeleton.c | 10 ++++++++++
 3 files changed, 31 insertions(+), 13 deletions(-)
  

Comments

DJ Delorie June 20, 2024, 9:06 p.m. UTC | #1
Florian Weimer <fweimer@redhat.com> writes:
> I think using a “-” prefix is less confusing than introducing
> double-negation construct (“no-no-tld-query”).

> +* In /etc/resolv.conf and the RES_OPTIONS, option flags can now be
> +  prefixed with “-” to clear previously set flags.  For example, if
> +  /etc/resolv.conf contains “options no-aaaa”, a process running with
> +  the RES_OPTIONS=-no-aaaa environment variable performs AAAA DNS
> +  queries when the glibc DNS stub resolver is used.

Ok.

> diff --git a/resolv/res_init.c b/resolv/res_init.c
> -            uint8_t clear;

Ok, field removed.

>  #define STRnLEN(str) str, sizeof (str) - 1
> -            { STRnLEN ("rotate"), 0, RES_ROTATE },
> -            { STRnLEN ("edns0"), 0, RES_USE_EDNS0 },
> -            { STRnLEN ("single-request-reopen"), 0, RES_SNGLKUPREOP },
> -            { STRnLEN ("single-request"), 0, RES_SNGLKUP },
> -            { STRnLEN ("no_tld_query"), 0, RES_NOTLDQUERY },
> -            { STRnLEN ("no-tld-query"), 0, RES_NOTLDQUERY },
> -            { STRnLEN ("no-reload"), 0, RES_NORELOAD },
> -            { STRnLEN ("use-vc"), 0, RES_USEVC },
> -            { STRnLEN ("trust-ad"), 0, RES_TRUSTAD },
> -            { STRnLEN ("no-aaaa"), 0, RES_NOAAAA },
> +            { STRnLEN ("rotate"), RES_ROTATE },
> +            { STRnLEN ("edns0"),  RES_USE_EDNS0 },
> +            { STRnLEN ("single-request-reopen"), RES_SNGLKUPREOP },
> +            { STRnLEN ("single-request"), RES_SNGLKUP },
> +            { STRnLEN ("no_tld_query"), RES_NOTLDQUERY },
> +            { STRnLEN ("no-tld-query"), RES_NOTLDQUERY },
> +            { STRnLEN ("no-reload"), RES_NORELOAD },
> +            { STRnLEN ("use-vc"),  RES_USEVC },
> +            { STRnLEN ("trust-ad"), RES_TRUSTAD },
> +            { STRnLEN ("no-aaaa"), RES_NOAAAA },

Ok.  No change other than to remove the unneeded 0.

>  #define noptions (sizeof (options) / sizeof (options[0]))
> +          bool negate_option = *cp == '-';
> +          if (negate_option)
> +            ++cp;

Ok.

>            for (int i = 0; i < noptions; ++i)
>              if (strncmp (cp, options[i].str, options[i].len) == 0)
>                {
> -                if (options[i].clear)
> -                  parser->template.options &= options[i].flag;
> +                if (negate_option)
> +                  parser->template.options &= ~options[i].flag;
>                  else
>                    parser->template.options |= options[i].flag;

Ok.

> diff --git a/resolv/tst-resolv-res_init-skeleton.c b/resolv/tst-resolv-res_init-skeleton.c
> index 6bef62cde2..d3a19eb305 100644
> --- a/resolv/tst-resolv-res_init-skeleton.c
> +++ b/resolv/tst-resolv-res_init-skeleton.c
> @@ -679,6 +679,16 @@ struct test_case test_cases[] =
>       "; nameserver[0]: [192.0.2.1]:53\n",
>       .res_options = "attempts:5 ndots:3 edns0 ",
>      },
> +    {.name = "RES_OPTIONS can clear flags",
> +     .conf = "options ndots:2 use-vc no-aaaa edns0\n"
> +             "nameserver 192.0.2.1\n",
> +     .expected = "options ndots:3 use-vc\n"

> +                 "search example.com\n"
> +                 "; search[0]: example.com\n"

Where does this search come from?  Unrelated to what you're testing, but...

(ok as long as the test succeeds, I suppose :)

> +                 "nameserver 192.0.2.1\n"
> +                 "; nameserver[0]: [192.0.2.1]:53\n",
> +     .res_options = "ndots:3 -edns0 -no-aaaa",
> +    },

Otherwise OK.

LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>
  

Patch

diff --git a/NEWS b/NEWS
index 20e263f581..495bbd5cbc 100644
--- a/NEWS
+++ b/NEWS
@@ -36,6 +36,12 @@  Major new features:
 * On Linux, update epoll header to include epoll ioctl definitions and
   related structure added in Linux kernel 6.9.
 
+* In /etc/resolv.conf and the RES_OPTIONS, option flags can now be
+  prefixed with “-” to clear previously set flags.  For example, if
+  /etc/resolv.conf contains “options no-aaaa”, a process running with
+  the RES_OPTIONS=-no-aaaa environment variable performs AAAA DNS
+  queries when the glibc DNS stub resolver is used.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * Architectures which use a 32-bit seconds-since-epoch field in struct
diff --git a/resolv/res_init.c b/resolv/res_init.c
index 263263d474..243532b3ad 100644
--- a/resolv/res_init.c
+++ b/resolv/res_init.c
@@ -682,27 +682,29 @@  res_setoptions (struct resolv_conf_parser *parser, const char *options)
           {
             char str[22];
             uint8_t len;
-            uint8_t clear;
             unsigned long int flag;
           } options[] = {
 #define STRnLEN(str) str, sizeof (str) - 1
-            { STRnLEN ("rotate"), 0, RES_ROTATE },
-            { STRnLEN ("edns0"), 0, RES_USE_EDNS0 },
-            { STRnLEN ("single-request-reopen"), 0, RES_SNGLKUPREOP },
-            { STRnLEN ("single-request"), 0, RES_SNGLKUP },
-            { STRnLEN ("no_tld_query"), 0, RES_NOTLDQUERY },
-            { STRnLEN ("no-tld-query"), 0, RES_NOTLDQUERY },
-            { STRnLEN ("no-reload"), 0, RES_NORELOAD },
-            { STRnLEN ("use-vc"), 0, RES_USEVC },
-            { STRnLEN ("trust-ad"), 0, RES_TRUSTAD },
-            { STRnLEN ("no-aaaa"), 0, RES_NOAAAA },
+            { STRnLEN ("rotate"), RES_ROTATE },
+            { STRnLEN ("edns0"),  RES_USE_EDNS0 },
+            { STRnLEN ("single-request-reopen"), RES_SNGLKUPREOP },
+            { STRnLEN ("single-request"), RES_SNGLKUP },
+            { STRnLEN ("no_tld_query"), RES_NOTLDQUERY },
+            { STRnLEN ("no-tld-query"), RES_NOTLDQUERY },
+            { STRnLEN ("no-reload"), RES_NORELOAD },
+            { STRnLEN ("use-vc"),  RES_USEVC },
+            { STRnLEN ("trust-ad"), RES_TRUSTAD },
+            { STRnLEN ("no-aaaa"), RES_NOAAAA },
           };
 #define noptions (sizeof (options) / sizeof (options[0]))
+          bool negate_option = *cp == '-';
+          if (negate_option)
+            ++cp;
           for (int i = 0; i < noptions; ++i)
             if (strncmp (cp, options[i].str, options[i].len) == 0)
               {
-                if (options[i].clear)
-                  parser->template.options &= options[i].flag;
+                if (negate_option)
+                  parser->template.options &= ~options[i].flag;
                 else
                   parser->template.options |= options[i].flag;
                 break;
diff --git a/resolv/tst-resolv-res_init-skeleton.c b/resolv/tst-resolv-res_init-skeleton.c
index 6bef62cde2..d3a19eb305 100644
--- a/resolv/tst-resolv-res_init-skeleton.c
+++ b/resolv/tst-resolv-res_init-skeleton.c
@@ -679,6 +679,16 @@  struct test_case test_cases[] =
      "; nameserver[0]: [192.0.2.1]:53\n",
      .res_options = "attempts:5 ndots:3 edns0 ",
     },
+    {.name = "RES_OPTIONS can clear flags",
+     .conf = "options ndots:2 use-vc no-aaaa edns0\n"
+     "nameserver 192.0.2.1\n",
+     .expected = "options ndots:3 use-vc\n"
+     "search example.com\n"
+     "; search[0]: example.com\n"
+     "nameserver 192.0.2.1\n"
+     "; nameserver[0]: [192.0.2.1]:53\n",
+     .res_options = "ndots:3 -edns0 -no-aaaa",
+    },
     {.name = "many search list entries (bug 19569)",
      .conf = "nameserver 192.0.2.1\n"
      "search corp.example.com support.example.com"