[vlc-devel] [PATCH 1/2] dvdnav: Improve user information about insufficient permissions

[Alexandre Janniaux]

Hi,

I would probably put #ifdef around DVDProbeMacOSPermission
instead of inside as the function seems to depicts a MacOS
specificity.

Otherwise LGTM.

Regards,
--
Alexandre Janniaux
Videolabs


On Mon, May 04, 2020 at 08:09:13PM +0200, david.fuhrmann at gmail.com wrote:
> From: David Fuhrmann <dfuhrmann at videolan.org>
>
> The mentioned security setting is only relevant for accessing
> RAW block devices (/dev/xxx), which is one of the main use cases
> for this module (accessing an optical drive).
> It is not relevant for file or folder access.
>
> Probe for this case explicitly to inform the user only there.
>
> This is only relevant starting with macOS Catalina.
> ---
>  modules/access/Makefile.am  |  2 +-
>  modules/access/dvd_helper.h | 57 +++++++++++++++++++++++++++++++++++++
>  modules/access/dvdnav.c     | 19 ++++++++-----
>  3 files changed, 70 insertions(+), 8 deletions(-)
>  create mode 100644 modules/access/dvd_helper.h
>
> diff --git a/modules/access/Makefile.am b/modules/access/Makefile.am
> index b73fda5f4c..265683c3f8 100644
> --- a/modules/access/Makefile.am
> +++ b/modules/access/Makefile.am
> @@ -242,7 +242,7 @@ endif
>  EXTRA_LTLIBRARIES += libvcd_plugin.la
>  access_LTLIBRARIES += $(LTLIBvcd)
>
> -libdvdnav_plugin_la_SOURCES = access/dvdnav.c demux/mpeg/ps.h demux/mpeg/pes.h
> +libdvdnav_plugin_la_SOURCES = access/dvd_helper.h access/dvdnav.c demux/mpeg/ps.h demux/mpeg/pes.h
>  libdvdnav_plugin_la_CFLAGS = $(AM_CFLAGS) $(DVDNAV_CFLAGS)
>  libdvdnav_plugin_la_LIBADD = $(DVDNAV_LIBS)
>  libdvdnav_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(accessdir)'
> diff --git a/modules/access/dvd_helper.h b/modules/access/dvd_helper.h
> new file mode 100644
> index 0000000000..403b44c542
> --- /dev/null
> +++ b/modules/access/dvd_helper.h
> @@ -0,0 +1,57 @@
> +/*****************************************************************************
> + * dvd_helper.h: dvdnav / dvdread helper functions
> + *****************************************************************************
> + * Copyright (C) 2020 VLC authors and VideoLAN
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> + *****************************************************************************/
> +
> +#include <sys/stat.h>
> +#include <unistd.h>
> +
> +#include <vlc_fs.h>
> +
> +
> +inline static int DVDProbeMacOSPermission( const char *psz_file )
> +{
> +#ifdef __APPLE__
> +    /* Check is only relevant starting macOS Catalina */
> +    if( __builtin_available( macOS 10.15, * ) )
> +    {
> +        /* Continue. The check above cannot be negated. */
> +    }
> +    else
> +    {
> +        return VLC_SUCCESS;
> +    }
> +
> +    /* This is only relevant when accessing RAW block devices */
> +    struct stat stat_buf;
> +    if( vlc_stat( psz_file, &stat_buf ) != 0 )
> +        return VLC_SUCCESS; // Continue with probing to be on the safe side
> +
> +    if( !S_ISBLK( stat_buf.st_mode ) && !S_ISCHR( stat_buf.st_mode ) )
> +        return VLC_SUCCESS;
> +
> +    /* Check that device access in fact fails with EPERM error */
> +    int retVal = access( psz_file, R_OK );
> +    if( retVal == -1 && errno == EPERM )
> +        return VLC_EGENERIC;
> +
> +    return VLC_SUCCESS;
> +#else
> +    return VLC_SUCCESS;
> +#endif
> +}
> diff --git a/modules/access/dvdnav.c b/modules/access/dvdnav.c
> index f082f1021b..a62950c605 100644
> --- a/modules/access/dvdnav.c
> +++ b/modules/access/dvdnav.c
> @@ -65,6 +65,8 @@ dvdnav_status_t dvdnav_jump_to_sector_by_time(dvdnav_t *, uint64_t, int32_t);
>  #include "../demux/mpeg/ps.h"
>  #include "../demux/timestamps_filter.h"
>
> +#include "dvd_helper.h"
> +
>  /*****************************************************************************
>   * Module descriptor
>   *****************************************************************************/
> @@ -364,6 +366,16 @@ static int AccessDemuxOpen ( vlc_object_t *p_this )
>      if( !forced && ProbeDVD( psz_file ) != VLC_SUCCESS )
>          goto bailout;
>
> +    if( forced && DVDProbeMacOSPermission( psz_file ) != VLC_SUCCESS )
> +    {
> +        msg_Err( p_demux, "Path %s cannot be opened due to unsufficient permissions", psz_file );
> +        vlc_dialog_display_error( p_demux, _("Problem accessing a system resource"),
> +                                 _("Potentially, macOS blocks access to your disc. "
> +                                   "Please open \"System Preferences\" -> \"Security & Privacy\" "
> +                                   "and allow VLC to access your external media in \"Files and Folders\" section."));
> +        goto bailout;
> +    }
> +
>      /* Open dvdnav */
>      psz_path = ToLocale( psz_file );
>  #if DVDNAV_VERSION >= 60100
> @@ -375,13 +387,6 @@ static int AccessDemuxOpen ( vlc_object_t *p_this )
>  #endif
>      {
>          msg_Warn( p_demux, "cannot open DVD (%s)", psz_file);
> -
> -#ifdef __APPLE__
> -        vlc_dialog_display_error( p_demux, _("Problem accessing a system resource"),
> -            _("Potentially, macOS blocks access to your disc. "
> -              "Please open \"System Preferences\" -> \"Security & Privacy\" "
> -              "and allow VLC to access your external media in \"Files and Folders\" section."));
> -#endif
>          goto bailout;
>      }
>
> --
> 2.21.1 (Apple Git-122.3)
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel