#9 Time for a new Release?

[Andi]

Hi Martin,
hi all!

I am currently working on the Debian atftp package for bookworm. I thought that with the implementation of the windowsize option and the switch from old PCRE to actively maintained PCRE2 it would be nice to prepare a new release 0.8.0. (Of course with the patches in https://sourceforge.net/p/atftp/bugs/8/ and https://sourceforge.net/p/atftp/support-requests/11/). Carrying the patches around makes packaging more complicated and distributions should build on a common upstream if possible.

What do you think?

Best regards,

Andi

[Martin Dummer]

Hello Andi,
at first thanks for you efforts in fixing bugs and developing.
Some time ago I took your PCRE2 patch, adapted the test suite to evaluate PCRE and made some tests with PCRE1 and PCRE2. AFAIR there were at least small differences in behaviour.
I will test again and post results at the right place.
Then we can decide how to continue. PCRE2, maybe with behaviour changes, would qualify for a 8.0.
I'll call you back....
Martin

[Andi]

Hi Martin,
thanks for your quick reply!

Some time ago I took your PCRE2 patch, adapted the test suite to evaluate PCRE and made some tests with PCRE1 and PCRE2. AFAIR there were at least small differences in behaviour.

Very interesting! I tried the test suite with the old implementation, and indeed:

 Substitution: "ppxelinux.cfg/012345" -> "pxelinux.cfg/default"
 Substitution: "ppxelinux.cfg/678" -> "pxelinux.cfg/default"
 Substitution: "ppxelinux.cfg/9ABCDE" -> "pxelinux.cfg/default"
-Substitution: "pppxelinux.0" -> "linux"
+Substitution: "pppxelinux.0" -> "pppxelinux.0"
 Substitution: "pxelinux.cfg/F" -> "pxelinux.cfg/default"
 Substitution: "linux" -> "linux"
-Substitution: "something_linux_like" -> "linux"
+Substitution: "something_linux_like" -> "something_linux_like"
 Substitution: "str" -> "replaced1"
-Substitution: "strong" -> "replaced2"
-Substitution: "validstr" -> "replaced3"
-Substitution: "doreplacethis" -> "mace"
+Substitution: "strong" -> "replaced2ong"
+Substitution: "validstr" -> "validreplaced3"
+Substitution: "doreplacethis" -> "domacethis"
 Substitution: "any.conf" -> "master.conf"

I wrote a little test script to understand the difference, attached. The interesting parts (old implementation) are:

----------
'strong':
strong strong strong strong replaced2ong 
perl -e '$x = "strong"; $x =~ s#^str#replaced2#; print "$x\n";'
Perl:   replacement: "strong" -> "replaced2ong"
atftp: Substitution: "strong" -> "replaced2"

----------
'validstr':
validstr validstr validstr validstr validstr validreplaced3 
perl -e '$x = "validstr"; $x =~ s#str$#replaced3#; print "$x\n";'
Perl:   replacement: "validstr" -> "validreplaced3"
atftp: Substitution: "validstr" -> "replaced3"

----------
'doreplacethis':
doreplacethis doreplacethis doreplacethis doreplacethis doreplacethis doreplacethis domacethis 
perl -e '$x = "doreplacethis"; $x =~ s#repl(ace)#m$1#; print "$x\n";'
Perl:   replacement: "doreplacethis" -> "domacethis"
atftp: Substitution: "doreplacethis" -> "mace"

For the new implementation, the replacemants are identical in all tested cases. The difference seems to be that the old implementation drops all parts that are not mached, kind of 'non-greedy' pattern matching. I'll continue investigating.

Best regards, Andi

[Andi]

One correction:

For the new implementation, the replacements are identical in all tested cases.

This is true except there is no match at all. Hm. I guess the best is to always return the same like:
perl -e '$x = "nomatchatall"; $x =~ s#PATTERN#SUBSTITUTION#; print "$x\n";'
So if there is no match, the initial string should be returned.

[Andi]

I think it's all fine with the new implementation:

atftp$ grep -A5 tftpd_pcre_sub *.c
tftpd.c:                    if (tftpd_pcre_sub(pcre_top, out, MAXLEN, string) < 0)
tftpd.c-                         printf("Substitution: \"%s\" -> \"\"\n", string);
tftpd.c-                    else
tftpd.c-                         printf("Substitution: \"%s\" -> \"%s\"\n", string, out);

and

/* search for a replacement and return a string after substitution */
/* if no match is found return -1 */
int tftpd_pcre_sub(tftpd_pcre_self_t *self, char *outstr, int outlen, char *str)

From man atftpd:

Upon reception of a read request,  the  server  will first try to
open the file name requested. If it fails, then it will search for a
replacement based on the content of the pattern file.

This works fine, as tested with an improved test.sh (attached).

[Andi]

This works fine, as tested with an improved test.sh (attached).

[Andi]

I have mostly rewritten the test script. Functionality is almost the same, but more structured and mostly cosmetics. The only part not tested is the timeout, which needs debugging in atftp enabled.
Best regards, Andi

[Andi]

The only part not tested is the timeout, which needs debugging in atftp enabled.

I've tested that now, too. Latest test script version attached.

[Andi]

I attached a patch improving the handling of lost or duplicated datagrams in https://sourceforge.net/p/atftp/support-requests/10/ .
This patch should be applied in case of a release too.

[Andi]

Hi, I tested (and fixed, see https://sourceforge.net/p/atftp/support-requests/10/ ) multicast and added the routines to the test script. For me, it needs other machines in the network to test and seems not to be possible on the local machine only. (At least without extra effort, but I am happy to learn better ways.)

I set up 12 mactap'ed VMs:

===== Test multicast option ...
  Run atftp on: 'ansible@daily0.lan', 'ansible@daily1.lan', 'ansible@daily2.lan', … , fetching '128K.bin'.
  Copy log file: 'ansible@daily0.lan' 'ansible@daily1.lan' 'ansible@daily2.lan' … 

  All 72 multicast downloads registered → OK
  Check md5sums → 128K.bin: OK
  Multicast client transfers detected: 71 → OK

A wireshark capture looks fine for these transfers too. :-)

Note that <interface type='direct' trustGuestRxFilters='yes'> is needed for the VMs (libvirt/kvm) to see the multicast packets.

[Martin Dummer]

Hi Adni,
I merged all your patches and pushed the repositories.
Please check is all your fixes and patches arrived properly.
Especially the PCD2 implenetation needs a check - I committed the patch from december and you mention a fixed version - but I see no newer patch.
PLS let me know.
Bye
Martin

[Andi]

Hi Martin,
great, many thanks! I checked differences from commit 982ba233fc6eac35 to current Debian sources after applying all patches there: It's all fine! Differences are only white space and typos (Debian sources miss some upstream fixes/cleanup). This is also the case for the PCRE part.

So from my point of view, it's good to go!
Best regards, Andi

[Andi]

Hi Martin,
many thanks for preparing the release. One minor cosmetic thing is the version string, patch attached.
Best regards, Andi

[Andi]

The release has happened, I guess this bug can be closed. Thanks!