Fix Syscall.readlink() for non-ascii targets

Syscall.readlink() currently returns an integer indicating the number of
bytes in the link. As buf contains chars, this value is useless if the
target contains non-ascii characters.

This commit creates a new overload which uses a byte array instead of a
StringBuilder and rewrites the old overload to return the number of chars
instead.

Fixes #11778 and #9611

mcs/class/Mono.Management/Mono.Attach/VirtualMachine.cs

@@ -42,20 +42,7 @@ public class VirtualMachine {
 		}
 
 		public string GetWorkingDirectory () {
-			int len = 256;
-
-			while (true) {
-				StringBuilder sb = new StringBuilder (len);
-
-				int res = Syscall.readlink ("/proc/" + pid + "/cwd", sb);
-				if (res == -1)
-					throw new IOException ("Syscall.readlink () failed with error " + res + ".");
-				else if (res == len) {
-					len = len * 2;
-				} else {
-					return sb.ToString ();
-				}
-			}
+			return UnixPath.ReadLink ("/proc/" + pid + "/cwd");
 		}
 
 		/*

mcs/class/Mono.Posix/Mono.Posix_test.dll.sources

@@ -1,3 +1,4 @@
+Mono.Unix/ReadlinkTest.cs
 Mono.Unix/StdioFileStreamTest.cs
 Mono.Unix/UnixEncodingTest.cs
 Mono.Unix/UnixGroupTest.cs

mcs/class/Mono.Posix/Mono.Unix.Native/Syscall.cs

@@ -3937,17 +3937,51 @@ public static bool isatty (int fd)
 				[MarshalAs (UnmanagedType.CustomMarshaler, MarshalTypeRef=typeof(FileNameMarshaler))]
 				string newpath);
 
+		delegate long DoReadlinkFun (byte[] target);
+
+		// Helper function for readlink(string, StringBuilder) and readlinkat (int, string, StringBuilder)
+		static int ReadlinkIntoStringBuilder (DoReadlinkFun doReadlink, [Out] StringBuilder buf, ulong bufsiz)
+		{
+			// bufsiz > int.MaxValue can't work because StringBuilder can store only int.MaxValue chars
+			int bufsizInt = checked ((int) bufsiz);
+			var target = new byte [bufsizInt];
+
+			var r = doReadlink (target);
+			if (r < 0)
+				return checked ((int) r);
+
+			buf.Length = 0;
+			var chars = UnixEncoding.Instance.GetChars (target, 0, checked ((int) r));
+			// Make sure that at more bufsiz chars are written
+			buf.Append (chars, 0, System.Math.Min (bufsizInt, chars.Length));
+			if (r == bufsizInt) {
+				// may not have read full contents; fill 'buf' so that caller can properly check
+				buf.Append (new string ('\x00', bufsizInt - buf.Length));
+			}
+			return buf.Length;
+		}
+
 		// readlink(2)
-		//    int readlink(const char *path, char *buf, size_t bufsize);
+		//    ssize_t readlink(const char *path, char *buf, size_t bufsize);
+		public static int readlink (string path, [Out] StringBuilder buf, ulong bufsiz)
+		{
+			return ReadlinkIntoStringBuilder (target => readlink (path, target), buf, bufsiz);
+		}
+
+		public static int readlink (string path, [Out] StringBuilder buf)
+		{
+			return readlink (path, buf, (ulong) buf.Capacity);
+		}
+
 		[DllImport (MPH, SetLastError=true,
 				EntryPoint="Mono_Posix_Syscall_readlink")]
-		public static extern int readlink (
+		private static extern long readlink (
 				[MarshalAs (UnmanagedType.CustomMarshaler, MarshalTypeRef=typeof(FileNameMarshaler))]
-				string path, [Out] StringBuilder buf, ulong bufsiz);
+				string path, byte[] buf, ulong bufsiz);
 
-		public static int readlink (string path, [Out] StringBuilder buf)
+		public static long readlink (string path, byte[] buf)
 		{
-			return readlink (path, buf, (ulong) buf.Capacity);
+			return readlink (path, buf, (ulong) buf.LongLength);
 		}
 
 		[DllImport (LIBC, SetLastError=true)]
@@ -4226,16 +4260,26 @@ public static int linkat (int olddirfd, string oldpath, int newdirfd, string new
 		}
 
 		// readlinkat(2)
-		//    int readlinkat(int dirfd, const char *pathname, char *buf, size_t bufsize);
+		//    ssize_t readlinkat(int dirfd, const char *pathname, char *buf, size_t bufsize);
+		public static int readlinkat (int dirfd, string pathname, [Out] StringBuilder buf, ulong bufsiz)
+		{
+			return ReadlinkIntoStringBuilder (target => readlinkat (dirfd, pathname, target), buf, bufsiz);
+		}
+
+		public static int readlinkat (int dirfd, string pathname, [Out] StringBuilder buf)
+		{
+			return readlinkat (dirfd, pathname, buf, (ulong) buf.Capacity);
+		}
+
 		[DllImport (MPH, SetLastError=true,
 				EntryPoint="Mono_Posix_Syscall_readlinkat")]
-		public static extern int readlinkat (int dirfd,
+		private static extern long readlinkat (int dirfd,
 				[MarshalAs (UnmanagedType.CustomMarshaler, MarshalTypeRef=typeof(FileNameMarshaler))]
-				string pathname, [Out] StringBuilder buf, ulong bufsiz);
+				string pathname, byte[] buf, ulong bufsiz);
 
-		public static int readlinkat (int dirfd, string pathname, [Out] StringBuilder buf)
+		public static long readlinkat (int dirfd, string pathname, byte[] buf)
 		{
-			return readlinkat (dirfd, pathname, buf, (ulong) buf.Capacity);
+			return readlinkat (dirfd, pathname, buf, (ulong) buf.LongLength);
 		}
 
 		[DllImport (LIBC, SetLastError=true)]

mcs/class/Mono.Posix/Mono.Unix/UnixPath.cs

@@ -209,69 +209,59 @@ public static string GetRealPath (string path)
 
 		// Read the specified symbolic link.  If the file isn't a symbolic link,
 		// return null; otherwise, return the contents of the symbolic link.
-		//
-		// readlink(2) is horribly evil, as there is no way to query how big the
-		// symlink contents are.  Consequently, it's trial and error...
 		internal static string ReadSymbolicLink (string path)
 		{
-			StringBuilder buf = new StringBuilder (256);
+			string target = TryReadLink (path);
+			if (target == null) {
+				Native.Errno errno = Native.Stdlib.GetLastError ();
+				if (errno != Native.Errno.EINVAL)
+					UnixMarshal.ThrowExceptionForError (errno);
+			}
+			return target;
+		}
+
+		public static string TryReadLink (string path)
+		{
+			byte[] buf = new byte[256];
 			do {
-				int r = Native.Syscall.readlink (path, buf);
-				if (r < 0) {
-					Native.Errno e;
-					switch (e = Native.Stdlib.GetLastError()) {
-					case Native.Errno.EINVAL:
-						// path isn't a symbolic link
-						return null;
-					default:
-						UnixMarshal.ThrowExceptionForError (e);
-						break;
-					}
-				}
-				else if (r == buf.Capacity) {
-					buf.Capacity *= 2;
-				}
+				long r = Native.Syscall.readlink (path, buf);
+				if (r < 0)
+					return null;
+				else if (r == buf.Length)
+					buf = new byte[checked (buf.LongLength * 2)];
 				else
-					return buf.ToString (0, r);
+					return UnixEncoding.Instance.GetString (buf, 0, checked ((int) r));
 			} while (true);
 		}
 
-		// Read the specified symbolic link.  If the file isn't a symbolic link,
-		// return null; otherwise, return the contents of the symbolic link.
-		//
-		// readlink(2) is horribly evil, as there is no way to query how big the
-		// symlink contents are.  Consequently, it's trial and error...
-		private static string ReadSymbolicLink (string path, out Native.Errno errno)
+		public static string TryReadLinkAt (int dirfd, string path)
 		{
-			errno = (Native.Errno) 0;
-			StringBuilder buf = new StringBuilder (256);
+			byte[] buf = new byte[256];
 			do {
-				int r = Native.Syscall.readlink (path, buf);
-				if (r < 0) {
-					errno = Native.Stdlib.GetLastError ();
+				long r = Native.Syscall.readlinkat (dirfd, path, buf);
+				if (r < 0)
 					return null;
-				}
-				else if (r == buf.Capacity) {
-					buf.Capacity *= 2;
-				}
+				else if (r == buf.Length)
+					buf = new byte[checked (buf.LongLength * 2)];
 				else
-					return buf.ToString (0, r);
+					return UnixEncoding.Instance.GetString (buf, 0, checked ((int) r));
 			} while (true);
 		}
 
-		public static string TryReadLink (string path)
+		public static string ReadLink (string path)
 		{
-			Native.Errno errno;
-			return ReadSymbolicLink (path, out errno);
+			string target = TryReadLink (path);
+			if (target == null)
+				UnixMarshal.ThrowExceptionForLastError (); 
+			return target;
 		}
 
-		public static string ReadLink (string path)
+		public static string ReadLinkAt (int dirfd, string path)
 		{
-			Native.Errno errno;
-			path = ReadSymbolicLink (path, out errno);
-			if (errno != 0)
-				UnixMarshal.ThrowExceptionForError (errno);
-			return path;
+			string target = TryReadLinkAt (dirfd, path);
+			if (target == null)
+				UnixMarshal.ThrowExceptionForLastError (); 
+			return target;
 		}
 
 		public static bool IsPathRooted (string path)

mcs/class/Mono.Posix/Mono.Unix/UnixSymbolicLinkInfo.cs

@@ -56,19 +56,18 @@ internal UnixSymbolicLinkInfo (string path, Native.Stat stat)
 
 		public string ContentsPath {
 			get {
-				return ReadLink ();
+				return UnixPath.ReadLink (FullPath);
 			}
 		}
 
 		public bool HasContents {
 			get {
-				return TryReadLink () != null;
+				return UnixPath.TryReadLink (FullPath) != null;
 			}
 		}
 
 		public UnixFileSystemInfo GetContents ()
 		{
-			ReadLink ();
 			return UnixFileSystemInfo.GetFileSystemEntry (
 						UnixPath.Combine (UnixPath.GetDirectoryName (FullPath), 
 							ContentsPath));
@@ -103,23 +102,6 @@ protected override bool GetFileStatus (string path, out Native.Stat stat)
 		{
 			return Native.Syscall.lstat (path, out stat) == 0;
 		}
-
-		private string ReadLink ()
-		{
-			string r = TryReadLink ();
-			if (r == null)
-				UnixMarshal.ThrowExceptionForLastError ();
-			return r;
-		}
-
-		private string TryReadLink ()
-		{
-			StringBuilder sb = new StringBuilder ((int) base.Length+1);
-			int r = Native.Syscall.readlink (FullPath, sb);
-			if (r == -1)
-				return null;
-			return sb.ToString (0, r);
-		}
 	}
 }