On Thu, Oct 31, 2019 at 01:05:09PM +0100, Vladimir 'phcoder' Serbinenko wrote: > Having // at the beginning of the path may have special meaning according > to posix. I don't know if it applies in particular case and if the special > meaning is useful for grub to begin with, just something to check
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13 "A pathname consisting of a single <slash> shall resolve to the root directory of the process. A null pathname shall not be successfully resolved. If a pathname begins with two successive <slash> characters, the first component following the leading <slash> characters may be interpreted in an implementation-defined manner, although more than two leading <slash> characters shall be treated as a single <slash> character." I do not see any reason to have "implementation-defined manner" for "//" in the GRUB. So, IMO we should replace any set of consecutive slashes with one slash. > On Thu, 31 Oct 2019, 11:37 Javier Martinez Canillas, <javi...@redhat.com> > wrote: > > > From: Lenny Szubowicz <lszub...@redhat.com> > > > > Some tftp servers do not handle multiple consecutive slashes correctly; > > this patch avoids sending tftp requests with non-normalized paths. > > > > Signed-off-by: Lenny Szubowicz <lszub...@redhat.com> > > Signed-off-by: Mark Salter <msal...@redhat.com> > > Signed-off-by: Javier Martinez Canillas <javi...@redhat.com> > > --- > > > > grub-core/net/tftp.c | 28 +++++++++++++++++++++++++--- > > 1 file changed, 25 insertions(+), 3 deletions(-) > > > > diff --git grub-core/net/tftp.c grub-core/net/tftp.c > > index 7d90bf66e76..6dbb9cdbb7a 100644 > > --- grub-core/net/tftp.c > > +++ grub-core/net/tftp.c > > @@ -300,6 +300,25 @@ destroy_pq (tftp_data_t data) > > grub_priority_queue_destroy (data->pq); > > } > > > > +/* Create a normalized copy of the filename. > > + Compress any string of consecutive forward slashes to a single forward > > + slash. */ https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Multi_002dLine-Comments Please fix this comment... > > +static void > > +grub_normalize_filename (char *normalized, const char *filename) > > +{ > > + char *dest = normalized; > > + const char *src = filename; > > + > > + while (*src != '\0') > > + { > > + if (src[0] == '/' && src[1] == '/') > > + src++; > > + else > > + *dest++ = *src++; > > + } > > + *dest = '\0'; > > +} > > + > > static grub_err_t > > tftp_open (struct grub_file *file, const char *filename) > > { > > @@ -337,9 +356,12 @@ tftp_open (struct grub_file *file, const char > > *filename) > > rrqlen = 0; > > > > tftph->opcode = grub_cpu_to_be16_compile_time (TFTP_RRQ); > > - grub_strcpy (rrq, filename); > > - rrqlen += grub_strlen (filename) + 1; > > - rrq += grub_strlen (filename) + 1; > > + > > + /* Copy and normalize the filename to work-around issues on some tftp > > + servers when file names are being matched for remapping. */ Ditto. > > + grub_normalize_filename (rrq, filename); > > + rrqlen += grub_strlen (rrq) + 1; > > + rrq += grub_strlen (rrq) + 1; If you fix these minor issues feel free to add Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> You can also add above excerpt from POSIX spec to the commit message. This way everybody will know that this is our deliberate decision. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
