diff --git a/changelog/unreleased/41741 b/changelog/unreleased/41741 new file mode 100644 index 000000000000..09c8670e8f41 --- /dev/null +++ b/changelog/unreleased/41741 @@ -0,0 +1,26 @@ +Bugfix: Route requests for .php urls through the front controller + +Legacy ajax endpoints returned an HTTP 500 with a PHP fatal error +'Class "OC" not found' instead of a response. Changing the language in the +personal settings was the most visible case, but the share dialog's email +autocomplete, public link thumbnails, trashbin previews and the Google Drive +OAuth flow were affected in the same way. + +Those endpoints are legacy routes whose url is also a real script on disk, for +example /settings/ajax/setlanguage.php. The front controller rewrite rules in +.htaccess only forwarded a request to index.php when the requested path did not +exist, so the web server executed such a script directly - that is without the +bootstrap index.php would have done - and the request died on the first +statement. The rewrite rules now always route .php urls through the front +controller, while static assets keep being served directly and the exempted +entry points (status.php, remote.php, public.php, cron.php, ocs/v1.php, +ocs/v2.php, core/ajax/update.php) keep bypassing the router. + +Existing installations pick up the new rules on the next upgrade, or by running +'occ maintenance:update:htaccess'. Independently of that, the personal language +endpoint is now registered as /settings/personal/changelanguage, matching the +sibling changepassword route, and the personal profile panel builds the url with +OC.generateUrl() instead of posting to a relative script path. + +https://github.com/owncloud/core/issues/41740 +https://github.com/owncloud/core/pull/41741 diff --git a/lib/private/Setup.php b/lib/private/Setup.php index 0d79276ab7aa..58ee356e003c 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -498,6 +498,12 @@ public static function updateHtaccess(\OCP\IConfig $config): void { $content .= "\n RewriteRule ^favicon.ico$ core/img/favicon.ico [L]"; $content .= "\n RewriteRule ^core/js/oc.js$ index.php [PT,E=PATH_INFO:$1]"; $content .= "\n RewriteRule ^core/preview.png$ index.php [PT,E=PATH_INFO:$1]"; + // Requests for .php urls have to reach the front controller as well. + // A couple of legacy routes declare a url which is also a real script + // on disk - e.g. /settings/ajax/setlanguage.php - and without this the + // -f check below lets the web server execute those scripts directly, + // that is without the bootstrap index.php would have done. + $content .= "\n RewriteCond %{REQUEST_URI} \\.php$ [OR]"; $content .= "\n RewriteCond %{REQUEST_FILENAME} !-f"; $content .= "\n RewriteCond %{REQUEST_URI} !^$rewriteBaseRe/core/img/favicon\\.ico$"; $content .= "\n RewriteCond %{REQUEST_URI} !^$rewriteBaseRe/robots\\.txt$"; diff --git a/settings/js/panels/profile.js b/settings/js/panels/profile.js index 6c181a3d5f87..a2c25e7b4dd6 100644 --- a/settings/js/panels/profile.js +++ b/settings/js/panels/profile.js @@ -213,7 +213,7 @@ $(document).ready(function () { // Serialize the data var post = $("#languageinput").serialize(); // Ajax foo - $.post('ajax/setlanguage.php', post, function (data) { + $.post(OC.generateUrl('/settings/personal/changelanguage'), post, function (data) { if (data.status === "success") { location.reload(); } diff --git a/settings/routes.php b/settings/routes.php index 21868b4f339f..cbe06da50da8 100644 --- a/settings/routes.php +++ b/settings/routes.php @@ -104,6 +104,12 @@ $this->create('settings_personal_changepassword', '/settings/personal/changepassword') ->post() ->action('OC\Settings\ChangePassword\Controller', 'changePersonalPassword'); +$this->create('settings_personal_changelanguage', '/settings/personal/changelanguage') + ->post() + ->actionInclude('settings/ajax/setlanguage.php'); +// Kept for backwards compatibility only. This url is shadowed by the script of +// the same name on disk, so the front controller rewrite never reaches it - see +// settings_personal_changelanguage above. $this->create('settings_ajax_setlanguage', '/settings/ajax/setlanguage.php') ->actionInclude('settings/ajax/setlanguage.php'); // apps diff --git a/tests/lib/Route/RouteShadowingTest.php b/tests/lib/Route/RouteShadowingTest.php new file mode 100644 index 000000000000..e1de1fed64ec --- /dev/null +++ b/tests/lib/Route/RouteShadowingTest.php @@ -0,0 +1,92 @@ + + * + * @copyright Copyright (c) 2026, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace Test\Route; + +/** + * The front controller rewrite in .htaccess only forwards a request to + * index.php when the requested path does NOT exist on disk: + * + * RewriteCond %{REQUEST_FILENAME} !-f + * RewriteRule . index.php [PT,E=PATH_INFO:$1] + * + * A route whose url maps onto a real .php file is therefore unreachable + * through the router - the web server executes that script directly, without + * the bootstrap that base.php/index.php would have done, and the request dies + * with 'Class "OC" not found'. See the personal language endpoint, which used + * to be posted to as /settings/ajax/setlanguage.php. + * + * This guards the endpoints the web ui posts to against regressing into that + * shape again. + */ +class RouteShadowingTest extends \Test\TestCase { + /** + * Urls the js of the web ui posts to, which therefore have to survive the + * front controller rewrite. + */ + public function providesUiEndpoints() { + return [ + 'personal language' => ['/settings/personal/changelanguage'], + 'personal password' => ['/settings/personal/changepassword'], + ]; + } + + /** + * @dataProvider providesUiEndpoints + * @param string $url + */ + public function testUiEndpointIsNotShadowedByAFileOnDisk($url) { + $path = \OC::$SERVERROOT . '/' . \ltrim($url, '/'); + + $this->assertFileDoesNotExist( + $path, + "The route $url is shadowed by a real file on disk. The front " . + 'controller rewrite skips existing files, so this endpoint would ' . + 'be executed without a bootstrap and fail with "Class OC not found".' + ); + } + + public function testPersonalLanguageRouteIsRegistered() { + $routes = \file_get_contents(\OC::$SERVERROOT . '/settings/routes.php'); + + $this->assertStringContainsString( + '/settings/personal/changelanguage', + $routes, + 'The personal language endpoint must be routed through the front controller.' + ); + } + + public function testPersonalProfileJsDoesNotPostToARelativeScriptPath() { + $js = \file_get_contents(\OC::$SERVERROOT . '/settings/js/panels/profile.js'); + + $this->assertStringNotContainsString( + "'ajax/setlanguage.php'", + $js, + 'The language selector must build its url with OC.generateUrl() so ' . + 'the request is routed instead of hitting the script on disk.' + ); + $this->assertStringContainsString( + "OC.generateUrl('/settings/personal/changelanguage')", + $js, + 'The language selector must post to the routed endpoint.' + ); + } +} diff --git a/tests/lib/SetupTest.php b/tests/lib/SetupTest.php index 77625549bd5d..f8bbed907721 100644 --- a/tests/lib/SetupTest.php +++ b/tests/lib/SetupTest.php @@ -218,7 +218,7 @@ public function testUpdateHtaccess(): void { ); } - public function testUpdateHtaccessWithRewriteBaseUsesFileExistenceCheck(): void { + public function testUpdateHtaccessWithRewriteBaseUsesFileExistenceCheckAndRoutesPhpUrls(): void { $origServerRoot = \OC::$SERVERROOT; $htaccessFile = \OC::$SERVERROOT . '/tests/data/.htaccess'; \touch($htaccessFile); @@ -256,5 +256,14 @@ public function testUpdateHtaccessWithRewriteBaseUsesFileExistenceCheck(): void 'REQUEST_URI} !\.(css|js|svg|gif|png|html|ttf|woff|ico|jpg|jpeg|json|properties)', $content ); + // .php urls have to be routed even when the script exists on disk, + // otherwise the web server runs it without a bootstrap + $phpCond = 'RewriteCond %{REQUEST_URI} \.php$ [OR]'; + $this->assertStringContainsString($phpCond, $content); + $this->assertLessThan( + \strpos($content, 'RewriteCond %{REQUEST_FILENAME} !-f'), + \strpos($content, $phpCond), + 'The .php condition has to precede the file existence check to be OR-ed with it' + ); } }