Commit50cbafc

committed

Add more test and remove password also from error logs

1 parentf7968d1 commit50cbafcCopy full SHA for 50cbafc

File tree

+24

-10

lines changed

+24

-10

lines changed

Lines changed: 2 additions & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -82,8 +82,8 @@ def pump_stream(cmdline, name, stream, is_decode, handler):`
`82`	`82`	`line=line.decode(defenc)`
`83`	`83`	`handler(line)`
`84`	`84`	`exceptExceptionasex:`
`85`		`-log.error("Pumping %r of cmd(%s) failed due to: %r",name,cmdline,ex)`
`86`		`-raiseCommandError(['<%s-pump>'%name]+cmdline,ex)fromex`
	`85`	`+log.error("Pumping %r of cmd(%s) failed due to: %r",name,remove_password_if_present(cmdline),ex)`
	`86`	`+raiseCommandError(['<%s-pump>'%name]+remove_password_if_present(cmdline),ex)fromex`
`87`	`87`	`finally:`
`88`	`88`	`stream.close()`
`89`	`89`

Lines changed: 6 additions & 7 deletions

Original file line number	Diff line number	Diff line change
`@@ -343,13 +343,13 @@ def expand_path(p, expand_vars=True):`
`343`	`343`	`defremove_password_if_present(cmdline):`
`344`	`344`	`"""`
`345`	`345`	`Parse any command line argument and if on of the element is an URL with a`
`346`		`- password, replace it by stars. If nothing found just returns a copy of the`
`347`		`- command line as-is.`
	`346`	`+ password, replace it by stars (in-place).`
	`347`	`+`
	`348`	`+ If nothing found just returns the command line as-is.`
`348`	`349`
`349`	`350`	`This should be used for every log line that print a command line.`
`350`	`351`	`"""`
`351`		`-redacted_cmdline= []`
`352`		`-forto_parseincmdline:`
	`352`	`+forindex,to_parseinenumerate(cmdline):`
`353`	`353`	`try:`
`354`	`354`	`url=urlsplit(to_parse)`
`355`	`355`	`# Remove password from the URL if present`
`@@ -358,12 +358,11 @@ def remove_password_if_present(cmdline):`
`358`	`358`
`359`	`359`	`edited_url=url._replace(`
`360`	`360`	`netloc=url.netloc.replace(url.password,"*****"))`
`361`		`-redacted_cmdline.append(urlunsplit(edited_url))`
	`361`	`+cmdline[index]=urlunsplit(edited_url)`
`362`	`362`	`exceptValueError:`
`363`		`-redacted_cmdline.append(to_parse)`
`364`	`363`	`# This is not a valid URL`
`365`	`364`	`pass`
`366`		`-returnredacted_cmdline`
	`365`	`+returncmdline`
`367`	`366`
`368`	`367`
`369`	`368`	`#} END utilities`

Lines changed: 16 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,8 @@`
`30`	`30`	`Actor,`
`31`	`31`	`IterableList,`
`32`	`32`	`cygpath,`
`33`		`-decygpath`
	`33`	`+decygpath,`
	`34`	`+remove_password_if_present,`
`34`	`35`	`)`
`35`	`36`
`36`	`37`
`@@ -322,3 +323,17 @@ def test_pickle_tzoffset(self):`
`322`	`323`	`t2=pickle.loads(pickle.dumps(t1))`
`323`	`324`	`self.assertEqual(t1._offset,t2._offset)`
`324`	`325`	`self.assertEqual(t1._name,t2._name)`
	`326`	`+`
	`327`	`+deftest_remove_password_from_command_line(self):`
	`328`	`+"""Check that the password is not printed on the logs"""`
	`329`	`+password="fakepassword1234"`
	`330`	`+url_with_pass="https://fakeuser:{}@fakerepo.example.com/testrepo".format(password)`
	`331`	`+url_without_pass="https://fakerepo.example.com/testrepo"`
	`332`	`+`
	`333`	`+cmd_1= ["git","clone","-v",url_with_pass]`
	`334`	`+cmd_2= ["git","clone","-v",url_without_pass]`
	`335`	`+cmd_3= ["no","url","in","this","one"]`
	`336`	`+`
	`337`	`+assertpasswordnotinremove_password_if_present(cmd_1)`
	`338`	`+assertcmd_2==remove_password_if_present(cmd_2)`
	`339`	`+assertcmd_3==remove_password_if_present(cmd_3)`

Comments

(0)