From 1265747e992ba7d34a469b6b2f527809f8bf7067 Mon Sep 17 00:00:00 2001 From: Steve Kowalik Date: Wed, 2 Jun 2021 15:00:00 +1000 Subject: [PATCH] Do not transform requestHeaders when logging (#1965) Requester.__log() sanitizes the headers of the request so that authentication details are not logged, but this has the side effect of meaning that future requests that use the same Requester object will fail. Usually, this is perfectly fine, since almost every method will only make one request -- where this falls down is when we make another after a redirect. Make a copy of the requestHeaders, and sanitize those. Fixes #1959 --- github/Requester.py | 11 ++++++----- tests/Logging_.py | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/github/Requester.py b/github/Requester.py index 480186a990..bade682567 100644 --- a/github/Requester.py +++ b/github/Requester.py @@ -629,17 +629,18 @@ def __log(self, verb, url, requestHeaders, input, status, responseHeaders, outpu if self.__logger is None: self.__logger = logging.getLogger(__name__) if self.__logger.isEnabledFor(logging.DEBUG): + headersForRequest = requestHeaders.copy() if "Authorization" in requestHeaders: if requestHeaders["Authorization"].startswith("Basic"): - requestHeaders[ + headersForRequest[ "Authorization" ] = "Basic (login and password removed)" elif requestHeaders["Authorization"].startswith("token"): - requestHeaders["Authorization"] = "token (oauth token removed)" + headersForRequest["Authorization"] = "token (oauth token removed)" elif requestHeaders["Authorization"].startswith("Bearer"): - requestHeaders["Authorization"] = "Bearer (jwt removed)" + headersForRequest["Authorization"] = "Bearer (jwt removed)" else: # pragma no cover (Cannot happen, but could if we add an authentication method => be prepared) - requestHeaders[ + headersForRequest[ "Authorization" ] = "(unknown auth removed)" # pragma no cover (Cannot happen, but could if we add an authentication method => be prepared) self.__logger.debug( @@ -648,7 +649,7 @@ def __log(self, verb, url, requestHeaders, input, status, responseHeaders, outpu self.__scheme, self.__hostname, url, - requestHeaders, + headersForRequest, input, status, responseHeaders, diff --git a/tests/Logging_.py b/tests/Logging_.py index 3f277f618e..d8e4a519d3 100644 --- a/tests/Logging_.py +++ b/tests/Logging_.py @@ -198,3 +198,19 @@ def testLoggingWithBaseUrl(self): } output = '{"type":"User","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","created_at":"2010-07-09T06:10:06Z","email":"vincent@vincent-jacques.net","hireable":false,"avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","public_gists":3,"bio":"","following":29,"name":"Vincent Jacques","blog":"http://vincent-jacques.net","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"public_repos":13,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"}' self.assertLogging("GET", url, requestHeaders, responseHeaders, output) + + def testLoggingDoesNotModifyRequestHeaders(self): + # Recorded replay data already sanitizes Authorization headers, so we + # need to go under the covers + requestHeaders = {"Authorization": "thisisnotatoken"} + responseHeaders = {"status": "200 OK"} + github.Github()._Github__requester._Requester__log( + "GET", + "http://example.com", + requestHeaders, + None, + 200, + responseHeaders, + None, + ) + self.assertEqual(requestHeaders["Authorization"], "thisisnotatoken")