NEW FEATURE: Add RFC4183 support to REV() (#2879)

Co-authored-by: Thomas Misilo <tmisilo@ksu.edu>
Co-authored-by: Jeffrey Cafferata <jeffrey@jcid.nl>

.goreleaser.yml

@@ -159,8 +159,9 @@ release:
     ## Deprecation warnings
 
     > [!WARNING]
+    > - **REV() will switch from RFC2317 to RFC4183 in v5.0.**  This is a breaking change. Warnings are output if your configuration is affected. No date has been announced for v5.0. See https://docs.dnscontrol.org/language-reference/top-level-functions/revcompat
     > - **MSDNS maintainer needed!** Without a new volunteer, this DNS provider will lose support after April 2025. See https://github.com/StackExchange/dnscontrol/issues/2878
-    > - **Call for new volunteer maintainers for NAMEDOTCOM and SOFTLAYER.** These providers have no maintainer. Maintainers respond to PRs and fix bugs in a timely manner, and try to stay on top of protocol changes.
+    > - **NAMEDOTCOM and SOFTLAYER need maintainers!** These providers have no maintainer. Maintainers respond to PRs and fix bugs in a timely manner, and try to stay on top of protocol changes.
     > - **get-certs/ACME support is frozen and will be removed without notice between now and July 2025.** It has been unsupported since December 2022.  If you don't use this feature, do not start. If you do use this feature, migrate ASAP.  See discussion in [issues/1400](https://github.com/StackExchange/dnscontrol/issues/1400)
 
     ## Install

README.md

@@ -160,8 +160,9 @@ See [dnscontrol-action](https://github.com/koenrh/dnscontrol-action) or [gacts/i
 
 ## Deprecation warnings (updated 2024-03-25)
 
+- **REV() will switch from RFC2317 to RFC4183 in v5.0.**  This is a breaking change. Warnings are output if your configuration is affected. No date has been announced for v5.0. See https://docs.dnscontrol.org/language-reference/top-level-functions/revcompat
 - **MSDNS maintainer needed!** Without a new volunteer, this DNS provider will lose support after April 2025. See https://github.com/StackExchange/dnscontrol/issues/2878
-- **Call for new volunteer maintainers for NAMEDOTCOM and SOFTLAYER.** These providers have no maintainer. Maintainers respond to PRs and fix bugs in a timely manner, and try to stay on top of protocol changes.
+- **NAMEDOTCOM and SOFTLAYER need maintainers!** These providers have no maintainer. Maintainers respond to PRs and fix bugs in a timely manner, and try to stay on top of protocol changes.
 - **get-certs/ACME support is frozen and will be removed without notice between now and July 2025.** It has been unsupported since December 2022.  If you don't use this feature, do not start. If you do use this feature, migrate ASAP.  See discussion in [issues/1400](https://github.com/StackExchange/dnscontrol/issues/1400)
 
 ## More info at our website

commands/ppreviewPush.go

@@ -15,6 +15,7 @@ import (
 	"github.com/StackExchange/dnscontrol/v4/pkg/normalize"
 	"github.com/StackExchange/dnscontrol/v4/pkg/notifications"
 	"github.com/StackExchange/dnscontrol/v4/pkg/printer"
+	"github.com/StackExchange/dnscontrol/v4/pkg/rfc4183"
 	"github.com/StackExchange/dnscontrol/v4/pkg/zonerecs"
 	"github.com/StackExchange/dnscontrol/v4/providers"
 	"github.com/urfave/cli/v2"
@@ -257,6 +258,7 @@ func prun(args PPreviewArgs, push bool, interactive bool, out printer.CLI, repor
 	if os.Getenv("TEAMCITY_VERSION") != "" {
 		fmt.Fprintf(os.Stderr, "##teamcity[buildStatus status='SUCCESS' text='%d corrections']", totalCorrections)
 	}
+	rfc4183.PrintWarning()
 	notifier.Done()
 	out.Printf("Done. %d corrections.\n", totalCorrections)
 	err = writeReport(report, reportItems)

commands/previewPush.go

@@ -16,6 +16,7 @@ import (
 	"github.com/StackExchange/dnscontrol/v4/pkg/normalize"
 	"github.com/StackExchange/dnscontrol/v4/pkg/notifications"
 	"github.com/StackExchange/dnscontrol/v4/pkg/printer"
+	"github.com/StackExchange/dnscontrol/v4/pkg/rfc4183"
 	"github.com/StackExchange/dnscontrol/v4/pkg/zonerecs"
 	"github.com/StackExchange/dnscontrol/v4/providers"
 	"github.com/urfave/cli/v2"
@@ -293,6 +294,7 @@ func run(args PreviewArgs, push bool, interactive bool, out printer.CLI, report
 	if os.Getenv("TEAMCITY_VERSION") != "" {
 		fmt.Fprintf(os.Stderr, "##teamcity[buildStatus status='SUCCESS' text='%d corrections']", totalCorrections)
 	}
+	rfc4183.PrintWarning()
 	notifier.Done()
 	out.Printf("Done. %d corrections.\n", totalCorrections)
 	if anyErrors {

commands/printIR.go

@@ -10,6 +10,7 @@ import (
 	"github.com/StackExchange/dnscontrol/v4/models"
 	"github.com/StackExchange/dnscontrol/v4/pkg/js"
 	"github.com/StackExchange/dnscontrol/v4/pkg/normalize"
+	"github.com/StackExchange/dnscontrol/v4/pkg/rfc4183"
 	"github.com/urfave/cli/v2"
 )
 
@@ -58,6 +59,7 @@ var _ = cmd(catDebug, func() *cli.Command {
 			log.SetOutput(os.Stdout)
 
 			err := exit(PrintIR(pargs))
+			rfc4183.PrintWarning()
 			if err == nil {
 				fmt.Fprintf(os.Stdout, "No errors.\n")
 			}

commands/types/dnscontrol.d.ts

@@ -2214,7 +2214,7 @@ declare function PANIC(message: string): never;
  *
  * Target should be a string representing the FQDN of a host.  Like all FQDNs in DNSControl, it must end with a `.`.
  *
- * **Magic Mode:**
+ * # Magic Mode
  *
  * PTR records are complex and typos are common. Therefore DNSControl
  * enables features to save labor and
@@ -2282,9 +2282,33 @@ declare function PANIC(message: string): never;
  * );
  * ```
  *
- * In the future we plan on adding a flag to [`A()`](A.md) which will insert
+ * # Automatic forward and reverse lookups
- * the correct PTR() record if the appropriate `.arpa` domain has been
+ *
- * defined.
+ * DNSControl does not automatically generate forward and reverse lookups. However
+ * it is possible to write a macro that does this by using the
+ * [`D_EXTEND()`](../global/D_EXTEND.md)
+ * function to insert `A` and `PTR` records into previously-defined domains.
+ *
+ * ```javascript
+ * function FORWARD_AND_REVERSE(ipaddr, fqdn) {
+ *     D_EXTEND(dom,
+ *         A(fqdn, ipaddr)
+ *     );
+ *     D_EXTEND(REV(ipaddr),
+ *         PTR(ipaddr, fqdn)
+ *     );
+ * }
+ *
+ * D("example.com", REGISTRAR, DnsProvider(DSP_NONE),
+ *     ...,
+ *     END);
+ * D(REV("10.20.30.0/24"), REGISTRAR, DnsProvider(DSP_NONE),
+ *     ...,
+ *     END);
+ *
+ * FORWARD_AND_REVERSE("10.20.30.77", "foo.example.com.");
+ * FORWARD_AND_REVERSE("10.20.30.99", "bar.example.com.");
+ * ```
  *
  * @see https://docs.dnscontrol.org/language-reference/domain-modifiers/ptr
  */
@@ -2395,53 +2419,115 @@ declare function R53_ZONE(zone_id: string): DomainModifier & RecordModifier;
  * `REV` returns the reverse lookup domain for an IP network. For
  * example `REV("1.2.3.0/24")` returns `3.2.1.in-addr.arpa.` and
  * `REV("2001:db8:302::/48")` returns `2.0.3.0.8.b.d.0.1.0.0.2.ip6.arpa.`.
- * This is used in [`D()`](D.md) functions to create reverse DNS lookup zones.
  *
- * This is a convenience function. You could specify `D("3.2.1.in-addr.arpa",
+ * `REV()` is commonly used with the [`D()`](D.md) functions to create reverse DNS lookup zones.
- * ...` if you like to do things manually but why would you risk making
- * typos?
  *
- * `REV` complies with RFC2317, "Classless in-addr.arpa delegation"
+ * These two are equivalent:
- * for netmasks of size /25 through /31.
- * While the RFC permits any format, we abide by the recommended format:
- * `FIRST/MASK.C.B.A.in-addr.arpa` where `FIRST` is the first IP address
- * of the zone, `MASK` is the netmask of the zone (25-31 inclusive),
- * and A, B, C are the first 3 octets of the IP address. For example
- * `172.20.18.130/27` is located in a zone named
- * `128/27.18.20.172.in-addr.arpa`
  *
- * If the address does not include a "/" then `REV` assumes /32 for IPv4 addresses
+ * ```javascript
+ * D("3.2.1.in-addr.arpa", ...
+ * ```
+ *
+ * ```javascript
+ * D(REV("1.2.3.0/24", ...
+ * ```
+ *
+ * The latter is easier to type and less error-prone.
+ *
+ * If the address does not include a "/" then `REV()` assumes /32 for IPv4 addresses
  * and /128 for IPv6 addresses.
  *
- * Note that the lower bits (the ones outside the netmask) must be zeros. They are not
+ * # RFC compliance
- * zeroed out automatically. Thus, `REV("1.2.3.4/24")` is an error.  This is done
+ *
- * to catch typos.
+ * `REV()` implements both RFC 2317 and the newer RFC 4183. The `REVCOMPAT()`
+ * function selects which mode is used. If `REVCOMPAT()` is not called, a default
+ * is selected for you.  The default will change to RFC 4183 in DNSControl v5.0.
+ *
+ * See [`REVCOMPAT()`](REVCOMPAT.md) for details.
+ *
+ * # Host bits
+ *
+ * v4.x:
+ * The host bits (the ones outside the netmask) must be zeros. They are not zeroed
+ * out automatically. Thus, `REV("1.2.3.4/24")` is an error.
+ *
+ * v5.0 and later:
+ * The host bits (the ones outside the netmask) are ignored.  Thus
+ * `REV("1.2.3.4/24")` and `REV("1.2.3.0/24")` are equivalent.
+ *
+ * # Examples
+ *
+ * Here's an example reverse lookup domain:
  *
  * ```javascript
  * D(REV("1.2.3.0/24"), REGISTRAR, DnsProvider(BIND),
  *   PTR("1", "foo.example.com."),
  *   PTR("2", "bar.example.com."),
  *   PTR("3", "baz.example.com."),
- *   // These take advantage of DNSControl's ability to generate the right name:
+ *   // If the first parameter is an IP address, DNSControl automatically calls REV() for you.
  *   PTR("1.2.3.10", "ten.example.com."),
  * );
  *
  * D(REV("2001:db8:302::/48"), REGISTRAR, DnsProvider(BIND),
  *   PTR("1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0", "foo.example.com."),  // 2001:db8:302::1
- *   // These take advantage of DNSControl's ability to generate the right name:
+ *   // If the first parameter is an IP address, DNSControl automatically calls REV() for you.
  *   PTR("2001:db8:302::2", "two.example.com."),                          // 2.0.0...
  *   PTR("2001:db8:302::3", "three.example.com."),                        // 3.0.0...
  * );
  * ```
  *
- * In the future we plan on adding a flag to [`A()`](../domain/A.md)which will insert
+ * # Automatic forward and reverse record generation
- * the correct PTR() record in the appropriate `D(REV())` domain (i.e. `.arpa` domain) has been
+ *
- * defined.
+ * DNSControl does not automatically generate forward and reverse lookups. However
+ * it is possible to write a macro that does this.  See
+ * [`PTR()`](../domain/PTR.md)   for an example.
  *
  * @see https://docs.dnscontrol.org/language-reference/top-level-functions/rev
  */
 declare function REV(address: string): string;
 
+/**
+ * `REVCOMPAT()` controls which RFC the [`REV()`](REV.md) function adheres to.
+ *
+ * Include one of these two commands near the top `dnsconfig.js` (at the global level):
+ *
+ * ```javascript
+ * REVCOMPAT("rfc2317");  // RFC 2117: Compatible with old files.
+ * REVCOMPAT("rfc4183");  // RFC 4183: Adopt the newer standard.
+ * ```
+ *
+ * `REVCOMPAT()` is global for all of `dnsconfig.js`. It must appear before any
+ * use of `REV()`; If not, behavior is undefined.
+ *
+ * # RFC 4183 vs RFC 2317
+ *
+ * RFC 2317 and RFC 4183 are two different ways to implement reverse lookups for
+ * CIDR blocks that are not on 8-bit boundaries (/24, /16, /8).
+ *
+ * Originally DNSControl implemented the older standard, which only specifies what
+ * to do for /8, /16, /24 - /32.  Using `REV()` for /9-17 and /17-23 CIDRs was an
+ * error.
+ *
+ * v4 defaults to RFC 2317.  In v5.0 the default will change to RFC 4183.
+ * `REVCOMPAT()` is provided for those that wish to retain the old behavior.
+ *
+ * For more information, see [Opinion #9](../../opinions.md#opinion-9-rfc-4183-is-better-than-rfc-2317).
+ *
+ * # Transition plan
+ *
+ * What's the default behavior if `REVCOMPAT()` is not used?
+ *
+ * | Version | /9 to /15 and /17 to /23 | /25 to 32 | Warnings                   |
+ * |---------|--------------------------|-----------|----------------------------|
+ * | v4      | RFC 4183                 | RFC 2317  | Only if /25 - /32 are used |
+ * | v5      | RFC 4183                 | RFC 4183  | none                       |
+ *
+ * No warnings are generated if the `REVCOMPAT()` function is used.
+ *
+ * @see https://docs.dnscontrol.org/language-reference/top-level-functions/revcompat
+ */
+declare function REVCOMPAT(rfc: string): string;
+
 /**
  * `SOA` adds an `SOA` record to a domain. The name should be `@`.  ns and mbox are strings. The other fields are unsigned 32-bit ints.
  *

documentation/SUMMARY.md

@@ -24,6 +24,7 @@
   * [NewRegistrar](functions/global/NewRegistrar.md)
   * [PANIC](functions/global/PANIC.md)
   * [REV](functions/global/REV.md)
+  * [REVCOMPAT](functions/global/REVCOMPAT.md)
   * [getConfiguredDomains](functions/global/getConfiguredDomains.md)
   * [require](functions/global/require.md)
   * [require_glob](functions/global/require_glob.md)

documentation/functions/domain/PTR.md

@@ -17,7 +17,7 @@ saving the user from having to reverse the IP address manually.
 
 Target should be a string representing the FQDN of a host.  Like all FQDNs in DNSControl, it must end with a `.`.
 
-**Magic Mode:**
+# Magic Mode
 
 PTR records are complex and typos are common. Therefore DNSControl
 enables features to save labor and
@@ -91,6 +91,32 @@ D(REV("2001:db8:302::/48"), REGISTRAR, DnsProvider(BIND),
 ```
 {% endcode %}
 
-In the future we plan on adding a flag to [`A()`](A.md) which will insert
+# Automatic forward and reverse lookups
-the correct PTR() record if the appropriate `.arpa` domain has been
+
-defined.
+DNSControl does not automatically generate forward and reverse lookups. However
+it is possible to write a macro that does this by using the 
+[`D_EXTEND()`](../global/D_EXTEND.md)
+function to insert `A` and `PTR` records into previously-defined domains.
+
+{% code title="dnsconfig.js" %}
+```javascript
+function FORWARD_AND_REVERSE(ipaddr, fqdn) {
+    D_EXTEND(dom,
+        A(fqdn, ipaddr)
+    );
+    D_EXTEND(REV(ipaddr),
+        PTR(ipaddr, fqdn)
+    );
+}
+
+D("example.com", REGISTRAR, DnsProvider(DSP_NONE),
+    ...,
+    END);
+D(REV("10.20.30.0/24"), REGISTRAR, DnsProvider(DSP_NONE),
+    ...,
+    END);
+
+FORWARD_AND_REVERSE("10.20.30.77", "foo.example.com.");
+FORWARD_AND_REVERSE("10.20.30.99", "bar.example.com.");
+```
+{% endcode %}

documentation/functions/global/REV.md

@@ -10,27 +10,50 @@ ts_return: string
 `REV` returns the reverse lookup domain for an IP network. For
 example `REV("1.2.3.0/24")` returns `3.2.1.in-addr.arpa.` and
 `REV("2001:db8:302::/48")` returns `2.0.3.0.8.b.d.0.1.0.0.2.ip6.arpa.`.
-This is used in [`D()`](D.md) functions to create reverse DNS lookup zones.
 
-This is a convenience function. You could specify `D("3.2.1.in-addr.arpa",
+`REV()` is commonly used with the [`D()`](D.md) functions to create reverse DNS lookup zones.
-...` if you like to do things manually but why would you risk making
-typos?
 
-`REV` complies with RFC2317, "Classless in-addr.arpa delegation"
+These two are equivalent:
-for netmasks of size /25 through /31.
-While the RFC permits any format, we abide by the recommended format:
-`FIRST/MASK.C.B.A.in-addr.arpa` where `FIRST` is the first IP address
-of the zone, `MASK` is the netmask of the zone (25-31 inclusive),
-and A, B, C are the first 3 octets of the IP address. For example
-`172.20.18.130/27` is located in a zone named
-`128/27.18.20.172.in-addr.arpa`
 
-If the address does not include a "/" then `REV` assumes /32 for IPv4 addresses
+{% code title="dnsconfig.js" %}
+```javascript
+D("3.2.1.in-addr.arpa", ...
+```
+{% endcode %}
+
+{% code title="dnsconfig.js" %}
+```javascript
+D(REV("1.2.3.0/24", ...
+```
+{% endcode %}
+
+The latter is easier to type and less error-prone.
+
+If the address does not include a "/" then `REV()` assumes /32 for IPv4 addresses
 and /128 for IPv6 addresses.
 
-Note that the lower bits (the ones outside the netmask) must be zeros. They are not
+# RFC compliance
-zeroed out automatically. Thus, `REV("1.2.3.4/24")` is an error.  This is done
+
-to catch typos.
+`REV()` implements both RFC 2317 and the newer RFC 4183. The `REVCOMPAT()`
+function selects which mode is used. If `REVCOMPAT()` is not called, a default
+is selected for you.  The default will change to RFC 4183 in DNSControl v5.0.
+
+See [`REVCOMPAT()`](REVCOMPAT.md) for details.
+
+
+# Host bits
+
+v4.x:
+The host bits (the ones outside the netmask) must be zeros. They are not zeroed
+out automatically. Thus, `REV("1.2.3.4/24")` is an error.
+
+v5.0 and later:
+The host bits (the ones outside the netmask) are ignored.  Thus
+`REV("1.2.3.4/24")` and `REV("1.2.3.0/24")` are equivalent.
+
+# Examples
+
+Here's an example reverse lookup domain:
 
 {% code title="dnsconfig.js" %}
 ```javascript
@@ -38,19 +61,21 @@ D(REV("1.2.3.0/24"), REGISTRAR, DnsProvider(BIND),
   PTR("1", "foo.example.com."),
   PTR("2", "bar.example.com."),
   PTR("3", "baz.example.com."),
-  // These take advantage of DNSControl's ability to generate the right name:
+  // If the first parameter is an IP address, DNSControl automatically calls REV() for you.
   PTR("1.2.3.10", "ten.example.com."),
 );
 
 D(REV("2001:db8:302::/48"), REGISTRAR, DnsProvider(BIND),
   PTR("1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0", "foo.example.com."),  // 2001:db8:302::1
-  // These take advantage of DNSControl's ability to generate the right name:
+  // If the first parameter is an IP address, DNSControl automatically calls REV() for you.
   PTR("2001:db8:302::2", "two.example.com."),                          // 2.0.0...
   PTR("2001:db8:302::3", "three.example.com."),                        // 3.0.0...
 );
 ```
 {% endcode %}
 
-In the future we plan on adding a flag to [`A()`](../domain/A.md)which will insert
+# Automatic forward and reverse record generation
-the correct PTR() record in the appropriate `D(REV())` domain (i.e. `.arpa` domain) has been
+
-defined.
+DNSControl does not automatically generate forward and reverse lookups. However
+it is possible to write a macro that does this.  See
+[`PTR()`](../domain/PTR.md)   for an example.

documentation/functions/global/REVCOMPAT.md

@@ -0,0 +1,47 @@
+---
+name: REVCOMPAT
+parameters:
+  - rfc
+parameter_types:
+  rfc: string
+ts_return: string
+---
+
+`REVCOMPAT()` controls which RFC the [`REV()`](REV.md) function adheres to.
+
+Include one of these two commands near the top `dnsconfig.js` (at the global level):
+
+{% code title="dnsconfig.js" %}
+```javascript
+REVCOMPAT("rfc2317");  // RFC 2117: Compatible with old files.
+REVCOMPAT("rfc4183");  // RFC 4183: Adopt the newer standard.
+```
+{% endcode %}
+
+`REVCOMPAT()` is global for all of `dnsconfig.js`. It must appear before any
+use of `REV()`; If not, behavior is undefined.
+
+# RFC 4183 vs RFC 2317
+
+RFC 2317 and RFC 4183 are two different ways to implement reverse lookups for
+CIDR blocks that are not on 8-bit boundaries (/24, /16, /8).
+
+Originally DNSControl implemented the older standard, which only specifies what
+to do for /8, /16, /24 - /32.  Using `REV()` for /9-17 and /17-23 CIDRs was an
+error.
+
+v4 defaults to RFC 2317.  In v5.0 the default will change to RFC 4183.
+`REVCOMPAT()` is provided for those that wish to retain the old behavior.
+
+For more information, see [Opinion #9](../../opinions.md#opinion-9-rfc-4183-is-better-than-rfc-2317).
+
+# Transition plan
+
+What's the default behavior if `REVCOMPAT()` is not used?
+
+| Version | /9 to /15 and /17 to /23 | /25 to 32 | Warnings                   |
+|---------|--------------------------|-----------|----------------------------|
+| v4      | RFC 4183                 | RFC 2317  | Only if /25 - /32 are used |
+| v5      | RFC 4183                 | RFC 4183  | none                       |
+
+No warnings are generated if the `REVCOMPAT()` function is used.

documentation/opinions.md

@@ -90,7 +90,7 @@ Some examples:
 * SPF records are stated in the most verbose way; DNSControl optimizes it for you in a safe, opt-in way.
 
 
-# Opinion #6 If it is ambiguous in DNS, it is forbidden in DNSControl
+# Opinion #6: If it is ambiguous in DNS, it is forbidden in DNSControl
 
 When there is ambiguity an expert knows what the system will do.
 Your coworkers should not be expected to be experts. (See [Opinion #2](#opinion-2-non-experts-should-be-able-to-safely-make-dns-changes)).
@@ -124,7 +124,7 @@ Therefore, we require all CNAME, MX, and NS targets to be FQDNs (they must
 end with a "."), or to be a shortname (no dots at all).  Everything
 else is ambiguous and therefore an error.
 
-# Opinion #7 Hostnames don't have underscores
+# Opinion #7: Hostnames don't have underscores
 
 DNSControl prints warnings if a hostname includes an underscore
 (`_`) because underscores are not permitted in hostnames.
@@ -151,7 +151,7 @@ unless the rtype is SRV, TLSA, TXT, or if the name starts with
 certain prefixes such as `_dmarc`.  We're always willing to
 [add more exceptions](https://github.com/StackExchange/dnscontrol/pull/453/files).
 
-# Opinion #8 TXT Records are one long string
+# Opinion #8: TXT Records are one long string
 
 * TXT records are a single string with a length of 0 to 65,280 bytes
   (the maximum possible TXT record size).
@@ -180,3 +180,25 @@ control panel let you specify the boundaries, (b) I've never seen a
 FAQ or reddit post asking how to specify those boundaries. Therefore,
 there is no need for this. I also assert that there will be no such
 need in the future.
+
+
+# Opinion #9: RFC 4183 is better than RFC 2317
+
+There is no standard for how to do reverse lookup zones (in-addr.arpa)
+for CIDR blocks that are not /8, /16, or /24. There are only
+recommendations.
+
+RFC 2317 is a good recommendation, but it only covers /25 to /32.
+It also uses `/` in zone names, which many DNS providers do not
+support.
+
+RFC 4183 covers /8 through /32 and uses hyphens, which are supported
+universally.
+
+Originally DNSControl implemented RFC 2317.
+
+In v5.0 we will adopt RFC 4183 as the default.  A new function,
+[REVCOMPAT()](functions/global/REVCOMPAT.md), will be provided to enable backwards compatibility.
+v4.x users can use the function to adopt the new behavior early.
+
+See [REVCOMPAT()](functions/global/REVCOMPAT.md) for details.

pkg/js/js.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/StackExchange/dnscontrol/v4/models"
 	"github.com/StackExchange/dnscontrol/v4/pkg/printer"
+	"github.com/StackExchange/dnscontrol/v4/pkg/rfc4183"
 	"github.com/StackExchange/dnscontrol/v4/pkg/transform"
 	"github.com/robertkrimen/otto"              // load underscore js into vm by default
 	_ "github.com/robertkrimen/otto/underscore" // required by otto
@@ -70,6 +71,7 @@ func ExecuteJavascriptString(script []byte, devMode bool, variables map[string]s
 
 	vm.Set("require", require)
 	vm.Set("REV", reverse)
+	vm.Set("REVCOMPAT", reverseCompat)
 	vm.Set("glob", listFiles) // used for require_glob()
 	vm.Set("PANIC", jsPanic)
 
@@ -290,3 +292,16 @@ func reverse(call otto.FunctionCall) otto.Value {
 	v, _ := otto.ToValue(rev)
 	return v
 }
+
+func reverseCompat(call otto.FunctionCall) otto.Value {
+	if len(call.ArgumentList) != 1 {
+		throw(call.Otto, "REVCOMPAT takes exactly one argument")
+	}
+	dom := call.Argument(0).String()
+	err := rfc4183.SetCompatibilityMode(dom)
+	if err != nil {
+		throw(call.Otto, err.Error())
+	}
+	v, _ := otto.ToValue(nil)
+	return v
+}

pkg/rfc4183/ipv6.go

@@ -0,0 +1,26 @@
+package rfc4183
+
+import (
+	"fmt"
+)
+
+// reverseIPv6 returns the ipv6.arpa string suitable for reverse DNS lookups.
+func reverseIPv6(ip []byte, maskbits int) (arpa string, err error) {
+	// Must be IPv6
+	if len(ip) != 16 {
+		return "", fmt.Errorf("not IPv6")
+	}
+
+	buf := []byte("x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.ip6.arpa")
+	// Poke hex digits into the template
+	pos := 128/4*2 - 2 // Position of the last "x"
+	for _, v := range ip {
+		buf[pos] = hexDigit[v>>4]
+		buf[pos-2] = hexDigit[v&0xF]
+		pos = pos - 4
+	}
+	// Return only the parts without x's
+	return string(buf[(128-maskbits)/4*2:]), nil
+}
+
+const hexDigit = "0123456789abcdef"

pkg/rfc4183/mode.go

@@ -0,0 +1,47 @@
+package rfc4183
+
+import (
+	"fmt"
+	"strings"
+)
+
+var newmode bool
+var modeset bool
+
+func SetCompatibilityMode(m string) error {
+	if modeset {
+		return fmt.Errorf("ERROR: REVCOMPAT() already set")
+	}
+	modeset = true
+
+	switch strings.ToLower(m) {
+	case "rfc2317", "2317", "2", "old":
+		newmode = false
+	case "rfc4183", "4183", "4":
+		newmode = true
+	default:
+		return fmt.Errorf("invalid value %q, must be rfc2317 or rfc4182", m)
+	}
+	return nil
+}
+
+func IsRFC4183Mode() bool {
+	return newmode
+}
+
+var warningNeeded bool = false
+
+func NeedsWarning() {
+	warningNeeded = true
+}
+
+func PrintWarning() {
+	if modeset {
+		// No warnings if REVCOMPAT() was used.
+		return
+	}
+	if !warningNeeded {
+		return
+	}
+	fmt.Printf("WARNING: REV() breaking change coming in v5.0. See https://docs.dnscontrol.org/functions/REVCOMPAT\n")
+}

pkg/rfc4183/reverse.go

@@ -0,0 +1,79 @@
+package rfc4183
+
+import (
+	"fmt"
+	"net/netip"
+	"strings"
+)
+
+// ReverseDomainName implements RFC4183 for turning a CIDR block into
+// a in-addr name.  IP addresses are assumed to be /32 or /128 CIDR blocks.
+// CIDR host bits are changed to 0s.
+func ReverseDomainName(cidr string) (string, error) {
+
+	// Mask missing? Add it.
+	if !strings.Contains(cidr, "/") {
+		a, err := netip.ParseAddr(cidr)
+		if err != nil {
+			return "", fmt.Errorf("not an IP address: %w", err)
+		}
+		if a.Is4() {
+			cidr = cidr + "/32"
+		} else {
+			cidr = cidr + "/128"
+		}
+	}
+
+	// Parse the CIDR.
+	p, err := netip.ParsePrefix(cidr)
+	if err != nil {
+		return "", fmt.Errorf("not a CIDR block: %w", err)
+	}
+
+	// RFC4183 4.1 step 4: The notion of fewer than 8 mask bits is not reasonable.
+	if p.Bits() < 8 {
+		return "", fmt.Errorf("mask fewer than 8 bits is unreasonable: %s", cidr)
+	}
+
+	// Handle IPv6 separately:
+	if p.Addr().Is6() {
+		return reverseIPv6(p.Addr().AsSlice(), p.Bits())
+	}
+
+	// Zero out any host bits.
+	p = p.Masked()
+
+	// IPv4: Implement the RFC4183 process:
+
+	// 4.p Step 1
+	b := p.Addr().AsSlice()
+	x, y, z, w := b[0], b[1], b[2], b[3]
+	m := p.Bits()
+
+	if m == 8 {
+		return fmt.Sprintf("%d.in-addr.arpa", x), nil
+	}
+	if m == 16 {
+		return fmt.Sprintf("%d.%d.in-addr.arpa", y, x), nil
+	}
+	if m == 24 {
+		return fmt.Sprintf("%d.%d.%d.in-addr.arpa", z, y, x), nil
+	}
+	if m == 32 {
+		return fmt.Sprintf("%d.%d.%d.%d.in-addr.arpa", w, z, y, x), nil
+	}
+
+	// 4.1 Step 2
+	n := w // I don't understand why the RFC changes variable names at this point, but it does.
+	if m >= 24 && m <= 32 {
+		return fmt.Sprintf("%d-%d.%d.%d.%d.in-addr.arpa", n, m, z, y, x), nil
+	}
+	if m >= 16 && m < 24 {
+		return fmt.Sprintf("%d-%d.%d.%d.in-addr.arpa", z, m, y, x), nil
+	}
+	if m >= 8 && m < 16 {
+		return fmt.Sprintf("%d-%d.%d.in-addr.arpa", y, m, x), nil
+	}
+	return "", fmt.Errorf("fewer than 8 mask bits is not reasonable: %v", cidr)
+
+}

pkg/rfc4183/reverse_test.go

@@ -0,0 +1,125 @@
+package rfc4183
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestReverse(t *testing.T) {
+	var tests = []struct {
+		in  string
+		out string
+	}{
+		// IPv4 "Classless in-addr.arpa delegation" RFC4183.
+		// Examples in the RFC:
+		{"10.100.2.0/26", "0-26.2.100.10.in-addr.arpa"},
+		{"10.192.0.0/13", "192-13.10.in-addr.arpa"},
+		{"10.20.128.0/23", "128-23.20.10.in-addr.arpa"},
+		{"10.20.129.0/23", "128-23.20.10.in-addr.arpa"}, // Not in the RFC but should be!
+
+		// IPv6
+		{"2001::/16", "1.0.0.2.ip6.arpa"},
+		{"2001:0db8:0123:4567:89ab:cdef:1234:5670/64", "7.6.5.4.3.2.1.0.8.b.d.0.1.0.0.2.ip6.arpa"},
+		{"2001:0db8:0123:4567:89ab:cdef:1234:5670/68", "8.7.6.5.4.3.2.1.0.8.b.d.0.1.0.0.2.ip6.arpa"},
+		{"2001:0db8:0123:4567:89ab:cdef:1234:5670/124", "7.6.5.4.3.2.1.f.e.d.c.b.a.9.8.7.6.5.4.3.2.1.0.8.b.d.0.1.0.0.2.ip6.arpa"},
+		{"2001:0db8:0123:4567:89ab:cdef:1234:5678/128", "8.7.6.5.4.3.2.1.f.e.d.c.b.a.9.8.7.6.5.4.3.2.1.0.8.b.d.0.1.0.0.2.ip6.arpa"},
+
+		// 8-bit boundaries
+		{"174.0.0.0/8", "174.in-addr.arpa"},
+		{"174.136.43.0/8", "174.in-addr.arpa"},
+		{"174.136.0.44/8", "174.in-addr.arpa"},
+		{"174.136.45.45/8", "174.in-addr.arpa"},
+		{"174.136.0.0/16", "136.174.in-addr.arpa"},
+		{"174.136.43.0/16", "136.174.in-addr.arpa"},
+		{"174.136.44.255/16", "136.174.in-addr.arpa"},
+		{"174.136.107.0/24", "107.136.174.in-addr.arpa"},
+		{"174.136.107.1/24", "107.136.174.in-addr.arpa"},
+		{"174.136.107.14/32", "14.107.136.174.in-addr.arpa"},
+
+		// /25 (all cases)
+		{"174.1.0.0/25", "0-25.0.1.174.in-addr.arpa"},
+		{"174.1.0.128/25", "128-25.0.1.174.in-addr.arpa"},
+		{"174.1.0.129/25", "128-25.0.1.174.in-addr.arpa"}, // host bits
+		// /26 (all cases)
+		{"174.1.0.0/26", "0-26.0.1.174.in-addr.arpa"},
+		{"174.1.0.0/26", "0-26.0.1.174.in-addr.arpa"},
+		{"174.1.0.64/26", "64-26.0.1.174.in-addr.arpa"},
+		{"174.1.0.128/26", "128-26.0.1.174.in-addr.arpa"},
+		{"174.1.0.192/26", "192-26.0.1.174.in-addr.arpa"},
+		{"174.1.0.194/26", "192-26.0.1.174.in-addr.arpa"}, // host bits
+		// /27 (all cases)
+		{"174.1.0.0/27", "0-27.0.1.174.in-addr.arpa"},
+		{"174.1.0.32/27", "32-27.0.1.174.in-addr.arpa"},
+		{"174.1.0.64/27", "64-27.0.1.174.in-addr.arpa"},
+		{"174.1.0.96/27", "96-27.0.1.174.in-addr.arpa"},
+		{"174.1.0.128/27", "128-27.0.1.174.in-addr.arpa"},
+		{"174.1.0.160/27", "160-27.0.1.174.in-addr.arpa"},
+		{"174.1.0.192/27", "192-27.0.1.174.in-addr.arpa"},
+		{"174.1.0.224/27", "224-27.0.1.174.in-addr.arpa"},
+		{"174.1.0.225/27", "224-27.0.1.174.in-addr.arpa"}, // host bits
+		// /28 (first 2, last 2)
+		{"174.1.0.0/28", "0-28.0.1.174.in-addr.arpa"},
+		{"174.1.0.16/28", "16-28.0.1.174.in-addr.arpa"},
+		{"174.1.0.224/28", "224-28.0.1.174.in-addr.arpa"},
+		{"174.1.0.240/28", "240-28.0.1.174.in-addr.arpa"},
+		{"174.1.0.241/28", "240-28.0.1.174.in-addr.arpa"}, // host bits
+		// /29 (first 2 cases)
+		{"174.1.0.0/29", "0-29.0.1.174.in-addr.arpa"},
+		{"174.1.0.8/29", "8-29.0.1.174.in-addr.arpa"},
+		{"174.1.0.9/29", "8-29.0.1.174.in-addr.arpa"}, // host bits
+		// /30 (first 2 cases)
+		{"174.1.0.0/30", "0-30.0.1.174.in-addr.arpa"},
+		{"174.1.0.4/30", "4-30.0.1.174.in-addr.arpa"},
+		{"174.1.0.5/30", "4-30.0.1.174.in-addr.arpa"}, // host bits
+		// /31 (first 2 cases)
+		{"174.1.0.0/31", "0-31.0.1.174.in-addr.arpa"},
+		{"174.1.0.2/31", "2-31.0.1.174.in-addr.arpa"},
+		{"174.1.0.3/31", "2-31.0.1.174.in-addr.arpa"}, // host bits
+
+		// Other tests:
+		{"10.100.2.255/23", "2-23.100.10.in-addr.arpa"},
+		{"10.100.2.255/22", "0-22.100.10.in-addr.arpa"},
+		{"10.100.2.255/21", "0-21.100.10.in-addr.arpa"},
+		{"10.100.2.255/20", "0-20.100.10.in-addr.arpa"},
+		{"10.100.2.255/19", "0-19.100.10.in-addr.arpa"},
+		{"10.100.2.255/18", "0-18.100.10.in-addr.arpa"},
+		{"10.100.2.255/17", "0-17.100.10.in-addr.arpa"},
+		//
+		{"10.100.2.255/15", "100-15.10.in-addr.arpa"},
+		{"10.100.2.255/14", "100-14.10.in-addr.arpa"},
+		{"10.100.2.255/13", "96-13.10.in-addr.arpa"},
+		{"10.100.2.255/12", "96-12.10.in-addr.arpa"},
+		{"10.100.2.255/11", "96-11.10.in-addr.arpa"},
+		{"10.100.2.255/10", "64-10.10.in-addr.arpa"},
+		{"10.100.2.255/9", "0-9.10.in-addr.arpa"},
+	}
+	for i, tst := range tests {
+		t.Run(fmt.Sprintf("%d--%s", i, tst.in), func(t *testing.T) {
+			d, err := ReverseDomainName(tst.in)
+			if err != nil {
+				t.Errorf("Should not have errored: %v", err)
+			} else if d != tst.out {
+				t.Errorf("Expected '%s' but got '%s'", tst.out, d)
+			}
+		})
+	}
+}
+
+func TestReverseErrors(t *testing.T) {
+	var tests = []struct {
+		in string
+	}{
+		{"0.0.0.0/0"},
+		{"2001::/0"},
+		{"4.5/16"},
+		{"foo.com"},
+	}
+	for i, tst := range tests {
+		t.Run(fmt.Sprintf("%d--%s", i, tst.in), func(t *testing.T) {
+			d, err := ReverseDomainName(tst.in)
+			if err == nil {
+				t.Errorf("Should have errored, but didn't. Got %s", d)
+			}
+		})
+	}
+}

pkg/transform/arpa.go

@@ -2,115 +2,60 @@ package transform
 
 import (
 	"fmt"
-	"net"
+	"net/netip"
 	"strings"
+
+	"github.com/StackExchange/dnscontrol/v4/pkg/rfc4183"
 )
 
 // ReverseDomainName turns a CIDR block into a reversed (in-addr) name.
+// For cases not covered by RFC2317, implement RFC4183
+// The host bits must all be zeros.
 func ReverseDomainName(cidr string) (string, error) {
 
-	// If it is an IP address, add the /32 or /128
+	if rfc4183.IsRFC4183Mode() {
-	ip := net.ParseIP(cidr)
+		return rfc4183.ReverseDomainName(cidr)
-	if ip != nil {
+	}
-		if ip.To4() != nil {
+
+	// Mask missing? Add it.
+	if !strings.Contains(cidr, "/") {
+		a, err := netip.ParseAddr(cidr)
+		if err != nil {
+			return "", fmt.Errorf("not an IP address: %w", err)
+		}
+		if a.Is4() {
 			cidr = cidr + "/32"
 		} else {
 			cidr = cidr + "/128"
 		}
 	}
 
-	a, c, err := net.ParseCIDR(cidr)
+	// Parse the CIDR.
+	p, err := netip.ParsePrefix(cidr)
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("not a CIDR block: %w", err)
 	}
-	base, err := reverseaddr(a.String())
+	bits := p.Bits()
-	if err != nil {
+
-		return "", err
+	if p.Masked() != p {
-	}
-	base = strings.TrimRight(base, ".")
-	if !a.Equal(c.IP) {
 		return "", fmt.Errorf("CIDR %v has 1 bits beyond the mask", cidr)
 	}
 
-	bits, total := c.Mask.Size()
+	// Cases where RFC4183 is the same as RFC2317:
-	var toTrim int
+	// IPV6, /0 - /24, /32
-	if bits == 0 {
+	if strings.Contains(cidr, ":") || bits <= 24 || bits == 32 {
-		return "", fmt.Errorf("cannot use /0 in reverse CIDR")
+		// There is no p.Is6() so we test for ":" as a workaround.
+		return rfc4183.ReverseDomainName(cidr)
 	}
 
+	// Record that the change to --revmode default will affect this configuration
+	rfc4183.NeedsWarning()
+
 	// Handle IPv4 "Classless in-addr.arpa delegation" RFC2317:
-	if total == 32 && bits >= 25 && bits < 32 {
+	// if bits >= 25 && bits < 32 {
-		// first address / netmask . Class-b-arpa.
+	// first address / netmask . Class-b-arpa.
-		fparts := strings.Split(c.IP.String(), ".")
-		first := fparts[3]
-		bparts := strings.SplitN(base, ".", 2)
-		return fmt.Sprintf("%s/%d.%s", first, bits, bparts[1]), nil
-	}
 
-	// Handle IPv4 Class-full and IPv6:
+	ip := p.Addr().AsSlice()
-	if total == 32 {
+	return fmt.Sprintf("%d/%d.%d.%d.%d.in-addr.arpa",
-		if bits%8 != 0 {
+		ip[3], bits, ip[2], ip[1], ip[0]), nil
-			return "", fmt.Errorf("IPv4 mask must be multiple of 8 bits")
-		}
-		toTrim = (total - bits) / 8
-	} else if total == 128 {
-		if bits%4 != 0 {
-			return "", fmt.Errorf("IPv6 mask must be multiple of 4 bits")
-		}
-		toTrim = (total - bits) / 4
-	} else {
-		return "", fmt.Errorf("invalid address (not IPv4 or IPv6): %v", cidr)
-	}
-
-	parts := strings.SplitN(base, ".", toTrim+1)
-	return parts[len(parts)-1], nil
 }
-
-// copied from go source.
-// https://github.com/golang/go/blob/bfc164c64d33edfaf774b5c29b9bf5648a6447fb/src/net/dnsclient.go#L15
-
-// reverseaddr returns the in-addr.arpa. or ip6.arpa. hostname of the IP
-// address addr suitable for rDNS (PTR) record lookup or an error if it fails
-// to parse the IP address.
-func reverseaddr(addr string) (arpa string, err error) {
-	ip := net.ParseIP(addr)
-	if ip == nil {
-		return "", &net.DNSError{Err: "unrecognized address", Name: addr}
-	}
-	if ip.To4() != nil {
-		return uitoa(uint(ip[15])) + "." + uitoa(uint(ip[14])) + "." + uitoa(uint(ip[13])) + "." + uitoa(uint(ip[12])) + ".in-addr.arpa.", nil
-	}
-	// Must be IPv6
-	buf := make([]byte, 0, len(ip)*4+len("ip6.arpa."))
-	// Add it, in reverse, to the buffer
-	for i := len(ip) - 1; i >= 0; i-- {
-		v := ip[i]
-		buf = append(buf, hexDigit[v&0xF])
-		buf = append(buf, '.')
-		buf = append(buf, hexDigit[v>>4])
-		buf = append(buf, '.')
-	}
-	// Append "ip6.arpa." and return (buf already has the final .)
-	buf = append(buf, "ip6.arpa."...)
-	return string(buf), nil
-}
-
-// Convert unsigned integer to decimal string.
-func uitoa(val uint) string {
-	if val == 0 { // avoid string allocation
-		return "0"
-	}
-	var buf [20]byte // big enough for 64bit value base 10
-	i := len(buf) - 1
-	for val >= 10 {
-		q := val / 10
-		buf[i] = byte('0' + val - q*10)
-		i--
-		val = q
-	}
-	// val < 10
-	buf[i] = byte('0' + val)
-	return string(buf[i:])
-}
-
-const hexDigit = "0123456789abcdef"

pkg/transform/arpa_test.go

@@ -73,6 +73,10 @@ func TestReverse(t *testing.T) {
 		{"174.1.0.0/31", false, "0/31.0.1.174.in-addr.arpa"},
 		{"174.1.0.2/31", false, "2/31.0.1.174.in-addr.arpa"},
 
+		// Use RFC4183 for cases not covered by RFC2317:
+		{"10.20.128.0/23", false, "128-23.20.10.in-addr.arpa"},
+		{"10.192.0.0/13", false, "192-13.10.in-addr.arpa"},
+
 		// Error Cases:
 		{"0.0.0.0/0", true, ""},
 		{"2001::/0", true, ""},