Merge pull request #133 from jocelyn/better_uploading_file

Improved the uploading of file in regard to temporary filename.

library/server/wsf/src/request/value/wsf_uploaded_file.e

@@ -102,6 +102,16 @@ feature -- Access: Uploaded File
 	filename: STRING
 			-- original filename
 
+	safe_filename: STRING
+			-- Safe name version of `filename'.
+			-- i.e: removing whitespace, accent, unicode characters ...
+		local
+			ut: WSF_FILE_UTILITIES [RAW_FILE]
+		do
+			create ut
+			Result := ut.safe_filename (filename)
+		end
+
 	content_type: STRING
 			-- Content type
 
@@ -118,7 +128,7 @@ feature -- Access: Uploaded File
 			end
 		end
 
-	tmp_basename: detachable STRING
+	tmp_basename: detachable READABLE_STRING_GENERAL
 			-- Basename of tmp file
 
 feature -- Conversion
@@ -156,92 +166,6 @@ feature -- Conversion
 			retry
 		end
 
-feature -- Implementation		
-
-	safe_filename: STRING
-		local
-			fn: like filename
-			c: CHARACTER
-			i, n: INTEGER
-		do
-			fn := filename
-
-				--| Compute safe filename, to avoid creating impossible filename, or dangerous one
-			from
-				i := 1
-				n := fn.count
-				create Result.make (n)
-			until
-				i > n
-			loop
-				c := fn[i]
-				inspect c
-				when '.', '-', '_' then
-					Result.extend (c)
-				when 'A' .. 'Z', 'a' .. 'z', '0' .. '9' then
-					Result.extend (c)
-				else
-					inspect c
-					when '%/192/' then Result.extend ('A') -- À
-					when '%/193/' then Result.extend ('A') -- Á
-					when '%/194/' then Result.extend ('A') -- Â
-					when '%/195/' then Result.extend ('A') -- Ã
-					when '%/196/' then Result.extend ('A') -- Ä
-					when '%/197/' then Result.extend ('A') -- Å
-					when '%/199/' then Result.extend ('C') -- Ç
-					when '%/200/' then Result.extend ('E') -- È
-					when '%/201/' then Result.extend ('E') -- É
-					when '%/202/' then Result.extend ('E') -- Ê
-					when '%/203/' then Result.extend ('E') -- Ë
-					when '%/204/' then Result.extend ('I') -- Ì
-					when '%/205/' then Result.extend ('I') -- Í
-					when '%/206/' then Result.extend ('I') -- Î
-					when '%/207/' then Result.extend ('I') -- Ï
-					when '%/210/' then Result.extend ('O') -- Ò
-					when '%/211/' then Result.extend ('O') -- Ó
-					when '%/212/' then Result.extend ('O') -- Ô
-					when '%/213/' then Result.extend ('O') -- Õ
-					when '%/214/' then Result.extend ('O') -- Ö
-					when '%/217/' then Result.extend ('U') -- Ù
-					when '%/218/' then Result.extend ('U') -- Ú
-					when '%/219/' then Result.extend ('U') -- Û
-					when '%/220/' then Result.extend ('U') -- Ü
-					when '%/221/' then Result.extend ('Y') -- Ý
-					when '%/224/' then Result.extend ('a') -- à
-					when '%/225/' then Result.extend ('a') -- á
-					when '%/226/' then Result.extend ('a') -- â
-					when '%/227/' then Result.extend ('a') -- ã
-					when '%/228/' then Result.extend ('a') -- ä
-					when '%/229/' then Result.extend ('a') -- å
-					when '%/231/' then Result.extend ('c') -- ç
-					when '%/232/' then Result.extend ('e') -- è
-					when '%/233/' then Result.extend ('e') -- é
-					when '%/234/' then Result.extend ('e') -- ê
-					when '%/235/' then Result.extend ('e') -- ë
-					when '%/236/' then Result.extend ('i') -- ì
-					when '%/237/' then Result.extend ('i') -- í
-					when '%/238/' then Result.extend ('i') -- î
-					when '%/239/' then Result.extend ('i') -- ï
-					when '%/240/' then Result.extend ('o') -- ð
-					when '%/242/' then Result.extend ('o') -- ò
-					when '%/243/' then Result.extend ('o') -- ó
-					when '%/244/' then Result.extend ('o') -- ô
-					when '%/245/' then Result.extend ('o') -- õ
-					when '%/246/' then Result.extend ('o') -- ö
-					when '%/249/' then Result.extend ('u') -- ù
-					when '%/250/' then Result.extend ('u') -- ú
-					when '%/251/' then Result.extend ('u') -- û
-					when '%/252/' then Result.extend ('u') -- ü
-					when '%/253/' then Result.extend ('y') -- ý
-					when '%/255/' then Result.extend ('y') -- ÿ
-					else
-						Result.extend ('-')
-					end
-				end
-				i := i + 1
-			end
-		end
-
 feature -- Basic operation
 
 	move_to (a_destination: READABLE_STRING_GENERAL): BOOLEAN
@@ -317,7 +241,7 @@ feature -- Element change
 		end
 
 note
-	copyright: "2011-2013, Jocelyn Fiat, Javier Velilla, Olivier Ligot, Eiffel Software and others"
+	copyright: "2011-2014, Jocelyn Fiat, Javier Velilla, Olivier Ligot, Colin Adams, Eiffel Software and others"
 	license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)"
 	source: "[
 			Eiffel Software

library/server/wsf/src/support/wsf_file_utilities.e

@@ -0,0 +1,171 @@
+note
+	description: "Summary description for {WSF_FILE_UTILITIES}."
+	date: "$Date$"
+	revision: "$Revision$"
+
+expanded class
+	WSF_FILE_UTILITIES [G -> FILE create make_with_path end]
+
+feature -- Factory
+
+	new_temporary_file (d: DIRECTORY; a_prefix: detachable READABLE_STRING_GENERAL; a_name: detachable READABLE_STRING_GENERAL): detachable G
+			-- New temporary file open for writing inside directory `d', with prefix `a_prefix' is set, and based on name `a_name' is set.
+			-- If it is unable to create such file opened for writing, then return Void.
+		require
+			d_valid: d.exists and then d.is_writable
+		local
+			f: G
+			fn: PATH
+			bn, tmp: STRING_32
+			dn: PATH
+			n: INTEGER
+		do
+			from
+				if a_prefix /= Void then
+					create tmp.make_from_string_general (a_prefix)
+				else
+					create tmp.make_from_string_general ("tmp")
+				end
+				dn := d.path
+				if a_name /= Void then
+					tmp.append_character ('-')
+					tmp.append_string_general (safe_filename (a_name))
+				end
+
+				fn := dn.extended (tmp)
+				create f.make_with_path (fn)
+				Result := new_file_opened_for_writing (f)
+				n := 0
+			until
+				Result /= Void
+				or else n > 1_000
+			loop
+				n := n + 1
+				create bn.make_from_string (tmp)
+				bn.append_character ('-')
+				bn.append_integer (n)
+				fn := dn.extended (bn)
+				f.make_with_path (fn)
+				Result := new_file_opened_for_writing (f)
+			end
+		ensure
+			result_opened_for_writing_if_set: Result /= Void implies Result.is_open_write
+		end
+
+	safe_filename (fn: READABLE_STRING_GENERAL): STRING
+			-- Safe filename that avoid impossible filename, or dangerous one.
+		local
+			c: CHARACTER_32
+			i, n: INTEGER
+		do
+			from
+				i := 1
+				n := fn.count
+				create Result.make (n)
+			until
+				i > n
+			loop
+				c := fn[i]
+				inspect c
+				when '.', '-', '_' then
+					Result.append_code (c.natural_32_code)
+				when 'A' .. 'Z', 'a' .. 'z', '0' .. '9' then
+					Result.append_code (c.natural_32_code)
+				else
+					inspect c
+					when '%/192/' then Result.extend ('A') -- À
+					when '%/193/' then Result.extend ('A') -- Á
+					when '%/194/' then Result.extend ('A') -- Â
+					when '%/195/' then Result.extend ('A') -- Ã
+					when '%/196/' then Result.extend ('A') -- Ä
+					when '%/197/' then Result.extend ('A') -- Å
+					when '%/199/' then Result.extend ('C') -- Ç
+					when '%/200/' then Result.extend ('E') -- È
+					when '%/201/' then Result.extend ('E') -- É
+					when '%/202/' then Result.extend ('E') -- Ê
+					when '%/203/' then Result.extend ('E') -- Ë
+					when '%/204/' then Result.extend ('I') -- Ì
+					when '%/205/' then Result.extend ('I') -- Í
+					when '%/206/' then Result.extend ('I') -- Î
+					when '%/207/' then Result.extend ('I') -- Ï
+					when '%/210/' then Result.extend ('O') -- Ò
+					when '%/211/' then Result.extend ('O') -- Ó
+					when '%/212/' then Result.extend ('O') -- Ô
+					when '%/213/' then Result.extend ('O') -- Õ
+					when '%/214/' then Result.extend ('O') -- Ö
+					when '%/217/' then Result.extend ('U') -- Ù
+					when '%/218/' then Result.extend ('U') -- Ú
+					when '%/219/' then Result.extend ('U') -- Û
+					when '%/220/' then Result.extend ('U') -- Ü
+					when '%/221/' then Result.extend ('Y') -- Ý
+					when '%/224/' then Result.extend ('a') -- à
+					when '%/225/' then Result.extend ('a') -- á
+					when '%/226/' then Result.extend ('a') -- â
+					when '%/227/' then Result.extend ('a') -- ã
+					when '%/228/' then Result.extend ('a') -- ä
+					when '%/229/' then Result.extend ('a') -- å
+					when '%/231/' then Result.extend ('c') -- ç
+					when '%/232/' then Result.extend ('e') -- è
+					when '%/233/' then Result.extend ('e') -- é
+					when '%/234/' then Result.extend ('e') -- ê
+					when '%/235/' then Result.extend ('e') -- ë
+					when '%/236/' then Result.extend ('i') -- ì
+					when '%/237/' then Result.extend ('i') -- í
+					when '%/238/' then Result.extend ('i') -- î
+					when '%/239/' then Result.extend ('i') -- ï
+					when '%/240/' then Result.extend ('o') -- ð
+					when '%/242/' then Result.extend ('o') -- ò
+					when '%/243/' then Result.extend ('o') -- ó
+					when '%/244/' then Result.extend ('o') -- ô
+					when '%/245/' then Result.extend ('o') -- õ
+					when '%/246/' then Result.extend ('o') -- ö
+					when '%/249/' then Result.extend ('u') -- ù
+					when '%/250/' then Result.extend ('u') -- ú
+					when '%/251/' then Result.extend ('u') -- û
+					when '%/252/' then Result.extend ('u') -- ü
+					when '%/253/' then Result.extend ('y') -- ý
+					when '%/255/' then Result.extend ('y') -- ÿ
+					else
+						Result.extend ('-')
+					end
+				end
+				i := i + 1
+			end
+		end
+
+feature {NONE} -- Implementation
+
+	new_file_opened_for_writing (f: G): detachable G
+			-- Returns a new file object opened for writing if possible
+			-- otherwise returns Void.
+		local
+			retried: BOOLEAN
+		do
+			if not retried then
+				if not f.exists then
+					f.open_write
+					if f.is_open_write then
+						Result := f
+					elseif not f.is_closed then
+						f.close
+					end
+				end
+			end
+		ensure
+			Result /= Void implies Result.is_open_write
+		rescue
+			retried := True
+			retry
+		end
+
+note
+	copyright: "2011-2014, Jocelyn Fiat, Javier Velilla, Olivier Ligot, Colin Adams, Eiffel Software and others"
+	license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)"
+	source: "[
+			Eiffel Software
+			5949 Hollister Ave., Goleta, CA 93117 USA
+			Telephone 805-685-1006, Fax 805-685-6869
+			Website http://www.eiffel.com
+			Customer support http://support.eiffel.com
+		]"
+end

library/server/wsf/src/wsf_request.e

@@ -1889,16 +1889,14 @@ feature {WSF_MIME_HANDLER} -- Temporary File handling
 		end
 
 	save_uploaded_file (a_up_file: WSF_UPLOADED_FILE; a_content: STRING)
-			-- Save uploaded file content to `a_filename'
+			-- Save uploaded file content `a_content' into `a_filename'.
 		local
-			bn: STRING
-			l_safe_name: STRING
-			f: RAW_FILE
 			dn: PATH
-			fn: PATH
 			d: DIRECTORY
-			n: INTEGER
 			rescued: BOOLEAN
+			temp_fac: WSF_FILE_UTILITIES [RAW_FILE]
+			l_prefix: STRING
+			dt: DATE_TIME
 		do
 			if not rescued then
 				if attached uploaded_file_path as p then
@@ -1909,26 +1907,24 @@ feature {WSF_MIME_HANDLER} -- Temporary File handling
 				end
 				create d.make_with_path (dn)
 				if d.exists and then d.is_writable then
-					l_safe_name := a_up_file.safe_filename
+					create temp_fac
-					from
-						bn := "tmp-" + l_safe_name
-						fn := dn.extended (bn)
-						create f.make_with_path (fn)
-						n := 0
-					until
-						not f.exists
-						or else n > 1_000
-					loop
-						n := n + 1
-						bn := "tmp-" + n.out + "-" + l_safe_name
-						fn := dn.extended (bn)
-						f.make_with_path (fn)
-					end
 
-					if not f.exists or else f.is_writable then
+					create l_prefix.make_from_string ("tmp_uploaded_")
+					create dt.make_now_utc
+					l_prefix.append_integer (dt.date.ordered_compact_date)
+					l_prefix.append_character ('_')
+					l_prefix.append_integer (dt.time.compact_time)
+					l_prefix.append_character ('.')
+					l_prefix.append_integer ((dt.time.fractional_second * 1_000_000_000).truncated_to_integer)
+
+					if attached temp_fac.new_temporary_file (d, l_prefix, a_up_file.filename) as f then
 						a_up_file.set_tmp_path (f.path)
-						a_up_file.set_tmp_basename (bn)
+						if attached f.path.entry as e then
-						f.open_write
+							a_up_file.set_tmp_basename (e.name)
+						else
+							a_up_file.set_tmp_basename (f.path.name) -- Should not occurs.
+						end
+						check f.is_open_write end
 						f.put_string (a_content)
 						f.close
 					else

library/server/wsf/tests/src/test_wsf_file_utilities.e

@@ -0,0 +1,149 @@
+note
+	description: "Summary description for {TEST_WSF_FILE_UTILITIES}."
+	date: "$Date$"
+	revision: "$Revision$"
+
+class
+	TEST_WSF_FILE_UTILITIES
+
+inherit
+	EQA_TEST_SET
+
+	SHARED_EXECUTION_ENVIRONMENT
+		undefine
+			default_create, copy
+		end
+
+feature -- Tests
+
+	test_temporary_file_0
+		do
+			test_temporary_file (0)
+		end
+
+	test_temporary_file_1_ns
+		do
+			test_temporary_file (1)
+		end
+
+	test_temporary_file_100_ns
+		do
+			test_temporary_file (100)
+		end
+
+	test_temporary_file_1_s
+		do
+			test_temporary_file (1_000_000_000)
+		end
+
+	new_temp_prefix: STRING
+		local
+			dt: DATE_TIME
+		do
+			create dt.make_now_utc
+			Result := "TEMP_"
+			Result.append_integer (dt.date.ordered_compact_date)
+			Result.append_character ('_')
+			Result.append_integer (dt.time.compact_time)
+			Result.append_character ('.')
+--			Result.append_integer (dt.time.milli_second)
+			Result.append_integer ((dt.time.fractional_second * 1_000_000_000).truncated_to_integer)
+		end
+
+	test_temporary_file (a_delay: INTEGER_64)
+		local
+			f: detachable FILE
+			f1,f2: detachable FILE
+			fac: WSF_FILE_UTILITIES [RAW_FILE]
+			d: DIRECTORY
+			logs: STRING_32
+			l_prefix: STRING
+		do
+
+			create logs.make (0)
+
+			create d.make_with_path ((create {PATH}.make_current).extended ("_tmp_"))
+			d.recursive_create_dir
+
+			create fac
+
+			l_prefix := new_temp_prefix
+			f := fac.new_temporary_file (d, l_prefix, "l'<27>t<EFBFBD> est l<>!")
+			if f /= Void then
+				create {RAW_FILE}f1.make_with_path (f.path)
+				f1.open_write
+				f1.close
+
+				logs.append ("f="); logs.append (f.path.name); logs.append ("%N")
+				if a_delay > 0 then
+						-- Wait `a_delay' in nanoseconds.
+					execution_environment.sleep (a_delay)
+					l_prefix := new_temp_prefix
+				end
+				f1 := fac.new_temporary_file (d, l_prefix, "l'<27>t<EFBFBD> est l<>!")
+
+				if a_delay > 0 then
+						-- Wait `a_delay' in nanoseconds.
+					execution_environment.sleep (a_delay)
+					l_prefix := new_temp_prefix
+				end
+				f2 := fac.new_temporary_file (d, l_prefix, "l'<27>t<EFBFBD> est l<>!")
+
+				if f1 /= Void then
+					logs.append ("f1="); logs.append (f1.path.name); logs.append ("%N")
+					f.put_string ("test")
+					f1.put_string ("blabla")
+					f1.close
+				else
+					assert ("able to create new file f1", False)
+				end
+				if f2 /= Void then
+					logs.append ("f2="); logs.append (f2.path.name); logs.append ("%N")
+					f.put_string ("TEST")
+					f2.put_string ("BLABLA")
+					f2.close
+				else
+					assert ("able to create new file f1", False)
+				end
+				assert ("f1 /= f2", (f1 /= Void and f2 /= Void) and then (not f1.path.is_same_file_as (f2.path)))
+				f.close
+
+				assert ("expected content for f", content (f).same_string ("testTEST"))
+				assert ("expected content for f1", content (f1).same_string ("blabla"))
+				assert ("expected content for f2", content (f2).same_string ("BLABLA"))
+			else
+				assert ("able to create new file f", False)
+			end
+				-- Cleaning
+
+			if f /= Void then
+				f.delete
+			end
+			if f1 /= Void then
+				f1.delete
+			end
+			if f2 /= Void then
+				f2.delete
+			end
+
+
+		end
+
+	content (f: detachable FILE): STRING
+		do
+			create Result.make (0)
+			if f /= Void then
+				from
+					f.open_read
+					assert (f.path.utf_8_name + " is opened", f.is_open_read)
+				until
+					f.end_of_file or f.exhausted
+				loop
+					f.read_stream (1_024)
+					Result.append (f.last_string)
+				end
+				f.close
+			end
+		end
+
+end

library/server/wsf/tests/tests-safe.ecf

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="ISO-8859-1"?>
-<system xmlns="http://www.eiffel.com/developers/xml/configuration-1-10-0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.eiffel.com/developers/xml/configuration-1-10-0 http://www.eiffel.com/developers/xml/configuration-1-10-0.xsd" name="wsf_tests" uuid="C4FF9CDA-B4E4-4841-97E0-7F799B85B657">
+<system xmlns="http://www.eiffel.com/developers/xml/configuration-1-13-0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.eiffel.com/developers/xml/configuration-1-13-0 http://www.eiffel.com/developers/xml/configuration-1-13-0.xsd" name="wsf_tests" uuid="C4FF9CDA-B4E4-4841-97E0-7F799B85B657">
 	<target name="server">
 		<root class="TEST" feature="make"/>
 		<file_rule>
@@ -23,6 +23,7 @@
 		<library name="http" location="..\..\..\network\protocol\http\http-safe.ecf" readonly="false"/>
 		<library name="http_client" location="..\..\..\network\http_client\http_client-safe.ecf" readonly="false"/>
 		<library name="thread" location="$ISE_LIBRARY\library\thread\thread-safe.ecf"/>
+		<library name="time" location="$ISE_LIBRARY\library\time\time-safe.ecf"/>
 		<library name="wsf" location="..\wsf-safe.ecf" readonly="false">
 			<option>
 				<assertions precondition="true" postcondition="true" check="true" supplier_precondition="true"/>