Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Follow-up to #2728815: Batch API uses request attributes instead of query in Symfony 3
Problem/Motivation
The semantics of Request::get() have changed in Symfony 3. Request attributes are now examined first before request query parameters.
Let's do it in 2 phases: commit this small patch first as soon as it passes to make progress with the Symfony 3 update. Phase 2: replace all remaining $request->get() usages.
Remains are
core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationContentEntity.php:94: $langcode = $request->get(static::QUERY_PARAMETER);
core/modules/search/src/Controller/SearchController.php:82: $keys = trim($request->get('keys'));
core/modules/system/src/Controller/ThemeController.php:63: $theme = $request->get('theme');
core/modules/system/src/Controller/ThemeController.php:105: $theme = $request->get('theme');
core/rebuild.php:41: ($request->get('token') && $request->get('timestamp') &&
core/rebuild.php:42: ((REQUEST_TIME - $request->get('timestamp')) < 300) &&
core/rebuild.php:43: Crypt::hashEquals(Crypt::hmacBase64($request->get('timestamp'), Settings::get('hash_salt')), $request->get('token'))
core/tests/Drupal/Tests/Core/StackMiddleware/NegotiationMiddlewareTest.php:104: $request->get('ajax_iframe_upload', FALSE)->shouldBeCalled();
core/tests/Drupal/Tests/Core/StackMiddleware/NegotiationMiddlewareTest.php:129: $request->get('ajax_iframe_upload', FALSE)->shouldBeCalled();
Proposed resolution
Explicitly call $request->query->get() in the Batch API to only access URL query parameters and never request attributes.
Remaining tasks
Patch.
User interface changes
None.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#6 | 2729247-6.patch | 6.63 KB | andypost |
#6 | interdiff.txt | 2.63 KB | andypost |
Comments
Comment #2
andypostLooks only
NegotiationMiddleware
tests should be fixed, let's see other failuresComment #4
andypostLooks we need functional test for
ajax_iframe_upload
Comment #5
jibranComment #6
andypostFix failed tests,
ajax_iframe_upload
passed asPOST
data in case ofiframe
uploads for old browsers so this testing out of scopeComment #7
klausiLooks good, tested that there are no remaining $request->get() usages left:
Yields no results with the patch. RTBC, assuming the testbot agrees.
Comment #9
klausiRandom test fail I guess? Back to RTBC.
Comment #10
catchCommitted/pushed to 8.2.x, thanks!