----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19093/#review37012 -----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp <https://reviews.apache.org/r/19093/#comment68265> How about introducing a new Permissions struct here? struct Permissions { explicit Permissions(mode_t mode) { owner.readable = mode & magic_to_determine_if_mode_is_readable; owner.writable = mode & magic_to_determine_if_mode_is_writable; owner.executable = mode & magic_to_determine_if_mode_is_executable; ... } struct { bool readable; bool writable; bool executable; } owner; struct { bool readable; bool writable; bool executable; } group; struct { bool readable; bool writable; bool executable; } others; bool setuid; bool setgid; }; Then in code you could do: Try<os::Permissions> permissions = os::permissions(path); if (!permissions.isError()) { if (permissions.get().others.readable || permissions.get().others.writable || permissions.get().others.executable) { LOG(WARNING) << "Permissions on credentials file are insufficient!"; } } The slight downside here is that we'll be spending some CPU cycles constructing a Permissions object, but that's highly unlikely to be a performance issue. The upside here is the code becomes very explicit and therefore maintainable! src/master/master.cpp <https://reviews.apache.org/r/19093/#comment68258> How about: const string& path = strings::remove(flags.credentials.get(), "file://", strings::PREFIX); All uses of 'path' are fine without the "file://" prefix. src/master/master.cpp <https://reviews.apache.org/r/19093/#comment68257> Let's use LOG(WARNING) here since this is just advisory. - Benjamin Hindman On March 12, 2014, 11:07 a.m., Isabel Jimenez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19093/ > ----------------------------------------------------------- > > (Updated March 12, 2014, 11:07 a.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Bugs: MESOS-1087 > https://issues.apache.org/jira/browse/MESOS-1087 > > > Repository: mesos-git > > > Description > ------- > > Adding permissions check on credentials file and deleting TODO comment > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/Makefile.am 1075b46 > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 3f475a4 > 3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp > PRE-CREATION > src/master/master.cpp 2a40333 > > Diff: https://reviews.apache.org/r/19093/diff/ > > > Testing > ------- > > > Thanks, > > Isabel Jimenez > >
