Code review comment for lp:~jtv/maas/bug-1042877

Thanks for the reviews.

> [1]
>
> -  # root: /var/lib/tftpboot
> +  root: /var/lib/maas/tftp
>
> The approach I've been taking with this file is to include the default
> value, commented out. This is the expectation that Andres has now. So,
> the value change here is fine, but it should still be commented. I see
> that you've updated the ConfigTFTP.root default.

Okay: I commented this one out because it matches the (new) default.

> [2]
>
> In tftp.py:
>
> -        ^/?
> ...
> +        ^/*
>
> I think this should stay as zero-or-one rather than zero-or-more. The
> latter speaks of slapdashery. The zero-or-one is there to accomodate
> ambiguity in the TFTP spec, iirc.

On the other hand, you'd normally expect multiple consecutive slashes to be treated as a single slash. If not to satisfy general Unix sensibilities, then because it's what the standard Ubuntu tftpd does.

I agree that multiple slashes would be a bit suspicious, but when it comes to debugging, wouldn't a failure to find e.g. //pxelinux.0 be a massive red herring when it comes to debugging? I could imagine circumstances where it would be convenient to risk including an extra slash, and if that means that we end up normalizing slashes so as not to displease our own server, then all we've gained is some extra complexity to compensate for our own unwarranted strictness. Bear in mind that we want users to edit the templates that these paths occur in: we don't want to frustrate them with unexpected breakage. Be strict in what you generate and liberal in what you accept.

Conversely, in a case where a double leading slash is actually a sign of something wrong, you'd expect a file-not-found anyway. We don't even have items of the same name occurring at different depths in the tree.