Skip to content

fix: make the CyberSource SDK work against the real urllib3 - #3984

Merged
rhysyngsun merged 1 commit into
mainfrom
fix/cybersource-urllib3-keepalive-kwargs
Sep 21, 2026
Merged

rhysyngsun merged 1 commit into
mainfrom
fix/cybersource-urllib3-keepalive-kwargs

Conversation

@rhysyngsun

Copy link
Copy Markdown
Contributor

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:

TypeError: PoolKey.__new__() got an unexpected keyword argument 'key_keepalive_delay'

New main/cybersource_compat.py strips those two kwargs, for the SDK only, applied from RootConfig.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.py L85-L94 passes keepalive_delay and keepalive_idle_window to urllib3.PoolManager (and the same pair to ProxyManager in the proxy branch). Real urllib3 funnels unknown kwargs into connection_pool_kw and only validates them later, when _default_key_normalizer builds PoolKey(**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-global urllib3 to a proxy that filters the kwargs and delegates everything else. Patching the module attribute rather than RESTClientObject.get_pool_manager keeps the vendor logic (its sha256 pool-manager cache key, the proxy/no-proxy branch) intact - the method reads hash_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: keeps num_pools, maxsize, cert_reqs, ca_certs, cert_file, key_file, key_password, proxy_url, proxy_headers; drops exactly the two keepalive ones. (A PoolKey-only allow-list would wrongly drop num_pools, which is a PoolManager.__init__ param rather than connection_pool_kw - hence the union.)

Scope: CyberSource.rest is the whole surface. Both calls there are urllib3.-qualified and all-keyword. The SDK's only other PoolManager(...) call sites are in utilities/flex/PublicKeyApiController.py and utilities/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 / maxKeepAliveIdleWindow in 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 does import 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 drives verify_user_with_exports() through the real SDK with only urlopen mocked. That last one is the interesting one: pytest-urllib3's urllib3_mock patches HTTPConnectionPool.urlopen, which sits below PoolManager.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.

docker compose run --rm web pytest main/cybersource_compat_test.py compliance/ -n0
docker compose run --rm web pytest ecommerce/ -n logical

What I ran locally: 6/6 new, 36/36 compliance, 956/956 ecommerce, ruff + ruff-format clean.

To see the bug itself, on main:

docker compose run --rm web python manage.py shell -c "
import CyberSource.rest as r
pm = r.urllib3.PoolManager(num_pools=4, maxsize=4, keepalive_delay=300, keepalive_idle_window=30)
pm.connection_from_url('https://apitest.cybersource.com')"

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 same TypeError from 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/ with EXPORT_COMPLIANCE_CHECK_ENABLED on 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.70 would also fix it and would make the shims/urllib3-future stub unnecessary - 0.0.69 has no keepalive kwargs and requires plain urllib3. I didn't go that way: it's transitive via mitol-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.

…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>
@github-actions

Copy link
Copy Markdown

OpenAPI Changes

Show/hide changes
## Changes for v0.yaml:
No changes detected

## Changes for v1.yaml:
No changes detected

## Changes for v2.yaml:
No changes detected

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

@jkachel jkachel self-assigned this Sep 18, 2026

@jkachel jkachel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 - tested with CyberSource keys and worked fine (once I figured out what to do with the NaCl key).

@rhysyngsun
rhysyngsun merged commit 73afd1a into main Sep 21, 2026
14 checks passed
@rhysyngsun
rhysyngsun deleted the fix/cybersource-urllib3-keepalive-kwargs branch September 21, 2026 14:11
@odlbot odlbot mentioned this pull request Sep 22, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants