Merge pull request #54 from demarey/53-command-and-arguments-should-be-encoded-with-the-platform-encoding

demarey · web-flow · commit 1c69d1284855 · 2020-03-24T16:07:53.000+01:00
Fixes #53 command and arguments should be encoded with the platform …
diff --git a/repository/OSSubprocess-Tests-Unit.package/OSSUnixSubprocessTest.class/instance/testBasicCommandWithNonAsciiArgument.st b/repository/OSSubprocess-Tests-Unit.package/OSSUnixSubprocessTest.class/instance/testBasicCommandWithNonAsciiArgument.st
@@ -0,0 +1,10 @@
+tests - basic
+testBasicCommandWithNonAsciiArgument
+	| command | 
+	
+	self withNonAsciiDirectoryDo: [ :dir |
+		command := self newCommand
+					command: '/bin/ls';
+					arguments: { dir fullName }.
+		command runAndWait
+	]
diff --git a/repository/OSSubprocess-Tests-Unit.package/OSSUnixSubprocessTest.class/instance/testBasicShellCommandWithNonAsciiCharacters.st b/repository/OSSubprocess-Tests-Unit.package/OSSUnixSubprocessTest.class/instance/testBasicShellCommandWithNonAsciiCharacters.st
@@ -0,0 +1,12 @@
+tests - shell
+testBasicShellCommandWithNonAsciiCharacters
+	
+	self withNonAsciiDirectoryDo: [ :dir |
+		(dir / 'ok.txt') ensureCreateFile.
+		self newCommand
+			shellCommand: 'ls ', dir fullName;
+			redirectStdout;
+			runAndWaitOnExitDo: [ :command :outString |
+				self assert: (outString includesSubstring: 'ok.txt').						
+		]
+	]
diff --git a/repository/OSSubprocess-Tests-Unit.package/OSSUnixSubprocessTest.class/instance/withNonAsciiDirectoryDo..st b/repository/OSSubprocess-Tests-Unit.package/OSSUnixSubprocessTest.class/instance/withNonAsciiDirectoryDo..st
@@ -0,0 +1,7 @@
+helpers
+withNonAsciiDirectoryDo: aBlock 
+	| directory |
+	directory := FileLocator temp / (self class name , '-éoï-' , UUIDGenerator next asString).
+	directory ensureCreateDirectory.
+	[ aBlock cull: directory asFileReference ]
+	ensure: [ directory ensureDeleteAll ]
diff --git a/repository/OSSubprocess-Tests-Unit.package/OSSVMProcessTest.class/instance/testChangeDir.st b/repository/OSSubprocess-Tests-Unit.package/OSSVMProcessTest.class/instance/testChangeDir.st
@@ -3,7 +3,8 @@ testChangeDir
 	| oldDir | 
 	oldDir := self systemAccessor getcwd.
 	OSSVMProcess vmProcess
-		lockCwdWithValue: '/tmp' 
+		lockCwdWithValue: '/tmp'
+		encoding: #utf8 
 		during: [
 			"Grrr in latest OSX /tmp is mapped to /private/tmp..."
 			self assert: ((self systemAccessor getcwd = '/tmp') or: [ self systemAccessor getcwd = '/private/tmp' ]).
diff --git a/repository/OSSubprocess-Tests-Unit.package/OSSVMProcessTest.class/instance/testChangeDirWithNonAsciiCharacters.st b/repository/OSSubprocess-Tests-Unit.package/OSSVMProcessTest.class/instance/testChangeDirWithNonAsciiCharacters.st
@@ -6,7 +6,8 @@ testChangeDirWithNonAsciiCharacters
 	newDir ensureCreateDirectory.
 
 	OSSVMProcess vmProcess
-		lockCwdWithValue: newDir fullName 
+		lockCwdWithValue: newDir fullName
+		encoding: #utf8
 		during: [ duringSystemCwd := self systemAccessor getcwd ].
 	duringSystemCwd := UnicodeNormalizer new toNFC: duringSystemCwd asByteArray utf8Decoded.
 
diff --git a/repository/OSSubprocess-Tests-Unit.package/OSSVMProcessTest.class/instance/testChangeDirWithNonExistingDir.st b/repository/OSSubprocess-Tests-Unit.package/OSSVMProcessTest.class/instance/testChangeDirWithNonExistingDir.st
@@ -1,8 +1,11 @@
 tests
 testChangeDirWithNonExistingDir
-	| oldDir | 
+	| oldDir |
 	oldDir := self systemAccessor getcwd.
-	[ OSSVMProcess vmProcess lockCwdWithValue: '/tmpWhatever'  during: [ ] ]
-	on: Error do:  [ :ex | ex printString includesSubstring: 'does not exist' ].
-	self assert: self systemAccessor getcwd equals: oldDir
-				
+	[ OSSVMProcess vmProcess
+		lockCwdWithValue: '/tmpWhatever'
+		encoding: #utf8
+		during: [  ] ]
+	on: Error
+	do: [ :ex | self assert: (ex printString includesSubstring: 'does not exist') ].
+	self assert: self systemAccessor getcwd equals: oldDir
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/arguments..st b/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/arguments..st
@@ -1,3 +1,3 @@
 settings
 arguments: anObject
-	arguments := anObject
+	arguments := anObject collect: [ :each | (each encodeWith: encoding) asString ]
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/command..st b/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/command..st
@@ -1,3 +1,3 @@
 settings
-command: anObject
-	command := anObject
+command: aString
+	command := (aString encodeWith: encoding) asString
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/defaultEncoding.st b/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/defaultEncoding.st
@@ -0,0 +1,3 @@
+accessing
+defaultEncoding 
+	^ OSEnvironment current defaultEncoding 
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/encoding..st b/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/encoding..st
@@ -0,0 +1,6 @@
+settings
+encoding: anEncoder
+	"Specify the encoder to use to encode/decode Strings given to/retrieved from the process.
+	Default encoding for Unix-like systems is UTF-8".
+	" self encoding: #utf8 "
+	encoding := anEncoder
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/initialize.st b/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/initialize.st
@@ -9,4 +9,5 @@ initialize
 	retrievedStdout := String new writeStream. 
 	retrievedStderr := String new writeStream. 
 	terminateOnShutdown := false.
-	waitPidCriticalSemaphore := Semaphore forMutualExclusion
+	waitPidCriticalSemaphore := Semaphore forMutualExclusion.
+	encoding := self defaultEncoding.
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/internalRun.st b/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/internalRun.st
@@ -17,7 +17,8 @@ internalRun
 			"This is a hack in order to support a #workingDirectory: kind of feature. 
 			Please read the comment of lockCwdWithValue:during: "
 			OSSVMProcess vmProcess
-				lockCwdWithValue: workingDirectory 
+				lockCwdWithValue: workingDirectory
+				encoding: encoding 
 				during: [ returnValue := self primitivePosixSpawn: pidtPointer command: command fileActions: posixSpawnFileActionsT attributes: nil argPointer: self buildPrimitiveArgVArgument envPointer: self buildPrimitiveEnvArgument. ]
 		].
 	(returnValue > 0) 
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/loginShellCommand..st b/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/loginShellCommand..st
@@ -2,5 +2,5 @@ shell
 loginShellCommand: aShellCommandString
 	"This is a simple facility method for the cases when the user wants to use a login shell as the program.
 	See comments of shellCommand: for more information."
-	command := self shellCommand.
-	arguments := Array with: '-l' with: '-c' with: aShellCommandString. 
+	self shellCommand: aShellCommandString.
+	arguments := Array with: '-l' withAll: arguments.
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/shell.command..st b/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/shell.command..st
@@ -3,4 +3,4 @@ shell: aShellPath command: aShellCommandString
 	"Please read comment of #shellCommand:.
 	This is the same as shellCommand: but with the addition that the use can define which shell to be used via 'aBasePath' "
 	command := aShellPath.
-	arguments := Array with: '-c' with: aShellCommandString. 
+	arguments := Array with: '-c' with: (aShellCommandString encodeWith: encoding) asString. 
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/shellCommand..st b/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/shellCommand..st
@@ -5,5 +5,5 @@ shellCommand: aShellCommandString
 	rather than having to do set the command sh, send the '-c' argument, etc etc etc.
 	We first try to use the SHELL defined in the OS by getting the env variable $SHELL. 
 	If not found, then we fallback to /bin/sh"
-	command := self shellCommand.
-	arguments := Array with: '-c' with: aShellCommandString. 
+	
+	self shell: self shellCommand command: aShellCommandString. 
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/workingDirectory..st b/repository/OSSubprocess.package/OSSUnixSubprocess.class/instance/workingDirectory..st
@@ -1,4 +1,6 @@
 settings
 workingDirectory: aString
-	"This is a setting that allows defining a working directory for the OS process once started. "
+	"This is a setting that allows defining a working directory for the OS process once started."
+	"Working directory is not encoded when set because it is used as FileReference at some places.
+	Instead, we encode it in #lockCwdWithValue:encoding:during:"
 	workingDirectory := aString
diff --git a/repository/OSSubprocess.package/OSSUnixSubprocess.class/properties.json b/repository/OSSubprocess.package/OSSUnixSubprocess.class/properties.json
@@ -27,7 +27,8 @@
 		"shouldContinueWaiting",
 		"terminateOnShutdown",
 		"workingDirectory",
-		"waitPidCriticalSemaphore"
+		"waitPidCriticalSemaphore",
+		"encoding"
 	],
 	"name" : "OSSUnixSubprocess",
 	"type" : "normal"
diff --git a/repository/OSSubprocess.package/OSSVMProcess.class/instance/lockCwdWithValue.encoding.during..st b/repository/OSSubprocess.package/OSSVMProcess.class/instance/lockCwdWithValue.encoding.during..st
@@ -1,5 +1,5 @@
 cwd
-lockCwdWithValue: cwdNewValue during: aBlock
+lockCwdWithValue: cwdNewValue encoding: encoding during: aBlock
 	"This method is a complete hack in order to support a #cmd: option in OSSUnixSubprocess. 
 	OSSUnixSubprocess relies in posix_spawn() family of functions to spwan processes in the OS,
 	and these functions do not allow specifying a starting working directory for the child process. 
@@ -26,7 +26,7 @@ lockCwdWithValue: cwdNewValue during: aBlock
 		| oldCwd |
 		oldCwd := self systemAccessor getcwd.
 		[
-			self systemAccessor chdir: cwdNewValue utf8Encoded asString.
+			self systemAccessor chdir: (cwdNewValue encodeWith: encoding) asString.
 			mutexForCwd critical: aBlock.
 		]
 		ensure: [ 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+accessing`
	`2`	`+defaultEncoding`
	`3`	`+ ^ OSEnvironment current defaultEncoding`