Skip to content

[IMPROVEMENT] HTTP: keep guzzlehttp/psr7 inside the HTTP component - #12266

Open
chfsx wants to merge 5 commits into
ILIAS-eLearning:trunkfrom
srsolutionsag:improvement/12/http-psr7-wrapping
Open

chfsx wants to merge 5 commits into
ILIAS-eLearning:trunkfrom
srsolutionsag:improvement/12/http-psr7-wrapping

Conversation

@chfsx

@chfsx chfsx commented Oct 23, 2026

Copy link
Copy Markdown
Member

The PSR-7 implementation guzzlehttp/psr7 is meant to be an implementation detail of the HTTP component, but several components instantiated or typed against its classes directly (GuzzleHttp\Psr7\Response, ServerRequest, Uri, UploadedFile, ...). This PR removes all of these usages, so the library can be updated or replaced in HTTP alone, and adds a code rule that keeps it that way.

HTTP: MessageFactory

  • New ILIAS\HTTP\Message\MessageFactory with request(), response() and uri(), implemented by MessageFactoryImpl on top of Guzzle.
  • Defined and implemented in the bootstrap ($use[MessageFactory::class]), available to legacy code as $DIC->http()->messages() (new method on GlobalHttpState).
  • Documented in the README of HTTP.

Components

  • ilCtrl gets the MessageFactory injected instead of creating a Guzzle Response.
  • Test, TestQuestionPool, StudyProgramme, IndividualAssessment, ILIASObject: types changed to the Psr\Http\Message interfaces.
  • TestQuestionPool\RequestDataCollector no longer reads the private property $file of Guzzle's UploadedFile via Closure::bind(); it uses getStream()->getMetadata('uri'), the identifier FileUploadImpl uses for its results.
  • SettingsScoringGUI relayed the whole serialized ServerRequest through the hidden field of the recalculation confirmation and unserialized it with the Guzzle classes in allowed_classes. It now relays only the parsed body as JSON and restores it with withParsedBody(), so no objects are unserialized from client input any more.
  • CmiXapi (XapiProxy, XapiProxyRequest, XapiProxyResponse) creates requests and responses with the MessageFactory and types against the PSR interfaces. The commented-out createProxyRequest() is removed.
  • Tests use MessageFactoryImpl, the interfaces or stubs instead of Guzzle classes.

xAPI proxy in trunk

The xAPI proxy in trunk still had XapiProxyResponse::checkResponse() accessing $response['state'] on a Response object, which ends in Error: Cannot use object of type GuzzleHttp\Psr7\Response as array. The fix 60f8537 ("fix xAPI sending statements") exists on release_11 but never reached trunk, so it is cherry-picked (-x) as the first commit, unchanged. @Uwe-Kohnle could you confirm this is the intended state for trunk?

Code rule

New PHPStan code rule ilias.wrappedLibraryUsage (scripts/PHPStan/Rules/WrappedLibraries/WrappedLibraryUsageRule.php), part of the code-rules gate: a wrapped library may only be referenced inside the component that wraps it. The mapping is a constant in the rule, currently GuzzleHttp\Psr7 → HTTP; other wrapped libraries can be added there. Exemptions work like for the superglobal rule, with #[AllowWrappedLibraryUsage('reason', 13)] or an inline @phpstan-ignore ilias.wrappedLibraryUsage.v12, and expire with the next major. list_exemptions.sh knows the new attribute.

Testing

  • Build: [OK] All static artifacts are build!
  • PHPUnit: HTTP, DI, Init, UICore, UI (Table, Navigation), FileDelivery, StaticURL, Badge, Test, TestQuestionPool, CmiXapi, IndividualAssessment, StudyProgramme, ILIASObject pass.
  • Code rules gate on components/ILIAS: green; a planted new \GuzzleHttp\Psr7\Uri() makes it fail.
  • php-cs-fixer on the changed files: clean, except the lines of the cherry-picked commit, which are left as they are on release_11.
  • On an installation: redirect after an asynchronous file upload (ilCtrl), scoring settings with recalculation confirmation (confirm and cancel), images in matching questions, tile image in the common object settings, xAPI/cmi5 statements through the proxy.

@dsstrassner @kergomard for Test/TestQuestionPool, @thibsy for UICore, @Uwe-Kohnle for CmiXapi.

If there are no objections, I will merge this before Coding Completed.

@dsstrassner

Copy link
Copy Markdown
Contributor

@thojou please have a Look for T&A!

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