From a0e09526b3851602ff9e9bc6c12e34d5f77db125 Mon Sep 17 00:00:00 2001
From: Oleg Kozlov <okozlov@redhat.com>
Date: Dec 07 2018 13:06:29 +0000
Subject: Check pager's executable before subprocess.Popen


Get the value of `PAGER` environment variable in case it's defined, check the executable, if it exists - use a pager, otherwise - print function.

Fixes: https://pagure.io/freeipa/issue/7746
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>

---

diff --git a/ipalib/cli.py b/ipalib/cli.py
index a36fb2a..5d10aac 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -49,7 +49,7 @@ import six
 from six.moves import input
 
 from ipalib.util import (
-    check_client_configuration, get_terminal_height, open_in_pager
+    check_client_configuration, get_pager, get_terminal_height, open_in_pager
 )
 
 if six.PY3:
@@ -721,9 +721,11 @@ class help(frontend.Local):
             self.buffer.append(unicode(string))
 
         def write(self):
-            if self.buffer_length > get_terminal_height():
+            pager = get_pager()
+
+            if pager and self.buffer_length > get_terminal_height():
                 data = "\n".join(self.buffer).encode("utf-8")
-                open_in_pager(data)
+                open_in_pager(data, pager)
             else:
                 try:
                     for line in self.buffer:
diff --git a/ipalib/util.py b/ipalib/util.py
index 6600ba2..e684145 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -37,6 +37,7 @@ import sys
 import ssl
 import termios
 import fcntl
+import shutil
 import struct
 import subprocess
 
@@ -1203,17 +1204,27 @@ def get_terminal_height(fd=1):
         return os.environ.get("LINES", 25)
 
 
-def open_in_pager(data):
+def get_pager():
+    """ Get path to a pager
+
+    :return: path to the file if it exists otherwise None
+    :rtype: str or None
+    """
+    pager = os.environ.get('PAGER', 'less')
+    return shutil.which(pager)
+
+
+def open_in_pager(data, pager):
     """
     Open text data in pager
 
     Args:
         data (bytes): data to view in pager
+        pager (str): path to the pager
 
     Returns:
         None
     """
-    pager = os.environ.get("PAGER", "less")
     pager_process = subprocess.Popen([pager], stdin=subprocess.PIPE)
 
     try:
diff --git a/ipatests/test_ipalib/test_util.py b/ipatests/test_ipalib/test_util.py
new file mode 100644
index 0000000..9e755d7
--- /dev/null
+++ b/ipatests/test_ipalib/test_util.py
@@ -0,0 +1,25 @@
+#
+# Copyright (C) 2018  FreeIPA Contributors see COPYING for license
+#
+"""Tests for ipalib.util module
+"""
+
+import os
+from unittest import mock
+
+import pytest
+
+from ipalib.util import get_pager
+
+
+@pytest.mark.parametrize('pager,expected_result', [
+    # Valid values
+    ('cat', '/usr/bin/cat'),
+    ('/usr/bin/cat', '/usr/bin/cat'),
+    # Invalid values (wrong command, package is not installed, etc)
+    ('cat_', None),
+    ('', None)
+])
+def test_get_pager(pager, expected_result):
+    with mock.patch.dict(os.environ, {'PAGER': pager}):
+        assert get_pager() == expected_result