bravado
bravado copied to clipboard
Making requests_client.py's Authenticator URL matches() port-tolerant
Authenticator::matches()'s documentation advertises full host, scheme and port match, when it just checks for hosts. However when checking hosts, it cannot compare well a configured self.host:withPort (eg. localhost:8080) with the outgoing request full URL's host. Such a corner case can happen with this piece of code: http_client.set_basic_auth( 'localhost:8080', BASIC_AUTH_LOGIN, BASIC_AUTH_PASSWORD )
Same issue with the set_api_key() method. This makes authenticated bravado requests fail when unit testing on a local server hosted at localhost:8080..
Coverage remained the same at 98.499% when pulling 16cf8ea151d0b3e7f3377321bdba0e17ed03bbb9 on myselfhimself:myselfhimself-non-port-tolerant-authenticator-matches into d30b9bdd612d6dc282b18cc3ba9fd0e47b2dd74d on Yelp:master.
:arrow_up:
Hello, I am not working on this topic anymore and will not increase the code coverage of this PR sorry.
Le mar. 26 nov. 2019 à 08:38, Alexander Sapronov [email protected] a écrit :
⬆️
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Yelp/bravado/pull/384?email_source=notifications&email_token=AAJU5QX7RWNVQHGOFU2QGOLQVTG7PA5CNFSM4FKZPXCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFFA7MY#issuecomment-558501811, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJU5QX7Y3I5FSUQBA7DNH3QVTG7PANCNFSM4FKZPXCA .
This has been reworked in the meantime (in #444), and now compares host and port (correctly). Are you able to test with the latest master and confirm that it fixes the issue for you @myselfhimself ? I could then make a release that includes the change.
Hello, I am sorry I do not have access to the customer's codebase anymore and have changed working fields so will not assist in testing more for now :-/ :)
čt 6. 2. 2020 v 9:21 odesílatel Stephan Jaensch [email protected] napsal:
This has been reworked in the meantime (in #444 https://github.com/Yelp/bravado/pull/444), and now compares host and port (correctly). Are you able to test with the latest master and confirm that it fixes the issue for you @myselfhimself https://github.com/myselfhimself ? I could then make a release that includes the change.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Yelp/bravado/pull/384?email_source=notifications&email_token=AAJU5QUJIMBYSUSYIALZBL3RBPB7VA5CNFSM4FKZPXCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK6KL7Q#issuecomment-582788606, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJU5QURNZGVBU7G32SGS73RBPB7VANCNFSM4FKZPXCA .