Use copy and not inplace remove password + working case test

Author: mickours

git/cmd.py

@@ -405,7 +405,7 @@ def read_all_from_possibly_closed_stream(stream):
 ifstatus!=0:
 errstr=read_all_from_possibly_closed_stream(self.proc.stderr)
 log.debug('AutoInterrupt wait stderr: %r'% (errstr,))
-raiseGitCommandError(self.args, status, errstr)
+raiseGitCommandError(remove_password_if_present(self.args), status, errstr)
 # END status handling
 returnstatus
 # END auto interrupt
@@ -638,7 +638,7 @@ def execute(self, command,
 
  :param env:
  A dictionary of environment variables to be passed to `subprocess.Popen`.
-
+
  :param max_chunk_size:
  Maximum number of bytes in one chunk of data passed to the output_stream in
  one invocation of write() method. If the given number is not positive then
@@ -706,7 +706,7 @@ def execute(self, command,
 ifis_win:
 cmd_not_found_exception=OSError
 ifkill_after_timeout:
-raiseGitCommandError(command, '"kill_after_timeout" feature is not supported on Windows.')
+raiseGitCommandError(redacted_command, '"kill_after_timeout" feature is not supported on Windows.')
 else:
 ifsys.version_info[0] >2:
 cmd_not_found_exception=FileNotFoundError# NOQA # exists, flake8 unknown @UndefinedVariable
@@ -721,7 +721,7 @@ def execute(self, command,
 ifistream:
 istream_ok="<valid stream>"
 log.debug("Popen(%s, cwd=%s, universal_newlines=%s, shell=%s, istream=%s)",
-command, cwd, universal_newlines, shell, istream_ok)
+redacted_command, cwd, universal_newlines, shell, istream_ok)
 try:
 proc=Popen(command,
 env=env,
@@ -737,7 +737,7 @@ def execute(self, command,
 **subprocess_kwargs
  )
 exceptcmd_not_found_exceptionaserr:
-raiseGitCommandNotFound(command, err) fromerr
+raiseGitCommandNotFound(redacted_command, err) fromerr
 
 ifas_process:
 returnself.AutoInterrupt(proc, command)
@@ -787,7 +787,7 @@ def _kill_process(pid):
 watchdog.cancel()
 ifkill_check.isSet():
 stderr_value= ('Timeout: the command "%s" did not complete in %d '
-'secs.'% (" ".join(command), kill_after_timeout))
+'secs.'% (" ".join(redacted_command), kill_after_timeout))
 ifnotuniversal_newlines:
 stderr_value=stderr_value.encode(defenc)
 # strip trailing "\n"
@@ -811,7 +811,7 @@ def _kill_process(pid):
 proc.stderr.close()
 
 ifself.GIT_PYTHON_TRACE=='full':
-cmdstr=" ".join(command)
+cmdstr=" ".join(redacted_command)
 
 defas_text(stdout_value):
 returnnotoutput_streamandsafe_decode(stdout_value) or'<OUTPUT_STREAM>'
@@ -827,7 +827,7 @@ def as_text(stdout_value):
 # END handle debug printing
 
 ifwith_exceptionsandstatus!=0:
-raiseGitCommandError(command, status, stderr_value, stdout_value)
+raiseGitCommandError(redacted_command, status, stderr_value, stdout_value)
 
 ifisinstance(stdout_value, bytes) andstdout_as_string: # could also be output_stream
 stdout_value=safe_decode(stdout_value)

git/util.py

@@ -349,7 +349,9 @@ def remove_password_if_present(cmdline):
 
  This should be used for every log line that print a command line.
  """
+new_cmdline= []
 forindex, to_parseinenumerate(cmdline):
+new_cmdline.append(to_parse)
 try:
 url=urlsplit(to_parse)
 # Remove password from the URL if present
@@ -358,11 +360,11 @@ def remove_password_if_present(cmdline):
 
 edited_url=url._replace(
 netloc=url.netloc.replace(url.password, "*****"))
-cmdline[index] =urlunsplit(edited_url)
+new_cmdline[index] =urlunsplit(edited_url)
 exceptValueError:
 # This is not a valid URL
 pass
-returncmdline
+returnnew_cmdline
 
 
 #} END utilities

test/test_repo.py

@@ -240,7 +240,6 @@ def test_clone_from_with_path_contains_unicode(self):
 
 @with_rw_directory
 deftest_leaking_password_in_clone_logs(self, rw_dir):
-"""Check that the password is not printed on the logs"""
 password="fakepassword1234"
 try:
 Repo.clone_from(
@@ -249,6 +248,10 @@ def test_leaking_password_in_clone_logs(self, rw_dir):
 to_path=rw_dir)
 exceptGitCommandErroraserr:
 assertpasswordnotinstr(err), "The error message '%s' should not contain the password"%err
+# Working example from a blank private project
+Repo.clone_from(
+url="https://gitlab+deploy-token-392045:[email protected]/mercierm/test_git_python",
+to_path=rw_dir)
 
 @with_rw_repo('HEAD')
 deftest_max_chunk_size(self, repo):

test/test_util.py

@@ -325,7 +325,6 @@ def test_pickle_tzoffset(self):
 self.assertEqual(t1._name, t2._name)
 
 deftest_remove_password_from_command_line(self):
-"""Check that the password is not printed on the logs"""
 password="fakepassword1234"
 url_with_pass="https://fakeuser:{}@fakerepo.example.com/testrepo".format(password)
 url_without_pass="https://fakerepo.example.com/testrepo"
@@ -334,6 +333,10 @@ def test_remove_password_from_command_line(self):
 cmd_2= ["git", "clone", "-v", url_without_pass]
 cmd_3= ["no", "url", "in", "this", "one"]
 
-assertpasswordnotinremove_password_if_present(cmd_1)
+redacted_cmd_1=remove_password_if_present(cmd_1)
+assertpasswordnotin" ".join(redacted_cmd_1)
+# Check that we use a copy
+assertcmd_1isnotredacted_cmd_1
+assertpasswordin" ".join(cmd_1)
 assertcmd_2==remove_password_if_present(cmd_2)
 assertcmd_3==remove_password_if_present(cmd_3)