Attachment 5316 Details for Bug 31923 – file upload with request options.patch

[patch] file upload with request options.patch

file_upload.patch (text/plain), 13.78 KB, created by Florian Best on 2013-07-10 16:35:38 CEST

(hide)

Description:

Filename:

MIME Type:

Creator: Florian Best

Created: 2013-07-10 16:35:38 CEST

Size: 13.78 KB

patch

obsolete

>Index: univention-management-console-frontend/univention-management-console-web-server
>===================================================================
>--- univention-management-console-frontend/univention-management-console-web-server	(Revision 42188)
>+++ univention-management-console-frontend/univention-management-console-web-server	(Arbeitskopie)
>@@ -40,11 +40,11 @@
> import simplejson
> import signal
> import sys
>-import tempfile
> import time
> import threading
> import uuid
> import datetime
>+import base64
> 
> import notifier
> from daemon.runner import DaemonRunner, DaemonRunnerStopFailureError, DaemonRunnerStartFailureError
>@@ -257,34 +257,6 @@
> 		except KeyError, e:
> 			CORE.info( 'Session %s not found' % sessionid )
> 
>-class UploadManager( dict ):
>-	def add( self, request_id, store ):
>-		tmpfile = tempfile.NamedTemporaryFile( prefix = request_id, dir = umcp.TEMPUPLOADDIR, delete = False )
>-		if hasattr(store, 'file') and store.file is None:
>-			tmpfile.write( store.value )
>-		else:
>-			tmpfile.write( store.file.read() )
>-		tmpfile.close()
>-		if request_id in self:
>-			self[ request_id ].append( tmpfile.name )
>-		else:
>-			self[ request_id ] = [ tmpfile.name ]
>-
>-		return tmpfile.name
>-
>-	def cleanup( self, request_id ):
>-		if request_id in self:
>-			filenames = self[ request_id ]
>-			for filename in filenames:
>-				if os.path.isfile( filename ):
>-					os.unlink( filename )
>-			del self[ request_id ]
>-			return True
>-
>-		return False
>-
>-_upload_manager = UploadManager()
>-
> class QueueRequest(object):
> 	"""Element for the request queue containing the assoziated session
> 	ID, the request object, a response queue and the request ip address."""
>@@ -457,46 +429,35 @@
> class CPUpload( CPgeneric ):
> 	def get_request(self, path, args ):
> 		self._log( 'info', 'Handle upload command' )
>-		global _upload_manager
> 		req = umcp.Request( 'UPLOAD', arguments = [ path ] )
> 
>-		options = []
>-		body = {}
>-		for iid, ifield in args.iteritems():
>+		options = {}
>+		for name, ifield in args.iteritems():
> 			if isinstance(ifield, cherrypy._cpcgifs.FieldStorage):
>-				# field is a FieldStorage object
>-				store = ifield
>-				tmpfile = _upload_manager.add( req.id, store )
>+				# lowercase all headers # TODO: check if not anymore required, yet
>+				ifield.headers = dict((key.lower(), val) for key, val in ifield.headers.items())
> 
>-				# check if filesize is allowed
>-				st = os.stat( tmpfile )
>-				max_size = int( configRegistry.get( 'umc/server/upload/max', 64 ) ) * 1024
>-				if st.st_size > max_size:
>-					self._log('warn', 'file of size %d could not be uploaded' % (st.st_size))
>-					raise cherrypy.HTTPError(httplib.BAD_REQUEST, 'The size of the uploaded file is too large')
>+				content = ifield.file.read()
>+				mimetype = ifield.headers.get('content-type')
>+				encoding = None
>+				if mimetype not in ('text/plain',):
>+					encoding = 'base64'
>+					content = base64.encodestring(content)
> 
>-				# check if enough free space is available
>-				min_size = int( configRegistry.get( 'umc/server/upload/min_free_space', 51200 ) ) # kilobyte
>-				s = os.statvfs(tmpfile)
>-				free_disk_space = s.f_bavail * s.f_frsize / 1024 # kilobyte
>-				if free_disk_space < min_size:
>-					self._log('error', 'there is not enough free space to upload files')
>-					raise cherrypy.HTTPError(httplib.BAD_REQUEST, 'There is not enough free space on disk')
>-
>-				filename = store.filename
>-				# some security
>-				for c in ('<>/'):
>-					filename = filename.replace(c, '_')
>-
>-				options.append( { 'filename' : filename, 'name' : store.name, 'tmpfile' : tmpfile } )
>+				options[name] = {
>+					'filename': ifield.filename,
>+					'name': ifield.name,
>+					'content': content,
>+					'mimetype': mimetype,
>+					'encoding': encoding
>+				}
> 			elif isinstance(ifield, basestring):
> 				# field is a string :)
>-				body[iid] = ifield
>+				options[name] = dict(name=name, content=ifield)
> 			else:
> 				# we cannot handle any other type
>-				CORE.warn( 'Unknown type of multipart/form entry: %s=%s' % (iid, ifield) )
>+				CORE.warn( 'Unknown type of multipart/form-data entry: %s=%s' % (name, ifield) )
> 
>-		req.body = body
> 		req.body['options'] = options
> 		return req
> 
>Index: univention-management-console/scripts/univention-management-console-client
>===================================================================
>--- univention-management-console/scripts/univention-management-console-client	(Revision 42188)
>+++ univention-management-console/scripts/univention-management-console-client	(Arbeitskopie)
>@@ -108,6 +108,8 @@
> 						value = eval( "%s('%s')" % ( typ, value ) )
> 					except NameError:
> 						print >>sys.stderr, "Invalid type for option: %s" % typ
>+				elif value.startswith('@'):
>+					value = open(value[1:]).read()
> 				msg.options[ key ] = value
> 		return msg
> 
>Index: univention-management-console/src/univention/management/console/protocol/session.py
>===================================================================
>--- univention-management-console/src/univention/management/console/protocol/session.py	(Revision 42188)
>+++ univention-management-console/src/univention/management/console/protocol/session.py	(Arbeitskopie)
>@@ -39,6 +39,8 @@
> import os
> import time
> import json
>+import tempfile
>+import os.path
> 
> import notifier
> import notifier.signals as signals
>@@ -70,6 +72,31 @@
> 
> TEMPUPLOADDIR = '/var/tmp/univention-management-console-frontend'
> 
>+class UploadManager(dict):
>+	def add(self, request_id, data):
>+		tmpfile = tempfile.NamedTemporaryFile(prefix=request_id, dir=TEMPUPLOADDIR, delete=False)
>+		tmpfile.write(data)
>+		tmpfile.close()
>+		if request_id in self:
>+			self[request_id].append(tmpfile.name)
>+		else:
>+			self[request_id] = [tmpfile.name]
>+
>+		return tmpfile.name
>+
>+	def cleanup(self, request_id):
>+		if request_id in self:
>+			filenames = self[request_id]
>+			for filename in filenames:
>+				if os.path.isfile(filename):
>+					os.unlink(filename)
>+			del self[request_id]
>+			return True
>+
>+		return False
>+
>+_upload_manager = UploadManager()
>+
> class State( signals.Provider ):
> 	"""Holds information about the state of an active session
> 
>@@ -165,6 +192,10 @@
> 		if msg.command == 'EXIT' and 'internal' in msg.arguments:
> 			return
> 
>+		if msg.command == 'UPLOAD':
>+			# remove temporary uploaded files
>+			_upload_manager.cleanup(msg.id)
>+
> 		self.signal_emit( 'result', msg )
> 
> 	def pid( self ):
>@@ -522,66 +553,110 @@
> 		module._inactivity_counter = MODULE_INACTIVITY_TIMER
> 
> 	def handle_request_upload( self, msg ):
>-		"""Handles an UPLOAD request. The command is used for the HTTP
>-		access to the UMC server. Incoming HTTP requests that send a
>-		list of files are passed on to the UMC server by storing the
>-		files in temporary files and passing the information about the
>-		files to the UMC server in the options of the request. The
>-		request options must be a list of dictionaries. Each dictionary
>-		must contain the following keys:
>+		"""Handles an UPLOAD request.
>+		The request options is a dictionary of dictionaries which
>+		can either be treaten as files or as normal strings
> 
>+
> 		* *filename* -- the original name of the file
> 		* *name* -- name of the form field
>-		* *tmpfile* -- filename of the temporary file
>+		* *content* -- the content of the file
>+		* *mimetype* -- the file mimetype
>+		* *enconding* -- encoding of the content if the content is encoded
> 
> 		:param Request msg: UMCP request
> 		"""
>-		# request.options = ( { 'filename' : store.filename, 'name' : store.name, 'tmpfile' : tmpfile } )
> 
>-		if not isinstance( msg.options, ( list, tuple ) ):
>+		if isinstance(msg.options, list):
>+			# backward compatibility
>+			try:
>+				msg.options = dict((val['name'], val) for val in msg.options)
>+			except (TypeError, KeyError):
>+				raise InvalidOptionsError
>+
>+		if not isinstance(msg.options, dict):
> 			raise InvalidOptionsError
> 
>-		for file_obj in msg.options:
>-			# check if required options exists and file_obj is a dict
>-			try:
>-				tmpfilename, filename, name = file_obj['tmpfile'], file_obj['filename'], file_obj['name']
>-			except:
>-				raise InvalidOptionsError('required options "tmpfile", "filename" or "name" is missing')
>+		def sanitize_item(item):
>+			if not isinstance(item, dict):
>+				raise InvalidOptionsError
> 
>-			# limit files to tmpdir
>-			if not os.path.realpath(tmpfilename).startswith(TEMPUPLOADDIR):
>-				raise InvalidOptionsError('invalid file: invalid path')
>+			res = dict((key, item.get(key, '')) for key in ('filename', 'name', 'content', 'mimetype'))
>+			# make sure some fields are strings
>+			if not all(isinstance(res[k], str) for k in ('filename', 'name', 'mimetype')):
>+				raise InvalidOptionsError
> 
>-			# check if file exists
>-			if not os.path.isfile( tmpfilename ):
>-				raise InvalidOptionsError('invalid file: file does not exists')
>+			# if the field is a file the content must be a string
>+			if res['filename'] and not isinstance(res['content'], str):
>+				raise InvalidOptionsError
> 
>-			# don't accept files bigger than umc/server/upload/max
>-			st = os.stat( tmpfilename )
>-			max_size = int( ucr.get( 'umc/server/upload/max', 64 ) ) * 1024
>-			if st.st_size > max_size:
>-				raise InvalidOptionsError('filesize is too large, maximum allowed filesize is %d' % (max_size,))
>+			# remove the possibility of XSS and path overwriting
>+			for c in '<>/':
>+				res['filename'] = res['filename'].replace(c, '_')
> 
>-		if msg.arguments and msg.arguments[0] not in ('', '/'):
>-			# The request has arguments, so it will be treaten as COMMAND
>-			self.handle_request_command(msg)
>+			return res
>+
>+		if not msg.arguments or msg.arguments[0] in ('', '/'):
>+			# The request is an generic UPLOAD command ('/upload')
>+			result = {}
>+			for item in msg.options.values():
>+				# read tmpfile and convert to base64
>+				res = sanitize_item(item)
>+				name = res['name']
>+
>+				# bring content into base64 (!)
>+				if item.get('encoding') != 'base64':
>+					res['content'] = base64.encodestring(res['content'])
>+				result[name] = res
>+
>+			response = Response( msg )
>+			response.result = result
>+			response.status = SUCCESS
>+
>+			self.signal_emit( 'response', response )
> 			return
> 
>-		# The request is an generic UPLOAD command (/upload)
>-		result = []
>-		for file_obj in msg.options:
>-			# read tmpfile and convert to base64
>-			tmpfilename, filename, name = file_obj['tmpfile'], file_obj['filename'], file_obj['name']
>-			with open( tmpfilename ) as buf:
>-				b64buf = base64.b64encode( buf.read() )
>-			result.append( { 'filename' : filename, 'name' : name, 'content' : b64buf } )
>+		ucr.load()
>+		max_upload_size = int(ucr.get('umc/server/upload/max', 64)) * 1024
>+		min_free_space = int(ucr.get('umc/server/upload/min_free_space', 51200)) # kilobyte
> 
>-		response = Response( msg )
>-		response.result = result
>-		response.status = SUCCESS
>+		# check if enough free space is available
>+		s = os.statvfs(TEMPUPLOADDIR)
>+		free_disk_space = s.f_bavail * s.f_frsize / 1024 # kilobyte
>+		if free_disk_space < min_free_space:
>+			CORE.error('There is not enough free space on disk to upload files')
>+			raise InvalidOptionsError('There is not enough free space on disk to upload files')
> 
>-		self.signal_emit( 'response', response )
>+		options = {}
>+		for item in msg.options.values():
>+			res = sanitize_item(item)
>+			name, filename = res['name'], res['filename']
> 
>+			if not filename:
>+				# field is not a file, it's a string
>+				res = res['content']
>+			else:
>+				content = res.pop('content')
>+
>+				# don't accept files bigger than umc/server/upload/max
>+				clen = len(content)
>+				if clen > max_upload_size:
>+					CORE.warn('File of size %d could not be uploaded' % (clen))
>+					raise InvalidOptionsError('Filesize is too large, maximum allowed filesize is %d' % (max_upload_size,))
>+
>+				# decode the file
>+				if item.get('encoding') == 'base64':
>+					content = base64.decodestring(content)
>+
>+				# write content to disk
>+				tmpfile = _upload_manager.add(msg.id, content)
>+				res['tmpfile'] = tmpfile
>+
>+			options[name] = res
>+		msg.options = options
>+
>+		self.handle_request_command(msg)
>+
> 	def handle_request_command( self, msg ):
> 		"""Handles a COMMAND request. The request must contain a valid
> 		and known command that can be accessed by the current user. If
>Index: univention-management-console/src/univention/management/console/protocol/message.py
>===================================================================
>--- univention-management-console/src/univention/management/console/protocol/message.py	(Revision 42188)
>+++ univention-management-console/src/univention/management/console/protocol/message.py	(Arbeitskopie)
>@@ -245,10 +245,6 @@
> 				PARSER.process( 'Error parsing UMCP message body' )
> 				raise ParseError( UMCP_ERR_UNPARSABLE_BODY, _( 'error parsing UMCP message body' ) )
> 
>-			for key in ( 'options', ):
>-				if key in self.body:
>-					setattr( self, key[ 1 : ], self.body[ key ] )
>-
> 		PARSER.info( 'UMCP %(type)s %(id)s parsed successfully' % groups )
> 
> 		return remains
>Index: univention-management-console-module-udm/umc/python/udm/__init__.py
>===================================================================
>--- univention-management-console-module-udm/umc/python/udm/__init__.py	(Revision 42188)
>+++ univention-management-console-module-udm/umc/python/udm/__init__.py	(Arbeitskopie)
>@@ -223,13 +223,11 @@
> 
> 	@LDAP_Connection
> 	def license_import( self, request, ldap_connection = None, ldap_position = None ):
>-		filename = None
>-		if isinstance(request.options, (list, tuple)) and request.options:
>-			# file upload
>-			filename = request.options[ 0 ][ 'tmpfile' ]
>-			if not os.path.realpath(filename).startswith(TEMPUPLOADDIR):
>-				self.finished(request.id, [{'success': False, 'message': 'invalid file path'}])
>-				return
>+		if request.command == 'UPLOAD':
>+			key = 'licenseUpload' # FIXME: in umc.widgets.Uploader the name of the form field get overwritten into 'uploadedfile'
>+			key = 'uploadedfile'
>+			self.required_options(request, key)
>+			filename = request.options[key]['tmpfile']
> 		else:
> 			self.required_options( request, 'license' )
> 			lic = request.options[ 'license' ]

Actions: View | Diff

Attachments on bug 31923: 5316 | 5317