Skip to content

Commit c7febbf

Browse files
committed
Fix paramter passing in logging messages
Pass parameters into logger calls directly instead of pre-merging with the logger message. This way the logs are easier to maintain and process. The code is also faster when logging is turned off since it doesn't have to evaluate all params and render them into strings.
1 parent 1228db6 commit c7febbf

34 files changed

+442
-447
lines changed

example/idp2/idp.py

Lines changed: 44 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ def dict2list_of_tuples(d):
9191
class Service(object):
9292
def __init__(self, environ, start_response, user=None):
9393
self.environ = environ
94-
logger.debug("ENVIRON: %s" % environ)
94+
logger.debug("ENVIRON: %s", environ)
9595
self.start_response = start_response
9696
self.user = user
9797

@@ -104,7 +104,7 @@ def unpack_redirect(self):
104104

105105
def unpack_post(self):
106106
_dict = parse_qs(get_post(self.environ))
107-
logger.debug("unpack_post:: %s" % _dict)
107+
logger.debug("unpack_post:: %s", _dict)
108108
try:
109109
return dict([(k, v[0]) for k, v in _dict.items()])
110110
except Exception:
@@ -124,11 +124,11 @@ def unpack_either(self):
124124
_dict = self.unpack_post()
125125
else:
126126
_dict = None
127-
logger.debug("_dict: %s" % _dict)
127+
logger.debug("_dict: %s", _dict)
128128
return _dict
129129

130130
def operation(self, saml_msg, binding):
131-
logger.debug("_operation: %s" % saml_msg)
131+
logger.debug("_operation: %s", saml_msg)
132132
if not (saml_msg and 'SAMLRequest' in saml_msg):
133133
resp = BadRequest('Error parsing request or no request')
134134
return resp(self.environ, self.start_response)
@@ -210,7 +210,7 @@ def soap(self):
210210
"""
211211
logger.debug("- SOAP -")
212212
_dict = self.unpack_soap()
213-
logger.debug("_dict: %s" % _dict)
213+
logger.debug("_dict: %s", _dict)
214214
return self.operation(_dict, BINDING_SOAP)
215215

216216
def uri(self):
@@ -273,19 +273,19 @@ def verify_request(self, query, binding):
273273

274274
logger.info("parsed OK")
275275
_authn_req = self.req_info.message
276-
logger.debug("%s" % _authn_req)
276+
logger.debug("%s", _authn_req)
277277

278278
try:
279279
self.binding_out, self.destination = IDP.pick_binding(
280280
"assertion_consumer_service",
281281
bindings=self.response_bindings,
282282
entity_id=_authn_req.issuer.text, request=_authn_req)
283283
except Exception as err:
284-
logger.error("Couldn't find receiver endpoint: %s" % err)
284+
logger.error("Couldn't find receiver endpoint: %s", err)
285285
raise
286286

287-
logger.debug("Binding: %s, destination: %s" % (self.binding_out,
288-
self.destination))
287+
logger.debug("Binding: %s, destination: %s", self.binding_out,
288+
self.destination)
289289

290290
resp_args = {}
291291
try:
@@ -313,18 +313,18 @@ def do(self, query, binding_in, relay_state="", encrypt_cert=None,
313313
try:
314314
resp_args, _resp = self.verify_request(query, binding_in)
315315
except UnknownPrincipal as excp:
316-
logger.error("UnknownPrincipal: %s" % (excp,))
316+
logger.error("UnknownPrincipal: %s", excp)
317317
resp = ServiceError("UnknownPrincipal: %s" % (excp,))
318318
return resp(self.environ, self.start_response)
319319
except UnsupportedBinding as excp:
320-
logger.error("UnsupportedBinding: %s" % (excp,))
320+
logger.error("UnsupportedBinding: %s", excp)
321321
resp = ServiceError("UnsupportedBinding: %s" % (excp,))
322322
return resp(self.environ, self.start_response)
323323

324324
if not _resp:
325325
identity = USERS[self.user].copy()
326326
# identity["eduPersonTargetedID"] = get_eptid(IDP, query, session)
327-
logger.info("Identity: %s" % (identity,))
327+
logger.info("Identity: %s", identity)
328328

329329
if REPOZE_ID_EQUIVALENT:
330330
identity[REPOZE_ID_EQUIVALENT] = self.user
@@ -345,7 +345,7 @@ def do(self, query, binding_in, relay_state="", encrypt_cert=None,
345345
resp = ServiceError("Exception: %s" % (excp,))
346346
return resp(self.environ, self.start_response)
347347

348-
logger.info("AuthNResponse: %s" % _resp)
348+
logger.info("AuthNResponse: %s", _resp)
349349
if self.op_type == "ecp":
350350
kwargs = {"soap_headers": [
351351
ecp.Response(
@@ -357,12 +357,12 @@ def do(self, query, binding_in, relay_state="", encrypt_cert=None,
357357
"%s" % _resp, self.destination,
358358
relay_state, response=True, **kwargs)
359359

360-
logger.debug("HTTPargs: %s" % http_args)
360+
logger.debug("HTTPargs: %s", http_args)
361361
return self.response(self.binding_out, http_args)
362362

363363
@staticmethod
364364
def _store_request(saml_msg):
365-
logger.debug("_store_request: %s" % saml_msg)
365+
logger.debug("_store_request: %s", saml_msg)
366366
key = sha1(saml_msg["SAMLRequest"]).hexdigest()
367367
# store the AuthnRequest
368368
IDP.ticket[key] = saml_msg
@@ -508,7 +508,7 @@ def do_authentication(environ, start_response, authn_context, key,
508508

509509
if len(auth_info):
510510
method, reference = auth_info[0]
511-
logger.debug("Authn chosen: %s (ref=%s)" % (method, reference))
511+
logger.debug("Authn chosen: %s (ref=%s)", method, reference)
512512
return method(environ, start_response, reference, key, redirect_uri, headers)
513513
else:
514514
resp = Unauthorized("No usable authentication method")
@@ -546,7 +546,7 @@ def username_password_authn(environ, start_response, reference, key,
546546
"authn_reference": reference,
547547
"redirect_uri": redirect_uri
548548
}
549-
logger.info("do_authentication argv: %s" % argv)
549+
logger.info("do_authentication argv: %s", argv)
550550
return resp(environ, start_response, **argv)
551551

552552

@@ -562,7 +562,7 @@ def verify_username_and_password(dic):
562562
def do_verify(environ, start_response, _):
563563
query = parse_qs(get_post(environ))
564564

565-
logger.debug("do_verify: %s" % query)
565+
logger.debug("do_verify: %s", query)
566566

567567
try:
568568
_ok, user = verify_username_and_password(query)
@@ -576,13 +576,13 @@ def do_verify(environ, start_response, _):
576576
uid = rndstr(24)
577577
IDP.cache.uid2user[uid] = user
578578
IDP.cache.user2uid[user] = uid
579-
logger.debug("Register %s under '%s'" % (user, uid))
579+
logger.debug("Register %s under '%s'", user, uid)
580580

581581
kaka = set_cookie("idpauthn", "/", uid, query["authn_reference"][0])
582582

583583
lox = "%s?id=%s&key=%s" % (query["redirect_uri"][0], uid,
584584
query["key"][0])
585-
logger.debug("Redirect => %s" % lox)
585+
logger.debug("Redirect => %s", lox)
586586
resp = Redirect(lox, headers=[kaka], content="text/html")
587587

588588
return resp(environ, start_response)
@@ -610,17 +610,17 @@ def do(self, request, binding, relay_state="", encrypt_cert=None, **kwargs):
610610

611611
logger.info("--- Single Log Out Service ---")
612612
try:
613-
logger.debug("req: '%s'" % request)
613+
logger.debug("req: '%s'", request)
614614
req_info = IDP.parse_logout_request(request, binding)
615615
except Exception as exc:
616-
logger.error("Bad request: %s" % exc)
616+
logger.error("Bad request: %s", exc)
617617
resp = BadRequest("%s" % exc)
618618
return resp(self.environ, self.start_response)
619619

620620
msg = req_info.message
621621
if msg.name_id:
622622
lid = IDP.ident.find_local_id(msg.name_id)
623-
logger.info("local identifier: %s" % lid)
623+
logger.info("local identifier: %s", lid)
624624
if lid in IDP.cache.user2uid:
625625
uid = IDP.cache.user2uid[lid]
626626
if uid in IDP.cache.uid2user:
@@ -630,8 +630,8 @@ def do(self, request, binding, relay_state="", encrypt_cert=None, **kwargs):
630630
try:
631631
IDP.session_db.remove_authn_statements(msg.name_id)
632632
except KeyError as exc:
633-
logger.error("Unknown session: %s" % exc)
634-
resp = ServiceError("Unknown session: %s" % exc)
633+
logger.error("Unknown session: %s", exc)
634+
resp = ServiceError("Unknown session: %s", exc)
635635
return resp(self.environ, self.start_response)
636636

637637
resp = IDP.create_logout_response(msg, [binding])
@@ -649,15 +649,15 @@ def do(self, request, binding, relay_state="", encrypt_cert=None, **kwargs):
649649
hinfo = IDP.apply_binding(binding, "%s" % resp, destination,
650650
relay_state, response=response)
651651
except Exception as exc:
652-
logger.error("ServiceError: %s" % exc)
652+
logger.error("ServiceError: %s", exc)
653653
resp = ServiceError("%s" % exc)
654654
return resp(self.environ, self.start_response)
655655

656656
#_tlh = dict2list_of_tuples(hinfo["headers"])
657657
delco = delete_cookie(self.environ, "idpauthn")
658658
if delco:
659659
hinfo["headers"].append(delco)
660-
logger.info("Header: %s" % (hinfo["headers"],))
660+
logger.info("Header: %s", (hinfo["headers"],))
661661

662662
if binding == BINDING_HTTP_REDIRECT:
663663
for key, value in hinfo['headers']:
@@ -688,7 +688,7 @@ def do(self, query, binding, relay_state="", encrypt_cert=None):
688688
request.name_id, request.new_id, request.new_encrypted_id,
689689
request.terminate)
690690

691-
logger.debug("New NameID: %s" % name_id)
691+
logger.debug("New NameID: %s", name_id)
692692

693693
_resp = IDP.create_manage_name_id_response(request)
694694

@@ -718,12 +718,12 @@ def do(self, aid, binding, relay_state="", encrypt_cert=None):
718718

719719
hinfo = IDP.apply_binding(BINDING_URI, "%s" % assertion, response=True)
720720

721-
logger.debug("HINFO: %s" % hinfo)
721+
logger.debug("HINFO: %s", hinfo)
722722
resp = Response(hinfo["data"], headers=hinfo["headers"])
723723
return resp(self.environ, self.start_response)
724724

725725
def operation(self, _dict, binding, **kwargs):
726-
logger.debug("_operation: %s" % _dict)
726+
logger.debug("_operation: %s", _dict)
727727
if not _dict or "ID" not in _dict:
728728
resp = BadRequest('Error parsing request or no request')
729729
return resp(self.environ, self.start_response)
@@ -764,7 +764,7 @@ def do(self, request, binding, relay_state="", encrypt_cert=None):
764764
_query.requested_authn_context,
765765
_query.session_index)
766766

767-
logger.debug("response: %s" % msg)
767+
logger.debug("response: %s", msg)
768768
hinfo = IDP.apply_binding(BINDING_SOAP, "%s" % msg, "", "",
769769
response=True)
770770

@@ -787,15 +787,15 @@ def do(self, request, binding, relay_state="", encrypt_cert=None):
787787

788788
name_id = _query.subject.name_id
789789
uid = name_id.text
790-
logger.debug("Local uid: %s" % uid)
790+
logger.debug("Local uid: %s", uid)
791791
identity = EXTRA[uid]
792792

793793
# Comes in over SOAP so only need to construct the response
794794
args = IDP.response_args(_query, [BINDING_SOAP])
795795
msg = IDP.create_attribute_response(identity,
796796
name_id=name_id, **args)
797797

798-
logger.debug("response: %s" % msg)
798+
logger.debug("response: %s", msg)
799799
hinfo = IDP.apply_binding(BINDING_SOAP, "%s" % msg, "", "",
800800
response=True)
801801

@@ -842,7 +842,7 @@ def do(self, query, binding, relay_state="", encrypt_cert=None):
842842
# Cookie handling
843843
# ----------------------------------------------------------------------------
844844
def info_from_cookie(kaka):
845-
logger.debug("KAKA: %s" % kaka)
845+
logger.debug("KAKA: %s", kaka)
846846
if kaka:
847847
cookie_obj = SimpleCookie(kaka)
848848
morsel = cookie_obj.get("idpauthn", None)
@@ -859,14 +859,14 @@ def info_from_cookie(kaka):
859859

860860
def delete_cookie(environ, name):
861861
kaka = environ.get("HTTP_COOKIE", '')
862-
logger.debug("delete KAKA: %s" % kaka)
862+
logger.debug("delete KAKA: %s", kaka)
863863
if kaka:
864864
cookie_obj = SimpleCookie(kaka)
865865
morsel = cookie_obj.get(name, None)
866866
cookie = SimpleCookie()
867867
cookie[name] = ""
868868
cookie[name]['path'] = "/"
869-
logger.debug("Expire: %s" % morsel)
869+
logger.debug("Expire: %s", morsel)
870870
cookie[name]["expires"] = _expiration("dawn")
871871
return tuple(cookie.output().split(": ", 1))
872872
return None
@@ -877,7 +877,7 @@ def set_cookie(name, _, *args):
877877
cookie[name] = base64.b64encode(":".join(args))
878878
cookie[name]['path'] = "/"
879879
cookie[name]["expires"] = _expiration(5) # 5 minutes from now
880-
logger.debug("Cookie expires: %s" % cookie[name]["expires"])
880+
logger.debug("Cookie expires: %s", cookie[name]["expires"])
881881
return tuple(cookie.output().split(": ", 1))
882882

883883
# ----------------------------------------------------------------------------
@@ -940,7 +940,7 @@ def metadata(environ, start_response):
940940
start_response('200 OK', [('Content-Type', "text/xml")])
941941
return metadata
942942
except Exception as ex:
943-
logger.error("An error occured while creating metadata:" + ex.message)
943+
logger.error("An error occured while creating metadata: %s", ex.message)
944944
return not_found(environ, start_response)
945945

946946

@@ -959,7 +959,7 @@ def staticfile(environ, start_response):
959959
start_response('200 OK', [('Content-Type', "text/xml")])
960960
return open(path, 'r').read()
961961
except Exception as ex:
962-
logger.error("An error occured while creating metadata:" + ex.message)
962+
logger.error("An error occured while creating metadata: %s", ex.message)
963963
return not_found(environ, start_response)
964964

965965

@@ -971,9 +971,9 @@ def application(environ, start_response):
971971
the functions from above can access the url placeholders.
972972
973973
If nothing matches, call the `not_found` function.
974-
974+
975975
:param environ: The HTTP application environment
976-
:param start_response: The application to run when the handling of the
976+
:param start_response: The application to run when the handling of the
977977
request is done
978978
:return: The response as a list of lines
979979
"""
@@ -984,7 +984,7 @@ def application(environ, start_response):
984984
return metadata(environ, start_response)
985985

986986
kaka = environ.get("HTTP_COOKIE", None)
987-
logger.info("<application> PATH: %s" % path)
987+
logger.info("<application> PATH: %s", path)
988988

989989
if kaka:
990990
logger.info("= KAKA =")
@@ -994,7 +994,7 @@ def application(environ, start_response):
994994
else:
995995
try:
996996
query = parse_qs(environ["QUERY_STRING"])
997-
logger.debug("QUERY: %s" % query)
997+
logger.debug("QUERY: %s", query)
998998
user = IDP.cache.uid2user[query["id"][0]]
999999
except KeyError:
10001000
user = None
@@ -1013,7 +1013,7 @@ def application(environ, start_response):
10131013
except IndexError:
10141014
environ['myapp.url_args'] = path
10151015

1016-
logger.debug("Callback: %s" % (callback,))
1016+
logger.debug("Callback: %s", callback)
10171017
if isinstance(callback, tuple):
10181018
cls = callback[0](environ, start_response, user)
10191019
func = getattr(cls, callback[1])

0 commit comments

Comments
 (0)