Allow unverified SSL and additional HTTP headers #89
Conversation
| ssl._create_default_https_context = ssl._create_unverified_context | ||
|
|
||
| if len(extra_headers) > 0: | ||
| self.transport._headers += extra_headers |
There was a problem hiding this comment.
This possibly overrides an existing value set for this attribute.
You can pass headers in Transport.__init__() so better add this in lines 128 & 130.
| url, | ||
| allow_unverified_ssl: bool = False, | ||
| extra_headers: list[tuple[str, str]] = [], | ||
| ): |
There was a problem hiding this comment.
Will have to define these for the inherited class TCMSKerbXmlrpc, and take care to pass them down appropriately otherwise we have broken method signatures in the inheritance chain.
| @@ -123,6 +132,12 @@ def __init__(self, username, password, url): | |||
| else: | |||
| raise RuntimeError("Unrecognized URL scheme") | |||
|
|
|||
| if allow_unverified_ssl: | |||
| ssl._create_default_https_context = ssl._create_unverified_context | |||
There was a problem hiding this comment.
This is modifying the global state of the ssl module which potentially can lead to side effects depending on how that module and the tcms_api module are used by the developer. This could lead to very difficult to debug issues.
This also affects the SSL module even if for whatever reason the user is connecting to a plain-text endpoint (e.g. reusing the same code base).
Instead of doing it like this, see the context argument of SafeTransport.__init__().
|
Thank you for your review and comments. I'll try to make your suggestions and will let you know if I have any questions. |
Hi, thank you for creating this API client and Kiwi TCMS itself.
I've added two new features that I've found useful:
Proxy-Authorizationheader).The changes should be backwards compatible due to setting default values for these new parameters.
Please let me know if there is anything you'd like to change though before you'd consider merging.