Skip to content

Commit

Permalink
Fix logging without format string (#871)
Browse files Browse the repository at this point in the history
* Fix logging without format string
* Fix test to use Logrecord args instead of msg
  • Loading branch information
schlenk committed Feb 28, 2024
1 parent 37024ad commit 346f950
Show file tree
Hide file tree
Showing 13 changed files with 26 additions and 21 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@ The format is based on the [KeepAChangeLog] project.

## Unreleased

### Changed

- [#871]: Fix log messages without format string

### Removed

- [#870]: Remove Python 3.7

[#870]: https://github.com/CZ-NIC/pyoidc/pull/870
[#871]: https://github.com/CZ-NIC/pyoidc/pull/871

## 1.6.1 [2023-07-13]
- [#862] Fixed pydantic dependency
Expand Down
2 changes: 1 addition & 1 deletion src/oic/extension/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ def get_token_info(self, authn, req, endpoint):
try:
client_id = self.client_authn(self, req, authn)
except FailedAuthentication as err:
logger.error(err)
logger.error("%s", err)
error = TokenErrorResponse(
error="unauthorized_client", error_description="%s" % err
)
Expand Down
2 changes: 1 addition & 1 deletion src/oic/oauth2/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def http_request(self, url: str, method="GET", **kwargs) -> requests.Response:
try:
set_cookie(self.cookiejar, SimpleCookie(_cookie))
except CookieError as err:
logger.error(err)
logger.error("%s", err)
raise NonFatalException(r, "{}".format(err))
except (AttributeError, KeyError):
pass
Expand Down
2 changes: 1 addition & 1 deletion src/oic/oauth2/consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ def handle_authorization_response(self, query="", **kwargs):
AuthorizationResponse, info=query, sformat="urlencoded"
)
except Exception as err:
logger.error("%s" % err)
logger.error("%s", err)
raise

if isinstance(aresp, Message):
Expand Down
4 changes: 2 additions & 2 deletions src/oic/oauth2/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ def auth_init(self, request):
try:
areq = self.server.parse_authorization_request(query=request)
except (MissingRequiredValue, MissingRequiredAttribute, AuthzError) as err:
logger.debug("%s" % err)
logger.debug("%s", err)
areq = request_class()
areq.lax = True
if isinstance(request, dict):
Expand Down Expand Up @@ -1015,7 +1015,7 @@ def token_endpoint(self, request="", authn="", dtype="urlencoded", **kwargs):
try:
client_id = self.client_authn(self, areq, authn)
except (FailedAuthentication, AuthnFailure) as err:
logger.error(err)
logger.error("%s", err)
error = TokenErrorResponse(
error="unauthorized_client", error_description="%s" % err
)
Expand Down
4 changes: 2 additions & 2 deletions src/oic/oic/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ def handle_registration_info(self, response):
try:
resp.verify()
except oauth2.message.MissingRequiredAttribute as err:
logger.error(err)
logger.error("%s", err)
raise RegistrationError(err)
except Exception:
resp = ErrorResponse().deserialize(response.text, "json")
Expand Down Expand Up @@ -1677,7 +1677,7 @@ def parse_authorization_request(
except Exception as err:
if self.events:
self.events.store("Exception", err)
logger.error(err)
logger.error("%s", err)
raise

return _req
Expand Down
12 changes: 6 additions & 6 deletions src/oic/oic/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ def code_grant_type(self, areq):
try:
_tinfo = _sdb.upgrade_to_token(_access_code, issue_refresh=issue_refresh)
except AccessCodeUsed as err:
logger.error("%s" % err)
logger.error("%s", err)
# Should revoke the token issued to this access code
_sdb.revoke_all_tokens(_access_code)
return error_response("access_denied", descr="Access Code already used")
Expand All @@ -886,7 +886,7 @@ def code_grant_type(self, areq):
_info, client_info, areq, user_info=userinfo
)
except (JWEException, NoSuitableSigningKeys) as err:
logger.warning(str(err))
logger.warning("%s", err)
return error_response(
"invalid_request", descr="Could not sign/encrypt id_token"
)
Expand Down Expand Up @@ -934,7 +934,7 @@ def refresh_token_grant_type(self, areq):
_info, client_info, areq, user_info=userinfo
)
except (JWEException, NoSuitableSigningKeys) as err:
logger.warning(str(err))
logger.warning("%s", err)
return error_response(
"invalid_request", descr="Could not sign/encrypt id_token"
)
Expand Down Expand Up @@ -1376,7 +1376,7 @@ def _verify_sector_identifier(self, request):
try:
res = self.server.http_request(si_url)
except RequestException as err:
logger.error(err)
logger.error("%s", err)
res = None

if not res:
Expand Down Expand Up @@ -1798,7 +1798,7 @@ def create_authn_response(self, areq, sid):
_sinfo, client_info, areq, user_info=user_info, **hargs
)
except (JWEException, NoSuitableSigningKeys) as err:
logger.warning(str(err))
logger.warning("%s", err)
return error_response(
"invalid_request", descr="Could not sign/encrypt id_token"
)
Expand Down Expand Up @@ -1935,7 +1935,7 @@ def verify_post_logout_redirect_uri(
return redirect_uri
except Exception as exc:
msg = "An error occurred while verifying redirect URI: %s"
logger.debug(msg, str(exc))
logger.debug(msg, exc)

return None

Expand Down
2 changes: 1 addition & 1 deletion src/oic/utils/authn/saml.py
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ def _redirect_to_auth(self, _cli, entity_id, query, end_point_index, vorg_name="

logger.debug("ht_args: %s" % ht_args)
except Exception as exc:
logger.exception(exc)
logger.exception("%s", exc)
raise ServiceErrorException(
"Failed to construct the AuthnRequest: %s" % exc
)
Expand Down
2 changes: 1 addition & 1 deletion src/oic/utils/http_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ def wsgi_wrapper(environ, start_response, func, **kwargs):
resp = args
return resp(environ, start_response)
except Exception as err:
logger.error("%s" % err)
logger.error("%s", err)
raise


Expand Down
2 changes: 1 addition & 1 deletion src/oic/utils/keyio.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ def do_remote(self):
logger.debug("KeyBundle fetch keys from: %s", self.source)
r = requests.get(self.source, timeout=self.timeout, **args)
except Exception as err:
logger.error(err)
logger.error("%s", err)
raise_exception(UpdateFailed, REMOTE_FAILED.format(self.source, str(err)))

if r.status_code == 304: # file has not changed
Expand Down
2 changes: 1 addition & 1 deletion src/oic/utils/rp/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def _do_code(self, response, authresp):
msg = "Access token response: {}"
logger.info(msg.format(sanitize(atresp)))
except Exception as err:
logger.error("%s" % err)
logger.error("%s", err)
raise

if isinstance(atresp, ErrorResponse):
Expand Down
2 changes: 1 addition & 1 deletion src/oic/utils/rp/oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def callback(self, response, session, format="dict"):
)
logger.info("Access token response: {}".format(sanitize(atresp)))
except Exception as err:
logger.error("%s" % err)
logger.error("%s", err)
raise

if isinstance(atresp, ErrorResponse):
Expand Down
6 changes: 3 additions & 3 deletions tests/test_oic_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -1349,12 +1349,12 @@ def test_verify_sector_identifier_no_scheme(self):

assert len(logcap.records) == 2
# First log record is from server...
assert isinstance(logcap.records[1].msg, MissingSchema)
assert isinstance(logcap.records[1].args[0], MissingSchema)
error = (
"Invalid URL 'example.com': No scheme supplied. Perhaps you meant "
"https://example.com?"
)
assert str(logcap.records[1].msg) == error
assert logcap.records[1].getMessage() == error

def test_verify_sector_identifier_nonreachable(self):
rr = RegistrationRequest(
Expand Down Expand Up @@ -1386,7 +1386,7 @@ def test_verify_sector_identifier_error(self):

assert len(logcap.records) == 2
# First log record is from server...
assert logcap.records[1].msg == error
assert logcap.records[1].args[0] == error

def test_verify_sector_identifier_malformed(self):
rr = RegistrationRequest(
Expand Down

0 comments on commit 346f950

Please sign in to comment.