Hi everyone, Many thanks for all your great feedback.
> In any case, Aleksander, I don't mean to sign you up for all that; the > `ssl` suite also seems good enough to me if you're interested in > pursuing that side of the patch further. OK, I moved the named tests to src/test/ssl/t/003_sslinfo.pl: ``` $ PG_TEST_EXTRA=ssl meson test -C build --suite postgresql:ssl $ less build/meson-logs/testlog.txt [...] ok 22 - inet_client_addr() returned non-NULL value ok 23 - inet_client_port() returned non-NULL value ok 24 - inet_server_addr() returned non-NULL value ok 25 - inet_server_port() returned non-NULL value [...] ``` > Adding the following cases to the set_masklen() > test would further improve coverage. > * netmask = -1 > * netmask > maxvalue(33 when ipv4) Good idea. I included the proposed tests into the attached patch v2. While on it I noticed the following comment: ``` -- check the conversion to/from text and set_netmask ``` Since we don't have set_netmask() I think the comment is wrong, so I corrected it. -- Best regards, Aleksander Alekseev
From cd27cd694b53c4c8914ef9467a7af69de91017d5 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev <aleksander@timescale.com> Date: Thu, 31 Oct 2024 17:54:56 +0300 Subject: [PATCH v2] Improve code coverage of network address functions The following functions were not covered by tests: - abbrev(inet) - set_masklen(cidr,int4) - netmask(inet) - hostmask(inet) - inet_client_addr() - inet_client_port() - inet_server_addr() - inet_server_port() Correct this. Aleksander Alekseev, reviewed by Keisuke Kuroda, Jacob Champion, Tom Lane Discussion: https://postgr.es/m/CAJ7c6TOyZ9bGNrDK6Z3Q0gr9ow8ZpOm+=+01mpE0dsdH4C+u9A@mail.gmail.com --- src/test/regress/expected/inet.out | 118 ++++++++++++++++++++++++++++- src/test/regress/sql/inet.sql | 14 +++- src/test/ssl/t/003_sslinfo.pl | 19 +++++ 3 files changed, 149 insertions(+), 2 deletions(-) diff --git a/src/test/regress/expected/inet.out b/src/test/regress/expected/inet.out index b6895d9ced0..056b283bc32 100644 --- a/src/test/regress/expected/inet.out +++ b/src/test/regress/expected/inet.out @@ -190,6 +190,72 @@ SELECT c AS cidr, masklen(c) AS "masklen(cidr)", 10.0.0.0/8 | 8 | 9.1.2.3/8 | 8 (4 rows) +SELECT i AS inet, abbrev(i) AS "abbrev(inet)" FROM INET_TBL; + inet | abbrev(inet) +------------------+------------------ + 192.168.1.226/24 | 192.168.1.226/24 + 192.168.1.226 | 192.168.1.226 + 192.168.1.0/24 | 192.168.1.0/24 + 192.168.1.0/25 | 192.168.1.0/25 + 192.168.1.255/24 | 192.168.1.255/24 + 192.168.1.255/25 | 192.168.1.255/25 + 10.1.2.3/8 | 10.1.2.3/8 + 10.1.2.3/8 | 10.1.2.3/8 + 10.1.2.3 | 10.1.2.3 + 10.1.2.3/24 | 10.1.2.3/24 + 10.1.2.3/16 | 10.1.2.3/16 + 10.1.2.3/8 | 10.1.2.3/8 + 11.1.2.3/8 | 11.1.2.3/8 + 9.1.2.3/8 | 9.1.2.3/8 + 10:23::f1/64 | 10:23::f1/64 + 10:23::ffff | 10:23::ffff + ::4.3.2.1/24 | ::4.3.2.1/24 +(17 rows) + +SELECT i AS inet, netmask(i) AS "netmask(inet)" FROM INET_TBL; + inet | netmask(inet) +------------------+----------------------------------------- + 192.168.1.226/24 | 255.255.255.0 + 192.168.1.226 | 255.255.255.255 + 192.168.1.0/24 | 255.255.255.0 + 192.168.1.0/25 | 255.255.255.128 + 192.168.1.255/24 | 255.255.255.0 + 192.168.1.255/25 | 255.255.255.128 + 10.1.2.3/8 | 255.0.0.0 + 10.1.2.3/8 | 255.0.0.0 + 10.1.2.3 | 255.255.255.255 + 10.1.2.3/24 | 255.255.255.0 + 10.1.2.3/16 | 255.255.0.0 + 10.1.2.3/8 | 255.0.0.0 + 11.1.2.3/8 | 255.0.0.0 + 9.1.2.3/8 | 255.0.0.0 + 10:23::f1/64 | ffff:ffff:ffff:ffff:: + 10:23::ffff | ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff + ::4.3.2.1/24 | ffff:ff00:: +(17 rows) + +SELECT i AS inet, hostmask(i) AS "hostmask(inet)" FROM INET_TBL; + inet | hostmask(inet) +------------------+------------------------------------ + 192.168.1.226/24 | 0.0.0.255 + 192.168.1.226 | 0.0.0.0 + 192.168.1.0/24 | 0.0.0.255 + 192.168.1.0/25 | 0.0.0.127 + 192.168.1.255/24 | 0.0.0.255 + 192.168.1.255/25 | 0.0.0.127 + 10.1.2.3/8 | 0.255.255.255 + 10.1.2.3/8 | 0.255.255.255 + 10.1.2.3 | 0.0.0.0 + 10.1.2.3/24 | 0.0.0.255 + 10.1.2.3/16 | 0.0.255.255 + 10.1.2.3/8 | 0.255.255.255 + 11.1.2.3/8 | 0.255.255.255 + 9.1.2.3/8 | 0.255.255.255 + 10:23::f1/64 | ::ffff:ffff:ffff:ffff + 10:23::ffff | :: + ::4.3.2.1/24 | 0:ff:ffff:ffff:ffff:ffff:ffff:ffff +(17 rows) + SELECT c AS cidr, i AS inet FROM INET_TBL WHERE c = i; cidr | inet @@ -238,7 +304,7 @@ SELECT max(c) AS max, min(c) AS min FROM INET_TBL; 10:23::8000/113 | 10.0.0.0/8 (1 row) --- check the conversion to/from text and set_netmask +-- check the conversion to/from text and setting netmask SELECT set_masklen(inet(text(i)), 24) FROM INET_TBL; set_masklen ------------------ @@ -261,6 +327,56 @@ SELECT set_masklen(inet(text(i)), 24) FROM INET_TBL; ::4.3.2.1/24 (17 rows) +SELECT set_masklen(cidr(text(c)), 24) FROM INET_TBL; + set_masklen +---------------- + 192.168.1.0/24 + 192.168.1.0/24 + 192.168.1.0/24 + 192.168.1.0/24 + 192.168.1.0/24 + 192.168.1.0/24 + 10.0.0.0/24 + 10.0.0.0/24 + 10.1.2.0/24 + 10.1.2.0/24 + 10.1.0.0/24 + 10.0.0.0/24 + 10.0.0.0/24 + 10.0.0.0/24 + 10::/24 + 10::/24 + ::/24 +(17 rows) + +-- check that netmask is treated as maximum value when it equals -1 +SELECT set_masklen(cidr(text(c)), -1) FROM INET_TBL; + set_masklen +-------------------- + 192.168.1.0/32 + 192.168.1.0/32 + 192.168.1.0/32 + 192.168.1.0/32 + 192.168.1.0/32 + 192.168.1.0/32 + 10.0.0.0/32 + 10.0.0.0/32 + 10.1.2.3/32 + 10.1.2.0/32 + 10.1.0.0/32 + 10.0.0.0/32 + 10.0.0.0/32 + 10.0.0.0/32 + 10:23::f1/128 + 10:23::8000/128 + ::ffff:1.2.3.4/128 +(17 rows) + +-- check that invalid netmask is rejected +SELECT set_masklen(inet(text(i)), 33) FROM INET_TBL; +ERROR: invalid mask length: 33 +SELECT set_masklen(cidr(text(c)), 33) FROM INET_TBL; +ERROR: invalid mask length: 33 -- check that btree index works correctly CREATE INDEX inet_idx1 ON inet_tbl(i); SET enable_seqscan TO off; diff --git a/src/test/regress/sql/inet.sql b/src/test/regress/sql/inet.sql index 3910eac3bc4..a946d7d170b 100644 --- a/src/test/regress/sql/inet.sql +++ b/src/test/regress/sql/inet.sql @@ -46,6 +46,10 @@ SELECT c AS cidr, masklen(c) AS "masklen(cidr)", i AS inet, masklen(i) AS "masklen(inet)" FROM INET_TBL WHERE masklen(c) <= 8; +SELECT i AS inet, abbrev(i) AS "abbrev(inet)" FROM INET_TBL; +SELECT i AS inet, netmask(i) AS "netmask(inet)" FROM INET_TBL; +SELECT i AS inet, hostmask(i) AS "hostmask(inet)" FROM INET_TBL; + SELECT c AS cidr, i AS inet FROM INET_TBL WHERE c = i; @@ -60,8 +64,16 @@ SELECT i, c, SELECT max(i) AS max, min(i) AS min FROM INET_TBL; SELECT max(c) AS max, min(c) AS min FROM INET_TBL; --- check the conversion to/from text and set_netmask +-- check the conversion to/from text and setting netmask SELECT set_masklen(inet(text(i)), 24) FROM INET_TBL; +SELECT set_masklen(cidr(text(c)), 24) FROM INET_TBL; + +-- check that netmask is treated as maximum value when it equals -1 +SELECT set_masklen(cidr(text(c)), -1) FROM INET_TBL; + +-- check that invalid netmask is rejected +SELECT set_masklen(inet(text(i)), 33) FROM INET_TBL; +SELECT set_masklen(cidr(text(c)), 33) FROM INET_TBL; -- check that btree index works correctly CREATE INDEX inet_idx1 ON inet_tbl(i); diff --git a/src/test/ssl/t/003_sslinfo.pl b/src/test/ssl/t/003_sslinfo.pl index b9eae8d641b..780c8b29452 100644 --- a/src/test/ssl/t/003_sslinfo.pl +++ b/src/test/ssl/t/003_sslinfo.pl @@ -191,4 +191,23 @@ foreach my $c (@cases) "ssl_client_cert_present() for $c->{'opts'}"); } +# Make sure the following functions are callable and don't crash. +# Unlike SQL tests this TAP test uses TCP connections. This makes it +# a good place for testing named functions due to better code coverage. +my @inet_funcs = qw( + inet_client_addr + inet_client_port + inet_server_addr + inet_server_port +); + +for my $f (@inet_funcs) +{ + $result = $node->safe_psql( + "certdb", + "SELECT $f() IS NULL;", + connstr => $common_connstr); + is($result, 'f', "$f() returned non-NULL value"); +} + done_testing(); -- 2.48.1
