From 912a9675c0b7f9e8c836983e525b180c48693925 Mon Sep 17 00:00:00 2001
From: Aaron Turon <aturon@mozilla.com>
Date: Wed, 14 May 2014 14:39:16 -0700
Subject: [PATCH] Make `from_bits` in `bitflags!` safe; add
 `from_bits_truncate`

Previously, the `from_bits` function in the `std::bitflags::bitflags`
macro was marked as unsafe, as it did not check that the bits being
converted actually corresponded to flags.

This patch changes the function to check against the full set of
possible flags and return an `Option` which is `None` if a non-flag bit
is set. It also adds a `from_bits_truncate` function which simply zeros
any bits not corresponding to a flag.

This addresses the concern raised in https://github.com/mozilla/rust/pull/13897
---
 src/libnative/io/file_unix.rs  |  4 +---
 src/libnative/io/file_win32.rs |  4 +---
 src/librustuv/file.rs          |  4 +---
 src/libstd/bitflags.rs         | 37 +++++++++++++++++++++++++++-------
 4 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/src/libnative/io/file_unix.rs b/src/libnative/io/file_unix.rs
index c2b69483fa1..046d2875d55 100644
--- a/src/libnative/io/file_unix.rs
+++ b/src/libnative/io/file_unix.rs
@@ -493,9 +493,7 @@ fn mkstat(stat: &libc::stat) -> io::FileStat {
     io::FileStat {
         size: stat.st_size as u64,
         kind: kind,
-        perm: unsafe {
-            io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions
-        },
+        perm: io::FilePermission::from_bits_truncate(stat.st_mode as u32),
         created: mktime(stat.st_ctime as u64, stat.st_ctime_nsec as u64),
         modified: mktime(stat.st_mtime as u64, stat.st_mtime_nsec as u64),
         accessed: mktime(stat.st_atime as u64, stat.st_atime_nsec as u64),
diff --git a/src/libnative/io/file_win32.rs b/src/libnative/io/file_win32.rs
index d721e1d67f1..3222c912dd0 100644
--- a/src/libnative/io/file_win32.rs
+++ b/src/libnative/io/file_win32.rs
@@ -492,9 +492,7 @@ fn mkstat(stat: &libc::stat) -> io::FileStat {
     io::FileStat {
         size: stat.st_size as u64,
         kind: kind,
-        perm: unsafe {
-          io::FilePermission::from_bits(stat.st_mode as u32)  & io::AllPermissions
-        },
+        perm: io::FilePermission::from_bits_truncate(stat.st_mode as u32),
         created: stat.st_ctime as u64,
         modified: stat.st_mtime as u64,
         accessed: stat.st_atime as u64,
diff --git a/src/librustuv/file.rs b/src/librustuv/file.rs
index 06271e61ce7..12636a3c490 100644
--- a/src/librustuv/file.rs
+++ b/src/librustuv/file.rs
@@ -285,9 +285,7 @@ impl FsRequest {
         FileStat {
             size: stat.st_size as u64,
             kind: kind,
-            perm: unsafe {
-                io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions
-            },
+            perm: io::FilePermission::from_bits_truncate(stat.st_mode as u32),
             created: to_msec(stat.st_birthtim),
             modified: to_msec(stat.st_mtim),
             accessed: to_msec(stat.st_atim),
diff --git a/src/libstd/bitflags.rs b/src/libstd/bitflags.rs
index 32f9bc1173b..163ccd22552 100644
--- a/src/libstd/bitflags.rs
+++ b/src/libstd/bitflags.rs
@@ -136,10 +136,20 @@ macro_rules! bitflags(
                 self.bits
             }
 
-            /// Convert from underlying bit representation. Unsafe because the
-            /// bits are not guaranteed to represent valid flags.
-            pub unsafe fn from_bits(bits: $T) -> $BitFlags {
-                $BitFlags { bits: bits }
+            /// Convert from underlying bit representation, unless that
+            /// representation contains bits that do not correspond to a flag.
+            pub fn from_bits(bits: $T) -> ::std::option::Option<$BitFlags> {
+                if (bits & !$BitFlags::all().bits()) != 0 {
+                    ::std::option::None
+                } else {
+                    ::std::option::Some($BitFlags { bits: bits })
+                }
+            }
+
+            /// Convert from underlying bit representation, dropping any bits
+            /// that do not correspond to flags.
+            pub fn from_bits_truncate(bits: $T) -> $BitFlags {
+                $BitFlags { bits: bits } & $BitFlags::all()
             }
 
             /// Returns `true` if no flags are currently stored.
@@ -209,6 +219,7 @@ macro_rules! bitflags(
 
 #[cfg(test)]
 mod tests {
+    use option::{Some, None};
     use ops::{BitOr, BitAnd, Sub, Not};
 
     bitflags!(
@@ -231,9 +242,21 @@ mod tests {
 
     #[test]
     fn test_from_bits() {
-        assert!(unsafe { Flags::from_bits(0x00000000) } == Flags::empty());
-        assert!(unsafe { Flags::from_bits(0x00000001) } == FlagA);
-        assert!(unsafe { Flags::from_bits(0x00000111) } == FlagABC);
+        assert!(Flags::from_bits(0) == Some(Flags::empty()));
+        assert!(Flags::from_bits(0x1) == Some(FlagA));
+        assert!(Flags::from_bits(0x10) == Some(FlagB));
+        assert!(Flags::from_bits(0x11) == Some(FlagA | FlagB));
+        assert!(Flags::from_bits(0x1000) == None);
+    }
+
+    #[test]
+    fn test_from_bits_truncate() {
+        assert!(Flags::from_bits_truncate(0) == Flags::empty());
+        assert!(Flags::from_bits_truncate(0x1) == FlagA);
+        assert!(Flags::from_bits_truncate(0x10) == FlagB);
+        assert!(Flags::from_bits_truncate(0x11) == (FlagA | FlagB));
+        assert!(Flags::from_bits_truncate(0x1000) == Flags::empty());
+        assert!(Flags::from_bits_truncate(0x1001) == FlagA);
     }
 
     #[test]