Fixes and improvements suggested by reviewer

Author: zooba

src/manage/aliasutils.py

@@ -66,6 +66,14 @@ def replace(self, **kwargs):
     @property
     def script_code(self):
         if self.mod and self.func:
+            if not self.mod.isidentifier():
+                LOGGER.warn("Alias %s has an entrypoint with invalid module "
+                            "%r.", self.name, self.mod)
+                return None
+            if not self.func.isidentifier():
+                LOGGER.warn("Alias %s has an entrypoint with invalid function "
+                            "%r.", self.name, self.func)
+                return None
             return SCRIPT_CODE.format(mod=self.mod, func=self.func)
 
 
@@ -183,7 +191,8 @@ def _create_alias(cmd, *, name, target, plat=None, windowed=0, script_code=None,
         except OSError:
             LOGGER.error("Failed to clean up existing alias. Re-run with -v "
                          "or check the install log for details.")
-            LOGGER.info("Failed to remove %s.", p_script, exc_info=True)
+            LOGGER.info("Failed to remove %s.", p_script)
+            LOGGER.debug("TRACEBACK", exc_info=True)
 
 
 def _parse_entrypoint_line(line):

src/pymanager/_launch.cpp

@@ -47,35 +47,35 @@ launch(
     STARTUPINFOW si;
     PROCESS_INFORMATION pi;
     int lastError = 0;
-    const wchar_t *cmdLine = NULL;
+    const wchar_t *cmd_line = NULL;
 
     if (orig_cmd_line[0] == L'"') {
-        cmdLine = wcschr(orig_cmd_line + 1, L'"');
+        cmd_line = wcschr(orig_cmd_line + 1, L'"');
     } else {
-        cmdLine = wcschr(orig_cmd_line, L' ');
+        cmd_line = wcschr(orig_cmd_line, L' ');
     }
 
-    size_t n = wcslen(executable) + wcslen(orig_cmd_line) + wcslen(insert_args) + 6;
-    wchar_t *newCmdLine = (wchar_t *)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, n * sizeof(wchar_t));
-    if (!newCmdLine) {
+    size_t n = wcslen(executable) + wcslen(orig_cmd_line) + (insert_args ? wcslen(insert_args) : 0) + 6;
+    wchar_t *new_cmd_line = (wchar_t *)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, n * sizeof(wchar_t));
+    if (!new_cmd_line) {
         lastError = GetLastError();
         goto exit;
     }
 
     // Skip any requested args, deliberately leaving any trailing spaces
-    // (we'll skip one later one and add our own space, and preserve multiple)
-    while (skip_argc-- > 0) {
+    // (we'll skip one later on and add our own space, and preserve multiple)
+    while (cmd_line && *cmd_line && skip_argc-- > 0) {
         wchar_t c;
-        while (*++cmdLine && *cmdLine == L' ') { }
-        while (*++cmdLine && *cmdLine != L' ') { }
+        while (*++cmd_line && *cmd_line == L' ') { }
+        while (*++cmd_line && *cmd_line != L' ') { }
     }
 
-    swprintf_s(newCmdLine, n, L"\"%s\"%s%s%s%s",
+    swprintf_s(new_cmd_line, n, L"\"%s\"%s%s%s%s",
                executable,
                (insert_args && *insert_args) ? L" ": L"",
                (insert_args && *insert_args) ? insert_args : L"",
-               (cmdLine && *cmdLine) ? L" " : L"",
-               (cmdLine && *cmdLine) ? cmdLine + 1 : L"");
+               (cmd_line && *cmd_line) ? L" " : L"",
+               (cmd_line && *cmd_line) ? cmd_line + 1 : L"");
 
 #if defined(_WINDOWS)
     /*
@@ -122,7 +122,7 @@ launch(
     }
 
     si.dwFlags |= STARTF_USESTDHANDLES;
-    if (!CreateProcessW(executable, newCmdLine, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) {
+    if (!CreateProcessW(executable, new_cmd_line, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) {
         lastError = GetLastError();
         goto exit;
     }
@@ -134,8 +134,8 @@ launch(
         lastError = GetLastError();
     }
 exit:
-    if (newCmdLine) {
-        HeapFree(GetProcessHeap(), 0, newCmdLine);
+    if (new_cmd_line) {
+        HeapFree(GetProcessHeap(), 0, new_cmd_line);
     }
     return lastError ? HRESULT_FROM_WIN32(lastError) : 0;
 }

src/pymanager/launcher.cpp

@@ -172,7 +172,7 @@ get_script(wchar_t **result_path)
         }
     }
 
-    wcscpy_s(&path[len], path_len, SUFFIX);
+    wcscpy_s(&path[len], path_len - len, SUFFIX);
 
     // Check that we have a script file. FindFirstFile should be fastest.
     WIN32_FIND_DATAW fd;
@@ -267,7 +267,7 @@ wmain(int argc, wchar_t **argv)
 {
     int exit_code;
     wchar_t executable[MAXLEN];
-    wchar_t *script;
+    wchar_t *script = NULL;
 
     int err = get_executable(executable, MAXLEN);
     if (err) {

tests/test_alias.py

@@ -1,8 +1,5 @@
-import json
-import os
 import pytest
 import secrets
-from pathlib import Path, PurePath
 
 from manage import aliasutils as AU
 from manage.exceptions import NoLauncherTemplateError
@@ -298,10 +295,6 @@ def test_cleanup_aliases(fake_config, tmp_path):
     target = tmp_path / "target.exe"
     target.write_bytes(b"")
 
-    created = []
-    def _on_create(cmd, **kwargs):
-        created.append(kwargs)
-
     aliases = [
         AU.AliasInfo(install=dict(prefix=tmp_path), name="A", target=target),
         AU.AliasInfo(install=dict(prefix=tmp_path), name="B.exe", target=target),