Development discussion of WireGuard
 help / color / mirror / Atom feed
* [PATCH] Fixed the failing BadConfigExceptionTest unit test
@ 2022-01-19  8:28 Michal Murin
  2022-01-26 15:02 ` Harsh Shandilya
  2022-01-26 18:25 ` Jason A. Donenfeld
  0 siblings, 2 replies; 3+ messages in thread
From: Michal Murin @ 2022-01-19  8:28 UTC (permalink / raw)
  To: wireguard; +Cc: Michal Murin

Fixed the test by changing the DNS to a string with an invalid char in the `invalid-value.conf` test configuration file. Also removed the `getParsingClass()` condition from the `parseDnsServers()` method as the condition can be never met - the `InetAddresses.parse(dnsServer)` method always throws the `ParseException` with the `parsingClass` set to `InetAddress.class`.
---
 tunnel/src/main/java/com/wireguard/config/Interface.java | 2 +-
 tunnel/src/test/resources/invalid-value.conf             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tunnel/src/main/java/com/wireguard/config/Interface.java b/tunnel/src/main/java/com/wireguard/config/Interface.java
index 694f313..5bd4da7 100644
--- a/tunnel/src/main/java/com/wireguard/config/Interface.java
+++ b/tunnel/src/main/java/com/wireguard/config/Interface.java
@@ -356,7 +356,7 @@ public final class Interface {
                     try {
                         addDnsServer(InetAddresses.parse(dnsServer));
                     } catch (final ParseException e) {
-                        if (e.getParsingClass() != InetAddress.class || !InetAddresses.isHostname(dnsServer))
+                        if (!InetAddresses.isHostname(dnsServer))
                             throw e;
                         addDnsSearchDomain(dnsServer);
                     }
diff --git a/tunnel/src/test/resources/invalid-value.conf b/tunnel/src/test/resources/invalid-value.conf
index 2889111..6a1e3b6 100644
--- a/tunnel/src/test/resources/invalid-value.conf
+++ b/tunnel/src/test/resources/invalid-value.conf
@@ -1,6 +1,6 @@
 [Interface]
 Address = 192.0.2.2/32,2001:db8:ffff:ffff:ffff:ffff:ffff:ffff/128
-DNS = 192.0.2.0,yes
+DNS = 192.0.2.0,invalid_value
 PrivateKey = TFlmmEUC7V7VtiDYLKsbP5rySTKLIZq1yn8lMqK83wo=
 [Peer]
 AllowedIPs = 0.0.0.0/0, ::0/0
-- 
2.30.1 (Apple Git-130)


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fixed the failing BadConfigExceptionTest unit test
  2022-01-19  8:28 [PATCH] Fixed the failing BadConfigExceptionTest unit test Michal Murin
@ 2022-01-26 15:02 ` Harsh Shandilya
  2022-01-26 18:25 ` Jason A. Donenfeld
  1 sibling, 0 replies; 3+ messages in thread
From: Harsh Shandilya @ 2022-01-26 15:02 UTC (permalink / raw)
  To: Michal Murin; +Cc: wireguard


Hey Michal,

On Jan 19 2022, at 1:58 pm, Michal Murin <michal.murin@jamf.com> wrote:

> Fixed the test by changing the DNS to a string with an invalid char in
> the `invalid-value.conf` test configuration file. Also removed the
> `getParsingClass()` condition from the `parseDnsServers()` method as
> the condition can be never met - the `InetAddresses.parse(dnsServer)`
> method always throws the `ParseException` with the `parsingClass` set
> to `InetAddress.class`.
> ---
> tunnel/src/main/java/com/wireguard/config/Interface.java | 2 +-
> tunnel/src/test/resources/invalid-value.conf             | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tunnel/src/main/java/com/wireguard/config/Interface.java b/tunnel/src/main/java/com/wireguard/config/Interface.java
> index 694f313..5bd4da7 100644
> --- a/tunnel/src/main/java/com/wireguard/config/Interface.java
> +++ b/tunnel/src/main/java/com/wireguard/config/Interface.java
> @@ -356,7 +356,7 @@ public final class Interface {
>                     try {
>                         addDnsServer(InetAddresses.parse(dnsServer));
>                     } catch (final ParseException e) {
> -                        if (e.getParsingClass() != InetAddress.class
> || !InetAddresses.isHostname(dnsServer))
> +                        if (!InetAddresses.isHostname(dnsServer))
>                             throw e;
>                         addDnsSearchDomain(dnsServer);
>                     }
> diff --git a/tunnel/src/test/resources/invalid-value.conf b/tunnel/src/test/resources/invalid-value.conf
> index 2889111..6a1e3b6 100644
> --- a/tunnel/src/test/resources/invalid-value.conf
> +++ b/tunnel/src/test/resources/invalid-value.conf
> @@ -1,6 +1,6 @@
> [Interface]
> Address = 192.0.2.2/32,2001:db8:ffff:ffff:ffff:ffff:ffff:ffff/128
> -DNS = 192.0.2.0,yes
> +DNS = 192.0.2.0,invalid_value
> PrivateKey = TFlmmEUC7V7VtiDYLKsbP5rySTKLIZq1yn8lMqK83wo=
> [Peer]
> AllowedIPs = 0.0.0.0/0, ::0/0
> -- 
> 2.30.1 (Apple Git-130)
> 
> 

Thanks! The patch looks good, if you can send a v2 with a Signed-off-by
line I'd be happy to apply this.

Cheers,
Harsh Shandilya

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fixed the failing BadConfigExceptionTest unit test
  2022-01-19  8:28 [PATCH] Fixed the failing BadConfigExceptionTest unit test Michal Murin
  2022-01-26 15:02 ` Harsh Shandilya
@ 2022-01-26 18:25 ` Jason A. Donenfeld
  1 sibling, 0 replies; 3+ messages in thread
From: Jason A. Donenfeld @ 2022-01-26 18:25 UTC (permalink / raw)
  To: Michal Murin; +Cc: WireGuard mailing list

On Tue, Jan 25, 2022 at 5:12 PM Michal Murin <michal.murin@jamf.com> wrote:
> Also removed the `getParsingClass()` condition from the `parseDnsServers()` method as the condition can be never met - the `InetAddresses.parse(dnsServer)` method always throws the `ParseException` with the `parsingClass` set to `InetAddress.class`.

Until one day it doesn't, and then we have to remember this subtle
behavior. On the contrary, what's there now means that the code is
correct regardless of changes.

> -                        if (e.getParsingClass() != InetAddress.class || !InetAddresses.isHostname(dnsServer))
> +                        if (!InetAddresses.isHostname(dnsServer))

Please drop this snippet from v2.

As Harsh said, we need your sign off line. It would also be
appreciated if you'd wrap your commit message and adjust your commit
subject to match what the project uses conventionally.

Thanks,
Jason

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-01-26 18:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-19  8:28 [PATCH] Fixed the failing BadConfigExceptionTest unit test Michal Murin
2022-01-26 15:02 ` Harsh Shandilya
2022-01-26 18:25 ` Jason A. Donenfeld

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).