mailing list of musl libc
 help / color / mirror / Atom feed
* [musl] [PATCH] make realpath replace leading // with a single /
@ 2021-01-13 13:28 Natanael Copa
  2021-01-13 13:38 ` Natanael Copa
  0 siblings, 1 reply; 3+ messages in thread
From: Natanael Copa @ 2021-01-13 13:28 UTC (permalink / raw)
  To: musl; +Cc: Natanael Copa

On some systems a leading double slash may have special meaning, so
POSIX[1] says that "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"

While current musl implementation is technically correct, most other
systems' (at least GNU libc, freebsd, openbsd, netbsd macOS)
implementations will replace a leading // with a single /. Make musl
do the same to avoid surprises.

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13
---
 src/misc/realpath.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/misc/realpath.c b/src/misc/realpath.c
index db8b74dc..414b4741 100644
--- a/src/misc/realpath.c
+++ b/src/misc/realpath.c
@@ -46,9 +46,6 @@ restart:
 			q=0;
 			output[q++] = '/';
 			p++;
-			/* Initial // is special. */
-			if (stack[p] == '/' && stack[p+1] != '/')
-				output[q++] = '/';
 			continue;
 		}
 
-- 
2.30.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] [PATCH] make realpath replace leading // with a single /
  2021-01-13 13:28 [musl] [PATCH] make realpath replace leading // with a single / Natanael Copa
@ 2021-01-13 13:38 ` Natanael Copa
  2021-01-13 18:01   ` Rich Felker
  0 siblings, 1 reply; 3+ messages in thread
From: Natanael Copa @ 2021-01-13 13:38 UTC (permalink / raw)
  To: musl

On Wed, 13 Jan 2021 14:28:35 +0100
Natanael Copa <ncopa@alpinelinux.org> wrote:

> On some systems a leading double slash may have special meaning, so
> POSIX[1] says that "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"
> 
> While current musl implementation is technically correct, most other
> systems' (at least GNU libc, freebsd, openbsd, netbsd macOS)
> implementations will replace a leading // with a single /. Make musl
> do the same to avoid surprises.
> 
> [1]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13
> ---
>  src/misc/realpath.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/src/misc/realpath.c b/src/misc/realpath.c
> index db8b74dc..414b4741 100644
> --- a/src/misc/realpath.c
> +++ b/src/misc/realpath.c
> @@ -46,9 +46,6 @@ restart:
>  			q=0;
>  			output[q++] = '/';
>  			p++;
> -			/* Initial // is special. */
> -			if (stack[p] == '/' && stack[p+1] != '/')
> -				output[q++] = '/';
>  			continue;
>  		}
>  

This fixes gettext's (gnulib) testsuite, which tests if realpath("//",
NULL) return "/" and fails if it doesn't.

I ran this testcase on multiple systems:

#include <stdio.h>
#include <stdlib.h>

int main() {
	printf("%s\n", realpath("//", NULL));
	return 0;
}



musl 1.1.24:	/
ubuntu 20.03:	/
macOS Big Sur:	/
OpenBSD 6.8:	/
FreeBSD 12.2:	/
NetBSD 9.1:	/
musl (current)	//

I don't know why this behavior was introduced in musl, but I think
this only adds meaningless friction to downstream users.

-nc

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] [PATCH] make realpath replace leading // with a single /
  2021-01-13 13:38 ` Natanael Copa
@ 2021-01-13 18:01   ` Rich Felker
  0 siblings, 0 replies; 3+ messages in thread
From: Rich Felker @ 2021-01-13 18:01 UTC (permalink / raw)
  To: Natanael Copa; +Cc: musl

On Wed, Jan 13, 2021 at 02:38:09PM +0100, Natanael Copa wrote:
> On Wed, 13 Jan 2021 14:28:35 +0100
> Natanael Copa <ncopa@alpinelinux.org> wrote:
> 
> > On some systems a leading double slash may have special meaning, so
> > POSIX[1] says that "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"
> > 
> > While current musl implementation is technically correct, most other
> > systems' (at least GNU libc, freebsd, openbsd, netbsd macOS)
> > implementations will replace a leading // with a single /. Make musl
> > do the same to avoid surprises.
> > 
> > [1]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13
> > ---
> >  src/misc/realpath.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/src/misc/realpath.c b/src/misc/realpath.c
> > index db8b74dc..414b4741 100644
> > --- a/src/misc/realpath.c
> > +++ b/src/misc/realpath.c
> > @@ -46,9 +46,6 @@ restart:
> >  			q=0;
> >  			output[q++] = '/';
> >  			p++;
> > -			/* Initial // is special. */
> > -			if (stack[p] == '/' && stack[p+1] != '/')
> > -				output[q++] = '/';
> >  			continue;
> >  		}
> >  
> 
> This fixes gettext's (gnulib) testsuite, which tests if realpath("//",
> NULL) return "/" and fails if it doesn't.
> 
> I ran this testcase on multiple systems:
> 
> #include <stdio.h>
> #include <stdlib.h>
> 
> int main() {
> 	printf("%s\n", realpath("//", NULL));
> 	return 0;
> }
> 
> 
> 
> musl 1.1.24:	/
> ubuntu 20.03:	/
> macOS Big Sur:	/
> OpenBSD 6.8:	/
> FreeBSD 12.2:	/
> NetBSD 9.1:	/
> musl (current)	//
> 
> I don't know why this behavior was introduced in musl, but I think
> this only adds meaningless friction to downstream users.

It wasn't so much introduced new as written to avoid hard-coding a
non-portable interpretation of pathnames into binaries. Previously, we
relied on the kernel's path normalization as displayed in /proc/*/fd/*
symlink contents to do the normalization, and it was able to convert
leading // to / on the basis that it (the kernel) does not treat them
differently. However POSIX specifies / and // as potentially being
different (with /// and beyond all being equivalent to /, though), and
if musl's userspace realpath resolution made the normalization of //
to /, it would be encoding the present-day-Linux-specific semantic
assumption. I did not want to do that.

POSIX reserves // presumably to allow things like network shares,
abstract volume references, drive letters, etc. not explcitly mounted
into the filesystem but referenced via a user-provided pathname. At
one point midipix was planning to use this for Windows drive letters,
but I think they hit too much friction with applications doing their
own pathname normalization that broke it (converting the // to /) and
dropped it. Part of my motive in preserving compatibility with kernel
implementations that use // specially was aimed at midipix (I wasn't
yet aware they'd dropped it) but I was also thinking about the ability
to run musl binaries on non-Linux kernels where pathnames should be
interpreted according to the host filesystem policy, not a hard-coded
assumption that Linux and other currently popular systems don't use //
for anything.

It would perhaps be nice to detect at realpath time when //foo and
/foo are the same inode and drop the extra / in that case, but that
requires pulling in stat, which is large nowadays thanks to the need
for translation of kernel structures. It might be worth doing anyway
if this ends up being a problem, but from my understanding on IRC
earlier the breakage you hit was just a testsuite asserting something
invalid, and nothing was actually broken at build or run time.

Rich

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-13 18:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 13:28 [musl] [PATCH] make realpath replace leading // with a single / Natanael Copa
2021-01-13 13:38 ` Natanael Copa
2021-01-13 18:01   ` Rich Felker

mailing list of musl libc

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/musl

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 musl musl/ http://inbox.vuxu.org/musl \
		musl@inbox.vuxu.org
	public-inbox-index musl

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.musl


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git