Skip to content

Add http_client_options for per-provider HTTP timeout/proxy/verify#31

Merged
turegjorup merged 5 commits into
developfrom
feature/http-client-options
May 8, 2026
Merged

Add http_client_options for per-provider HTTP timeout/proxy/verify#31
turegjorup merged 5 commits into
developfrom
feature/http-client-options

Conversation

@turegjorup
Copy link
Copy Markdown
Contributor

Summary

  • New per-provider http_client_options block (timeout, proxy, verify) forwarded to the underlying Guzzle HTTP client used by league/oauth2-client
  • Closes the long-standing inability to bound HTTP requests to the IdP — slow IdPs no longer hang worker processes indefinitely
  • README adds a short note on why the stack uses Guzzle, not Symfony HttpClient (league/oauth2-client hard-types httpClient as GuzzleHttp\ClientInterface)

Backward compatibility

Additive only. No existing config keys, signatures, defaults, or dependencies change. Targets the next minor (4.2.0).

Test plan

  • task test — 51 tests pass
  • task analyze:php — phpstan max level, no errors
  • task lint — php-cs-fixer, composer normalize, markdown, yaml all clean
  • task test:matrix — all 6 combinations (PHP 8.3/8.4/8.5 × prefer-lowest/prefer-stable) pass
  • Manual smoke in a consumer app: set http_client_options.timeout: 0.001 and verify a ConnectException fires rather than a long hang

🤖 Generated with Claude Code

Forwards the league/oauth2-client-whitelisted Guzzle options (timeout,
proxy, verify) through bundle configuration. Closes the long-standing
inability to bound HTTP requests to the IdP.

Additive only — existing config continues to work unchanged. README adds
a short note explaining why the stack uses Guzzle rather than Symfony
HttpClient (league/oauth2-client hard-types its httpClient as
GuzzleHttp\ClientInterface).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (5e378cf) to head (f211885).

Additional details and impacted files
@@             Coverage Diff             @@
##             develop       #31   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity        56        57    +1     
===========================================
  Files              9         9           
  Lines            248       268   +20     
===========================================
+ Hits             248       268   +20     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/DependencyInjection/Configuration.php
Comment on lines +140 to +141
// Block must either be absent or empty so nothing leaks into Guzzle.
$this->assertSame([], $providerOptions['http_client_options'] ?? []);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If i understand the code modifications correctly this should never be empty but always absent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed assert, added comment

Comment thread README.md Outdated
Comment on lines +137 to +138
Only the keys whitelisted by `league/oauth2-client` are forwarded: `timeout`,
`proxy`, and `verify` (the last only when `proxy` is set).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I read this as: non-whitelisted keys are stripped. But, according to testHttpClientOptionsRejectsUnknownKey the configurations actually throws an error. Am i misunderstanding?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Attempted to clairy

turegjorup and others added 3 commits May 8, 2026 09:25
Drop the `?? []` fallback that masked the difference between an absent
key and an empty array. Per the schema (no defaultValue on the
arrayNode), an omitted input must produce no key at all.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous wording read as if league silently strips non-whitelisted
keys. The bundle config rejects them up front with
InvalidConfigurationException at container compile time.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@turegjorup turegjorup requested a review from jekuaitk May 8, 2026 09:18
@turegjorup turegjorup merged commit 2d3781c into develop May 8, 2026
15 checks passed
@turegjorup turegjorup deleted the feature/http-client-options branch May 8, 2026 13:16
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.

3 participants