build: Monkeypatch subprocess on Windows to find .CMD scripts

Without this patch, subprocess fails to find .CMD scripts on Windows,
even though these are executables and should be treated as such
according to the PATHEXT environment variable.

Many people "work around" this issue on Windows with subprocess's
shell=True argument, but this comes with a risk of shell injection
vulnerabilities.

Another solution is to explicitly add ".CMD" to the end of some
commands on Windows, but this breaks portability and requires "if
windows" to be scattered around a codebase.  We previously took this
approach in Shaka Player.

This monkeypatch allows the caller of subprocess to stop worrying
about Windows nuances and to go back to the security best practice of
shell=False.  Any .CMD script that would be found by the Windows shell
will now be found by subprocess.  And because we're using the standard
Windows PATHEXT environment variable, this can be extended to other
types of executable scripts, as well.

This monkeypatch can be used in any Python project that has this
issue, and merely has to be imported to function.  It has been
verified in Python 3.8, both in the Cygwin version and in the native
Windows version of Python.

Change-Id: I37bb522fbf5f058431a209c73508bd225052999a

build/shakaBuildHelpers.py

@@ -32,6 +32,9 @@
 import sys
 import time
 
+import subprocessWindowsPatch
+
+
 # Python 3 no longer has a separate unicode type.  For type-checking done in
 # get_node_binary, create an alias to the str type.
 if sys.version_info[0] == 3:
@@ -212,8 +215,7 @@ def npm_version(is_dirty=False):
   """Gets the version of the library from NPM."""
   try:
     base = cygwin_safe_path(get_source_base())
-    cmd = 'npm.cmd' if is_windows() else 'npm'
-    cmd_line = [cmd, '--prefix', base, 'ls', 'shaka-player']
+    cmd_line = ['npm', '--prefix', base, 'ls', 'shaka-player']
     text = execute_get_output(cmd_line).decode('utf8')
   except subprocess.CalledProcessError as e:
     text = e.output.decode('utf8')
@@ -335,10 +337,9 @@ def update_node_modules():
     return True
 
   base = cygwin_safe_path(get_source_base())
-  cmd = 'npm.cmd' if is_windows() else 'npm'
 
   # Check the version of npm.
-  version = execute_get_output([cmd, '-v']).decode('utf8')
+  version = execute_get_output(['npm', '-v']).decode('utf8')
 
   if _parse_version(version) < _parse_version('5.0.0'):
     logging.error('npm version is too old, please upgrade.  e.g.:')
@@ -351,7 +352,7 @@ def update_node_modules():
   with InDir(base):
     # npm update seems to be the wrong thing in npm v5, so use install.
     # See google/shaka-player#854 for more details.
-    execute_get_output([cmd, 'install'])
+    execute_get_output(['npm', 'install'])
 
   # Update the timestamp of the file that tracks when we last updated.
   open(_node_modules_last_update_path(), 'wb').close()

build/subprocessWindowsPatch.py

@@ -0,0 +1,103 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Monkeypatch subprocess on Windows to find .CMD scripts.
+
+Without this patch, subprocess fails to find .CMD scripts on Windows, even
+though these are executables and should be treated as such according to the
+PATHEXT environment variable.
+
+Many people "work around" this issue on Windows with subprocess's shell=True
+argument, but this comes with a risk of shell injection vulnerabilities.
+
+Another solution is to explicitly add ".CMD" to the end of some commands on
+Windows, but this breaks portability and requires "if windows" to be scattered
+around a codebase.
+
+This monkeypatch allows the caller of subprocess to stop worrying about Windows
+nuances and to go back to the security best practice of shell=False.  Any .CMD
+script that would be found by the Windows shell will now be found by
+subprocess.  And because we're using the standard Windows PATHEXT environment
+variable, this can be extended to other types of executable scripts, as well.
+"""
+
+import os
+import subprocess
+import sys
+
+
+# NOTE: All of the higher-level methods eventually delegate to Popen, so we
+# only patch that one method.
+#   run => Popen
+#   call => Popen
+#   check_call => call => Popen
+#   check_output => run => Popen
+
+# These environment variables should almost certainly exist, but these are
+# defaults in case they are missing.
+DEFAULT_PATHEXT = '.COM;.EXE;.BAT;.CMD'
+DEFAULT_PATH = r'C:\WINDOWS\system32;C:\WINDOWS'
+
+
+def resolve(exe):
+  """Resolve a command name into a full path to the executable."""
+
+  if '/' in exe or '\\' in exe or ':' in exe:
+    # This is a path, so don't modify it.
+    return exe
+
+  if '.' in exe:
+    # This has an extension already.  Don't search for an extension.
+    exe_names = [exe]
+  else:
+    # This is a command name without an extension, so check for every extension
+    # in PATHEXT.
+    extensions = os.environ.get('PATHEXT', DEFAULT_PATHEXT).split(';')
+    exe_names = [exe + ext for ext in extensions]
+
+  exe_paths = os.environ.get('PATH', DEFAULT_PATH).split(';')
+
+  for path in exe_paths:
+    for name in exe_names:
+      candidate = os.path.join(path, name)
+      if os.access(candidate, os.X_OK):  # If executable
+        return candidate
+
+  # Failed to resolve, so return the original name and let Popen fail with a
+  # natural-looking error complaining that this command cannot be found.
+  return exe
+
+
+real_Popen = subprocess.Popen
+
+
+def Popen(args, *more_args, **kwargs):
+  """A patch to install over subprocess.Popen."""
+
+  # If the first argument is a list, resolve the command name, which is the
+  # first item in the list.
+  if isinstance(args, list):
+    args[0] = resolve(args[0])
+
+  # Delegate to the real Popen implementation.
+  return real_Popen(args, *more_args, **kwargs)
+
+
+# Only patch win32, but not cygwin.  Cygwin works correctly already.
+if sys.platform == 'win32':
+  # Patch over Popen.
+  subprocess.Popen = Popen
+  # Copy the docstring from the real Popen into the patch, so that
+  # help(subprocess.Popen) is still relatively sane with this patch installed.
+  Popen.__doc__ = real_Popen.__doc__