Mercurial Repository: p/roundup/code: roundup/cgi/client.py comparison

comparison roundup/cgi/client.py @ 7814:9adf37c63b56

chore(refactor): use with open, consolidate/un-nest if ... Various cleanups: use 'with open(...)' rather than 'try: .. finally' to close file. use "return ..." rather than "x = ...; return x" Change if/else to if ternaries Swap ' for " to prevent escaping nested " Add lint silencing comments Fix bare return that needs to be return None.

author	John Rouillard <rouilj@ieee.org>
date	Sat, 16 Mar 2024 22:57:19 -0400
parents	928c20d4344b
children	f11c982f01c8

comparison

equal deleted inserted replaced

-:928c20d4344b
+:9adf37c63b56
 # PATCH verbs. They are processed like POST. So FieldStorage
 # hangs on these verbs trying to read posted data that
 # will never arrive.
 # If not defined, set CONTENT_LENGTH to 0 so it doesn't
 # hang reading the data.
-if self.env['REQUEST_METHOD'] in ['OPTIONS', 'DELETE', 'PATCH']:
+if self.env['REQUEST_METHOD'] in ['OPTIONS', 'DELETE', 'PATCH'] \
-if 'CONTENT_LENGTH' not in self.env:
+and 'CONTENT_LENGTH' not in self.env:
 self.env['CONTENT_LENGTH'] = 0
 logger.debug("Setting CONTENT_LENGTH to 0 for method: %s",
 self.env['REQUEST_METHOD'])
 # cgi.FieldStorage must save all data as
 # binary/bytes. Subclass BinaryFieldStorage does this.
 # It's a workaround for a bug in cgi.FieldStorage. See class
 # def for details.
 self._ok_message = []
 self._error_message = []
 def _gen_nonce(self):
 """ generate a unique nonce """
-n = b2s(base64.b32encode(random_.token_bytes(40)))
+return b2s(base64.b32encode(random_.token_bytes(40)))
-return n
 def setTranslator(self, translator=None):
 """Replace the translation engine
 'translator'
 # because handle_rest is called. Preflight requests
 # are unauthenticated, so no need to check permissions.
 if (self.is_cors_preflight()):
 self.handle_preflight()
 return
-elif not self.db.security.hasPermission('Rest Access', self.userid):
+if not self.db.security.hasPermission('Rest Access', self.userid):
 output = s2b('{ "error": { "status": 403, "msg": "Forbidden." } }')
 self.reject_request(output,
 message_type="application/json",
 status=403)
 return
 except (UsageError, Unauthorised) as msg:
 csrf_ok = False
 self.form_wins = True
 self._error_message = msg.args
-if csrf_ok:
+# If csrf checks pass. Run actions etc.
-# csrf checks pass. Run actions etc.
+# handle_action() may handle a form submit action.
-# possibly handle a form submit action (may change
+# It can change self.classname and self.template,
-# self.classname and self.template, and may also
+# and may also append error/ok_messages.
-# append error/ok_messages)
+html = self.handle_action() if csrf_ok else None
-html = self.handle_action()
-else:
-html = None
 if html:
 self.write_html(html)
 return
 # browser to prompt the user again.
 if self.instance.config.WEB_HTTP_AUTH:
 self.response_code = http_.client.UNAUTHORIZED
 realm = self.instance.config.TRACKER_NAME
 self.setHeader("WWW-Authenticate",
-"Basic realm=\"%s\"" % realm)
+'Basic realm="%s"' % realm)
 else:
 self.response_code = http_.client.FORBIDDEN
 self.renderFrontPage(str(message))
 except Unauthorised as message:
 # users may always see the front page
 encoder = codecs.getencoder(self.STORAGE_CHARSET)
 re_charref = re.compile('&#([0-9]+|x[0-9a-f]+);', re.IGNORECASE)
 def _decode_charref(matchobj):
 num = matchobj.group(1)
-if num[0].lower() == 'x':
+uc = int(num[1:], 16) if num[0].lower() == 'x' else int(num)
-uc = int(num[1:], 16)
-else:
-uc = int(num)
 return uchr(uc)
 for field_name in self.form:
 field = self.form[field_name]
 if (field.type == 'text/plain') and not field.filename:
 if r.lower() not in all_rolenames:
 raise LoginError("Token roles are invalid.")
 # will be used later to override the get_roles method
 # having it defined as truthy allows it to be used.
-override_get_roles = lambda self: iter_roles(  # noqa: E731
+override_get_roles = lambda self: iter_roles(  # noqa: ARG005
 ','.join(token['roles']))
 # if user was not set by http authorization, try session lookup
 if not user:
 user = self.session_api.get('user')
 pass
 if isinstance(action, list):
 raise SeriousError(
 self._('broken form: multiple @action values submitted'))
 elif action != '':
+# '' is value when no action parameter was found so run
+# this to extract action string value when action found.
 action = action.value.lower()
 if action in ('login', 'register'):
 return
 # allow Anonymous to view the "user" "register" template if they're
 if (self.db.security.hasPermission('Register', self.userid, 'user')
 and self.classname == 'user' and self.template == 'register'):
 return
 # otherwise for everything else
-if self.user == 'anonymous':
+if self.user == 'anonymous' and \
-if not self.db.security.hasPermission('Web Access', self.userid):
+not self.db.security.hasPermission('Web Access', self.userid):
 raise Unauthorised(self._("Anonymous users are not "
 "allowed to use the web interface"))
 def is_origin_header_ok(self, api=False, credentials=False):
 """Determine if origin is valid for the context
 """
 try:
 origin = self.env['HTTP_ORIGIN']
 except KeyError:
-if self.env['REQUEST_METHOD'] == 'GET':
+return self.env['REQUEST_METHOD'] == 'GET'
-return True
-else:
-return False
 # note base https://host/... ends host with with a /,
 # so add it to origin.
 foundat = self.base.find(origin + '/')
 if foundat == 0:
 # same for example) so we don't include here.
 header_names = [
 "ORIGIN",
 "REFERER",
 "X-FORWARDED-HOST",
-"HOST"
+"HOST",
 ]
 header_pass = 0  # count of passing header checks
 # If required headers are missing, raise an error
 if header_pass < enforce:
 logger.error(self._("Csrf: unable to verify sufficient headers"))
 raise UsageError(self._("Unable to verify sufficient headers"))
 enforce = config['WEB_CSRF_ENFORCE_HEADER_X-REQUESTED-WITH']
-if api:
+if api and enforce in ['required', 'yes']:
-if enforce in ['required', 'yes']:
+# if we get here we have usually passed at least one
-# if we get here we have usually passed at least one
+# header check. We check for presence of this custom
-# header check. We check for presence of this custom
+# header for xmlrpc/rest calls only.
-# header for xmlrpc/rest calls only.
+# E.G. X-Requested-With: XMLHttpRequest
-# E.G. X-Requested-With: XMLHttpRequest
+# Note we do not use CSRF nonces for xmlrpc/rest requests.
-# Note we do not use CSRF nonces for xmlrpc/rest requests.
+#
-#
+# see: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Protecting_REST_Services:_Use_of_Custom_Request_Headers
-# see: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Protecting_REST_Services:_Use_of_Custom_Request_Headers
+if 'HTTP_X_REQUESTED_WITH' not in self.env:
-if 'HTTP_X_REQUESTED_WITH' not in self.env:
+logger.error(self._(
-logger.error(self._(
+''"csrf X-REQUESTED-WITH xmlrpc required header "
-''"csrf X-REQUESTED-WITH xmlrpc required header "
+''"check failed for user%s."),
-''"check failed for user%s."),
+current_user)
-current_user)
+raise UsageError(self._("Required Header Missing"))
-raise UsageError(self._("Required Header Missing"))
 # Expire old csrf tokens now so we don't use them.  These will
 # be committed after the otks.destroy below.  Note that the
 # self.clean_up run as part of determine_user() will run only
 # once an hour. If we have short lived (e.g. 5 minute) keys
 # open the database or only set the user
 if not hasattr(self, 'db'):
 self.db = self.instance.open(username)
 self.db.tx_Source = "web"
-else:
+elif self.instance.optimize:
-if self.instance.optimize:
 self.db.setCurrentUser(username)
 self.db.tx_Source = "web"
 else:
 self.db.close()
 self.db = self.instance.open(username)
 self.db.tx_Source = "web"
 # The old session API refers to the closed database;
 # we can no longer use it.
 self.session_api = Session(self)
 # match designator in URL stripping leading 0's. So:
 # https://issues.roundup-tracker.org/issue002551190 is the same as
 # https://issues.roundup-tracker.org/issue2551190
 # Note: id's are strings not numbers so "02" != "2" but 02 == 2
 if template_override is not None:
 self.template = template_override
 else:
 self.template = ''
 return
-elif path[0] in ('_file', '@@file'):
+if path[0] in ('_file', '@@file'):
 raise SendStaticFile(os.path.join(*path[1:]))
 else:
 self.classname = path[0]
 if len(path) > 1:
 # send the file identified by the designator in path[0]
 # detemine meta-type
 file = str(file)
 mime_type = mimetypes.guess_type(file)[0]
 if not mime_type:
-if file.endswith('.css'):
+mime_type = 'text/css' if file.endswith('.css') else 'text/plain'
-mime_type = 'text/css'
-else:
-mime_type = 'text/plain'
 # get filename: given a/b/c.js extract c.js
 fn = file.rpartition("/")[2]
 if fn in self.Cache_Control:
 # if filename matches, don't use cache control
 # right one to the view parameter. If we don't have alternate
 # templates, just leave view alone.
 if (view and view.find('|') != -1):
 # we have alternate templates, parse them apart.
 (oktmpl, errortmpl) = view.split("|", 2)
-if self._error_message:
-# we have an error, use errortmpl
+# Choose the right template
-view = errortmpl
+view = errortmpl if self._error_message else oktmpl
-else:
-# no error message recorded, use oktmpl
-view = oktmpl
 loader = self.instance.templates
 # if classname is not set, use "home" template
 if name is None:
 tplname = self.selectTemplate(self.classname, self.template)
 # catch errors so we can handle PT rendering errors more nicely
 args = {
 'ok_message': self._ok_message,
-'error_message': self._error_message
+'error_message': self._error_message,
 }
 pt = self.instance.templates.load(tplname)
 # let the template render figure stuff out
 try:
 result = pt.render(self, None, None, **args)
 # receive an error message, and the adminstrator will
 # receive a traceback, albeit with less information
 # than the one we tried to generate above.
 if sys.version_info[0] > 2:
 raise exc_info[0](exc_info[1]).with_traceback(exc_info[2])
-else:
+exec('raise exc_info[0], exc_info[1], exc_info[2]')  # nosec
-exec('raise exc_info[0], exc_info[1], exc_info[2]')  # nosec
 def renderError(self, error, response_code=400, use_template=True):
 self.response_code = response_code
 # see if error message already logged add if not
 # response.
 return str(SeriousError(error))
 args = {
 'ok_message': self._ok_message,
-'error_message': self._error_message
+'error_message': self._error_message,
 }
 try:
 pt = self.instance.templates.load(tplname)
 return pt.render(self, None, None, **args)
 or not if_range.endswith('"')):
 return None
 # If the condition doesn't match the entity tag, then we
 # must send the client the entire file.
 if if_range != etag:
-return
+return None
 # The grammar for the Range header value is:
 #
 #   ranges-specifier = byte-ranges-specifier
 #   byte-ranges-specifier = bytes-unit "=" byte-range-set
 #   byte-range-set = 1#( byte-range-spec | suffix-byte-range-spec )
 if hasattr(self.request, "sendfile"):
 self.header()
 self._socket_op(self.request.sendfile, filename, offset, length)
 return
 # Fallback to the "write" operation.
-f = open(filename, 'rb')
+with open(filename, 'rb') as f:
-try:
 if offset:
 f.seek(offset)
 content = f.read(length)
-finally:
-f.close()
 self.write(content)
 def setHeader(self, header, value):
 """Override or delete a header to be returned to the user's browser.
 """

Mercurial > p > roundup > code

comparison roundup/cgi/client.py @ 7814:9adf37c63b56