From 42063b0d15e1a5189fec1488be78f49064857e70 Mon Sep 17 00:00:00 2001 From: Steve Kowalik Date: Tue, 1 Jun 2021 22:15:36 +1000 Subject: [PATCH] Do not transform requestHeaders when logging 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")