fix: make the CyberSource SDK work against the real urllib3 - #3984
Merged
Merged
Conversation
…ool managers cybersource-rest-client-python >= 0.0.70 calls urllib3.PoolManager/ProxyManager with `keepalive_delay` and `keepalive_idle_window`, kwargs that only exist in urllib3-future. Since a82ed9f stopped urllib3-future from clobbering the real urllib3, every live CyberSource REST call fails on its first request with: TypeError: PoolKey.__new__() got an unexpected keyword argument 'key_keepalive_delay' Real urllib3 swallows unknown kwargs into connection_pool_kw at construction time and only raises later, when _default_key_normalizer builds PoolKey, which is why nothing caught this at import or client-construction time. Rebind CyberSource.rest's module-global urllib3 to a proxy that drops kwargs the real urllib3 cannot take. The allow-list is derived from urllib3 itself (manager __init__ params plus PoolKey._fields) rather than hardcoded, so a future SDK release adding another urllib3-future-only kwarg degrades to a debug log instead of a 500. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OpenAPI ChangesShow/hide changesUnexpected changes? Ensure your branch is up-to-date with |
jkachel
approved these changes
Sep 18, 2026
jkachel
left a comment
Contributor
There was a problem hiding this comment.
LGTM 👍 - tested with CyberSource keys and worked fine (once I figured out what to do with the NaCl key).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
N/A
Description (What does it do?)
Fixes a 500 on every live CyberSource REST call - export compliance checks and refunds - that started with #3975.
The CyberSource SDK hands urllib3 two keep-alive options that only exist in
urllib3-future. Now that we're back on the real urllib3, it accepts them quietly at construction and then dies on the first actual request:New
main/cybersource_compat.pystrips those two kwargs, for the SDK only, applied fromRootConfig.ready(). Nothing else's urllib3 behavior changes.Worth noting for anyone who reads #3975: the "the SDK doesn't depend on anything urllib3-future-specific" call there was right about imports and wrong about the API. The SDK never imports urllib3-future, it just calls urllib3 with kwargs only urllib3-future accepts - which no amount of import-level checking would have surfaced.
Implementation details
CyberSource/rest.pyL85-L94 passeskeepalive_delayandkeepalive_idle_windowtourllib3.PoolManager(and the same pair toProxyManagerin the proxy branch). Real urllib3 funnels unknown kwargs intoconnection_pool_kwand only validates them later, when_default_key_normalizerbuildsPoolKey(**context)- so the failure lands on first use, not at import or client construction. That's why nothing flagged it.The fix rebinds
CyberSource.rest's module-globalurllib3to a proxy that filters the kwargs and delegates everything else. Patching the module attribute rather thanRESTClientObject.get_pool_managerkeeps the vendor logic (its sha256 pool-manager cache key, the proxy/no-proxy branch) intact - the method readshash_candidates_dict['keepalive_delay']by key, so you can't just pre-filter its input dict without reimplementing the whole thing and re-syncing it on every SDK bump.The allow-list is derived from urllib3 itself - manager
__init__params ∪PoolKey._fields- not hardcoded, so a future SDK release adding another future-only kwarg degrades to a debug log instead of a 500. Checked against the exact kwargs the SDK passes: keepsnum_pools,maxsize,cert_reqs,ca_certs,cert_file,key_file,key_password,proxy_url,proxy_headers; drops exactly the two keepalive ones. (APoolKey-only allow-list would wrongly dropnum_pools, which is aPoolManager.__init__param rather thanconnection_pool_kw- hence the union.)Scope:
CyberSource.restis the whole surface. Both calls there areurllib3.-qualified and all-keyword. The SDK's only otherPoolManager(...)call sites are inutilities/flex/PublicKeyApiController.pyandutilities/pgpBatchUpload/mutual_auth_upload.py; both pass only real-urllib3 kwargs and neither is on a path we use.Dropping the options is functionally safe - they tune urllib3-future's HTTP/2+ keep-alive pinging, and real urllib3 has no equivalent knob. The one behavioral consequence is that
maxKeepAliveDelay/maxKeepAliveIdleWindowin merchant config would now be ignored. Nothing in this repo sets them.Also corrected the comment in
shims/urllib3-future/pyproject.toml, since its "the SDK only ever doesimport urllib3" note is what made this look safe in the first place.How can this be tested?
New
main/cybersource_compat_test.py(6 tests) covers both manager branches, attribute delegation, idempotency, and one end-to-end case that drivesverify_user_with_exports()through the real SDK with onlyurlopenmocked. That last one is the interesting one:pytest-urllib3'surllib3_mockpatchesHTTPConnectionPool.urlopen, which sits belowPoolManager.connection_from_host, so the pool-key construction that was crashing still runs for real.Every existing compliance test mocks
get_cybersource_client, which is exactly why none of them caught this.What I ran locally: 6/6 new, 36/36 compliance, 956/956 ecommerce, ruff + ruff-format clean.
To see the bug itself, on
main:I also confirmed the new tests aren't vacuous by commenting out the
ready()call - 5 of 6 fail, the end-to-end one with the sameTypeErrorfrom the original traceback.Not covered: I haven't hit a real CyberSource sandbox endpoint. If someone wants belt-and-braces before this ships, an enrollment through
POST /api/v1/enrollments/withEXPORT_COMPLIANCE_CHECK_ENABLEDon and test credentials would exercise the original failing path.Additional Context
Upstream still does this on
master(through the current 0.0.79) at identical line numbers, so the shim needs to stay until CyberSource changes it.Pinning
cybersource-rest-client-python<0.0.70would also fix it and would make theshims/urllib3-futurestub unnecessary - 0.0.69 has no keepalive kwargs and requires plainurllib3. I didn't go that way: it's transitive viamitol-django-payment-gateway==2026.8.5, it pins us ten releases back on a payment SDK, and it'd fight the resolver on every payment-gateway bump. Happy to switch if you'd rather carry the pin than the shim.