On 8/16/2019 8:34 PM, Nishanth Aravamudan wrote:
> On 15.08.2019 [17:08:39 +0200], Cyril Brulebois wrote:
>> Nishanth Aravamudan <naravamu...@digitalocean.com> (2019-08-14):
>>> We are able to reproduce this issue at will in Ubuntu Bionic's
>>> installer (not identical to Debian's, but code-wise in this path the
>>> same). While quite a while after the last update from Philipp, we
>>> tested the patch (netcfg_dhcp_domain.patch) after updating it to avoid
>>> a compilation issue, we found it did fix the problem for us.
>>>
>>> I am not sure if I can get Debian into our infrastructure to test
>>> explicitly, but I will work on it; at the same time, the code change
>>> seems straightforward.
>>
>> Thanks for your feedback. Care to share the fixed version? :)
>
> D'oh! I'm sorry, I thought I did. The patch we tested was:
>
> diff -Naur a/dhcp.c b/dhcp.c
> --- a/dhcp.c 2017-10-10 14:01:42.000000000 +0000
> +++ b/dhcp.c 2019-08-14 01:04:58.339325357 +0000
> @@ -590,7 +590,7 @@
> preseed_hostname_from_fqdn(client, buf);
> }
>
> - if (netcfg_get_hostname (client, "netcfg/get_hostname",
> hostname, 1)) {
> + if (netcfg_get_hostname (client, "netcfg/get_hostname",
> hostname, !have_domain)) {
> /*
> * Going back to POLL wouldn't make much sense.
> * However, it does make sense to go to the retry
> diff -Naur a/netcfg-common.c b/netcfg-common.c
> --- a/netcfg-common.c 2017-10-10 14:04:08.000000000 +0000
> +++ b/netcfg-common.c 2019-08-13 20:01:13.606510273 +0000
> @@ -1060,14 +1060,24 @@
> continue;
> }
>
> - if (accept_domain && (s = strchr(hostname, '.'))) {
> - di_info("Detected we have an FQDN; splitting and setting
> domain");
> - if (s[1] == '\0') { /* "somehostname." <- . should be ignored */
> + if ((s = strchr(hostname, '.'))) {
> + di_info("Detected an FQDN in hostname");
> + if (s[1] == '\0') {
> + /* "somehostname." <- . should be ignored */
> *s = '\0';
> - } else { /* assume we have a valid domain name given */
> - strncpy(domain, s + 1, MAXHOSTNAMELEN);
> - debconf_set(client, "netcfg/get_domain", domain);
> - have_domain = 1;
> + di_info("Stripped trailing dot from hostname");
> + } else {
> + /* assume that the domain is valid and copy it if
> + * accept_domain is set; just use the hostname if
> + * it is unset
> + */
> + if (accept_domain) {
> + strncpy(domain, s + 1, MAXHOSTNAMELEN);
> + di_info("Setting domain to %s", domain);
This needs indenting fix-up.
> + debconf_set(client, "netcfg/get_domain", domain);
> + have_domain = 1;
> + }
> + /* strip the domain from the hostname */
> *s = '\0';
> }
> }
>
>> I'm a little reluctant to blindly merging this patch (originally
>> labeled “untested”) without a go from its author. Philipp, should
>> I go ahead?
>
> Totally understood! I just wanted to make sure to revive this issue, as
> I'd also like to get it fixed in Ubuntu! Like I said, I will do my best
> to test and reproduce the fix with stock Debian.
I think this should be fine and we're early in the release cycle to find
potential problems if there are any.
Obviously it'd be great to have a test hardness with a DHCP server
sending various bits and us verifying that netcfg did the right thing.
But I'd surprised to find the time for that myself.
Kind regards and thanks
Philipp Kern
