diff --git a/apps/files/appinfo/routes.php b/apps/files/appinfo/routes.php index 4ebcfe819f91..a780096f67d3 100644 --- a/apps/files/appinfo/routes.php +++ b/apps/files/appinfo/routes.php @@ -66,11 +66,11 @@ /** @var $this \OC\Route\Router */ -$this->create('files_ajax_download', 'ajax/download.php') +$this->create('files_ajax_download', 'ajax/download') ->actionInclude('files/ajax/download.php'); -$this->create('files_ajax_getstoragestats', 'ajax/getstoragestats.php') +$this->create('files_ajax_getstoragestats', 'ajax/getstoragestats') ->actionInclude('files/ajax/getstoragestats.php'); -$this->create('files_ajax_list', 'ajax/list.php') +$this->create('files_ajax_list', 'ajax/list') ->actionInclude('files/ajax/list.php'); $this->create('download', 'download{file}') diff --git a/apps/files/js/files.js b/apps/files/js/files.js index 72eda238b9f1..1f9b8d8f74c5 100644 --- a/apps/files/js/files.js +++ b/apps/files/js/files.js @@ -25,7 +25,7 @@ state.call.abort(); } state.dir = currentDir; - state.call = $.getJSON(OC.filePath('files','ajax','getstoragestats.php') + '?dir=' + encodeURIComponent(currentDir),function(response) { + state.call = $.getJSON(OC.generateUrl('/apps/files/ajax/getstoragestats') + '?dir=' + encodeURIComponent(currentDir),function(response) { state.dir = null; state.call = null; Files.updateMaxUploadFilesize(response); @@ -200,7 +200,7 @@ if (params) { q = '?' + OC.buildQueryString(params); } - return OC.filePath('files', 'ajax', action + '.php') + q; + return OC.generateUrl('/apps/files/ajax/' + action) + q; }, /** diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 6c51aced6fcb..2aafe1ac0746 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -1684,11 +1684,11 @@ describe('OCA.Files.FileList tests', function() { }); it('returns correct download URL for multiple files', function() { expect(fileList.getDownloadUrl(['a b c.txt', 'd e f.txt'])) - .toEqual(OC.webroot + '/index.php/apps/files/ajax/download.php?dir=%2Fsubdir&files[]=a%20b%20c.txt&files[]=d%20e%20f.txt'); + .toEqual(OC.webroot + '/index.php/apps/files/ajax/download?dir=%2Fsubdir&files[]=a%20b%20c.txt&files[]=d%20e%20f.txt'); }); it('returns the correct ajax URL', function() { expect(fileList.getAjaxUrl('test', {a:1, b:'x y'})) - .toEqual(OC.webroot + '/index.php/apps/files/ajax/test.php?a=1&b=x%20y'); + .toEqual(OC.webroot + '/index.php/apps/files/ajax/test?a=1&b=x%20y'); }); }); describe('Upload Url', function() { @@ -2083,7 +2083,7 @@ describe('OCA.Files.FileList tests', function() { it('Opens download URL when clicking "Download"', function() { $('.selectedActions .download').click(); expect(redirectStub.calledOnce).toEqual(true); - expect(redirectStub.getCall(0).args[0]).toContain(OC.webroot + '/index.php/apps/files/ajax/download.php?' + + expect(redirectStub.getCall(0).args[0]).toContain(OC.webroot + '/index.php/apps/files/ajax/download?' + 'dir=%2Fsubdir&files[]=One.txt&files[]=Three.pdf&files[]=somedir'); redirectStub.restore(); }); @@ -2092,13 +2092,13 @@ describe('OCA.Files.FileList tests', function() { $('.select-all').click(); $('.selectedActions .download').click(); expect(redirectStub.calledOnce).toEqual(true); - expect(redirectStub.getCall(0).args[0]).toContain(OC.webroot + '/index.php/apps/files/ajax/download.php?dir=%2F&files='); + expect(redirectStub.getCall(0).args[0]).toContain(OC.webroot + '/index.php/apps/files/ajax/download?dir=%2F&files='); }); it('Downloads parent folder when all selected in subfolder', function() { $('.select-all').click(); $('.selectedActions .download').click(); expect(redirectStub.calledOnce).toEqual(true); - expect(redirectStub.getCall(0).args[0]).toContain(OC.webroot + '/index.php/apps/files/ajax/download.php?dir=%2F&files=subdir'); + expect(redirectStub.getCall(0).args[0]).toContain(OC.webroot + '/index.php/apps/files/ajax/download?dir=%2F&files=subdir'); }); }); describe('Delete', function() { diff --git a/apps/files/tests/js/filesSpec.js b/apps/files/tests/js/filesSpec.js index 3a7684c946d8..6c6519940706 100644 --- a/apps/files/tests/js/filesSpec.js +++ b/apps/files/tests/js/filesSpec.js @@ -95,7 +95,7 @@ describe('OCA.Files.Files tests', function() { }); it('returns the ajax download URL when multiple files specified', function() { var url = Files.getDownloadUrl(['test file.txt', 'abc.txt'], '/subdir'); - expect(url).toEqual(OC.webroot + '/index.php/apps/files/ajax/download.php?dir=%2Fsubdir&files[]=test%20file.txt&files[]=abc.txt'); + expect(url).toEqual(OC.webroot + '/index.php/apps/files/ajax/download?dir=%2Fsubdir&files[]=test%20file.txt&files[]=abc.txt'); }); }); describe('handleDownload', function() { diff --git a/apps/files_external/appinfo/routes.php b/apps/files_external/appinfo/routes.php index 01c325c8acdf..25df921529d3 100644 --- a/apps/files_external/appinfo/routes.php +++ b/apps/files_external/appinfo/routes.php @@ -62,9 +62,7 @@ ] ); -$this->create('files_external_oauth1', 'ajax/oauth1.php') - ->actionInclude('files_external/ajax/oauth1.php'); -$this->create('files_external_oauth2', 'ajax/oauth2.php') +$this->create('files_external_oauth2', 'ajax/oauth2') ->actionInclude('files_external/ajax/oauth2.php'); $this->create('files_external_list_applicable', '/applicable') diff --git a/apps/files_external/js/gdrive.js b/apps/files_external/js/gdrive.js index 333c906c992b..2dff045ae494 100644 --- a/apps/files_external/js/gdrive.js +++ b/apps/files_external/js/gdrive.js @@ -1,6 +1,6 @@ $(document).ready(function() { var backendId = 'googledrive'; - var backendUrl = OC.generateUrl('/index.php/apps/files_external/ajax/oauth2.php'); + var backendUrl = OC.generateUrl('/apps/files_external/ajax/oauth2'); function generateUrl($tr) { // no mapping between client ID and Google 'project', so we always load the same URL diff --git a/apps/files_sharing/appinfo/routes.php b/apps/files_sharing/appinfo/routes.php index a9a32030ad1f..75fe0c0122f3 100644 --- a/apps/files_sharing/appinfo/routes.php +++ b/apps/files_sharing/appinfo/routes.php @@ -144,9 +144,7 @@ function () { } ); -$this->create('files_sharing_ajax_list', 'ajax/list.php') - ->actionInclude('files_sharing/ajax/list.php'); -$this->create('files_sharing_ajax_publicpreview', 'ajax/publicpreview.php') +$this->create('files_sharing_ajax_publicpreview', 'ajax/publicpreview') ->actionInclude('files_sharing/ajax/publicpreview.php'); $this->create('sharing_external_shareinfo', '/shareinfo') ->actionInclude('files_sharing/ajax/shareinfo.php'); diff --git a/apps/files_sharing/js/public.js b/apps/files_sharing/js/public.js index ba09beaffb81..06986ecd0494 100644 --- a/apps/files_sharing/js/public.js +++ b/apps/files_sharing/js/public.js @@ -154,14 +154,14 @@ OCA.Sharing.PublicApp = { } else if ((previewSupported === 'true' && mimetype.substr(0, mimetype.indexOf('/')) !== 'video') || mimetype.substr(0, mimetype.indexOf('/')) === 'image' && mimetype !== 'image/svg+xml') { - img.attr('src', OC.filePath('files_sharing', 'ajax', 'publicpreview.php') + '?' + OC.buildQueryString(params)); + img.attr('src', OC.generateUrl('/apps/files_sharing/ajax/publicpreview') + '?' + OC.buildQueryString(params)); img.appendTo('#imgframe'); } else if (mimetype.substr(0, mimetype.indexOf('/')) !== 'video') { img.attr('src', OC.Util.replaceSVGIcon(mimetypeIcon)); img.attr('width', 128); img.appendTo('#imgframe'); } else if (previewSupported === 'true') { - $('#imgframe > video').attr('poster', OC.filePath('files_sharing', 'ajax', 'publicpreview.php') + '?' + OC.buildQueryString(params)); + $('#imgframe > video').attr('poster', OC.generateUrl('/apps/files_sharing/ajax/publicpreview') + '?' + OC.buildQueryString(params)); } if (this.fileList) { @@ -208,7 +208,7 @@ OCA.Sharing.PublicApp = { this.fileList.getAjaxUrl = function (action, params) { params = params || {}; params.t = token; - return OC.filePath('files_sharing', 'ajax', action + '.php') + '?' + OC.buildQueryString(params); + return OC.generateUrl('/apps/files_sharing/ajax/' + action) + '?' + OC.buildQueryString(params); }; this.fileList.linkTo = function (dir) { @@ -228,7 +228,7 @@ OCA.Sharing.PublicApp = { urlSpec.x = Math.ceil(urlSpec.x); urlSpec.y = Math.ceil(urlSpec.y); urlSpec.t = $('#dirToken').val(); - return OC.generateUrl('/index.php/apps/files_sharing/ajax/publicpreview.php') + '?' + $.param(urlSpec); + return OC.generateUrl('/apps/files_sharing/ajax/publicpreview') + '?' + $.param(urlSpec); }; this.fileList.updateEmptyContent = function () { diff --git a/apps/files_sharing/tests/js/publicAppSpec.js b/apps/files_sharing/tests/js/publicAppSpec.js index 87437bc029a2..f9a6ad3b3e7f 100644 --- a/apps/files_sharing/tests/js/publicAppSpec.js +++ b/apps/files_sharing/tests/js/publicAppSpec.js @@ -123,7 +123,7 @@ describe('OCA.Sharing.PublicApp tests', function() { }); it('returns the correct ajax URL', function() { expect(fileList.getAjaxUrl('test', {a:1, b:'x y'})) - .toEqual(OC.getRootPath() + '/index.php/apps/files_sharing/ajax/test.php?a=1&b=x%20y&t=sh4tok'); + .toEqual(OC.getRootPath() + '/index.php/apps/files_sharing/ajax/test?a=1&b=x%20y&t=sh4tok'); }); it('returns correct download URL for downloading everything', function() { expect(fileList.getDownloadUrl()) diff --git a/apps/files_trashbin/appinfo/routes.php b/apps/files_trashbin/appinfo/routes.php index 9fa407e65031..9b8b92cd6112 100644 --- a/apps/files_trashbin/appinfo/routes.php +++ b/apps/files_trashbin/appinfo/routes.php @@ -25,13 +25,13 @@ $application = new Application(); -$this->create('core_ajax_trashbin_preview', 'ajax/preview.php') +$this->create('core_ajax_trashbin_preview', 'ajax/preview') ->actionInclude('files_trashbin/ajax/preview.php'); -$this->create('files_trashbin_ajax_delete', 'ajax/delete.php') +$this->create('files_trashbin_ajax_delete', 'ajax/delete') ->actionInclude('files_trashbin/ajax/delete.php'); -$this->create('files_trashbin_ajax_isEmpty', 'ajax/isEmpty.php') +$this->create('files_trashbin_ajax_isEmpty', 'ajax/isEmpty') ->actionInclude('files_trashbin/ajax/isEmpty.php'); -$this->create('files_trashbin_ajax_list', 'ajax/list.php') +$this->create('files_trashbin_ajax_list', 'ajax/list') ->actionInclude('files_trashbin/ajax/list.php'); -$this->create('files_trashbin_ajax_undelete', 'ajax/undelete.php') +$this->create('files_trashbin_ajax_undelete', 'ajax/undelete') ->actionInclude('files_trashbin/ajax/undelete.php'); diff --git a/apps/files_trashbin/js/app.js b/apps/files_trashbin/js/app.js index ccfde24ea064..6086f7ac0e10 100644 --- a/apps/files_trashbin/js/app.js +++ b/apps/files_trashbin/js/app.js @@ -57,7 +57,7 @@ OCA.Trashbin.App = { var deleteAction = tr.children("td.date").children(".action.delete"); deleteAction.removeClass('icon-delete').addClass('icon-loading-small'); fileList.disableActions(); - $.post(OC.filePath('files_trashbin', 'ajax', 'undelete.php'), { + $.post(OC.generateUrl('/apps/files_trashbin/ajax/undelete'), { files: JSON.stringify([filename]), dir: fileList.getCurrentDirectory() }, @@ -86,7 +86,7 @@ OCA.Trashbin.App = { var deleteAction = tr.children("td.date").children(".action.delete"); deleteAction.removeClass('icon-delete').addClass('icon-loading-small'); fileList.disableActions(); - $.post(OC.filePath('files_trashbin', 'ajax', 'delete.php'), { + $.post(OC.generateUrl('/apps/files_trashbin/ajax/delete'), { files: JSON.stringify([filename]), dir: fileList.getCurrentDirectory() }, diff --git a/apps/files_trashbin/js/filelist.js b/apps/files_trashbin/js/filelist.js index 12ae829538c1..7c95edef71cc 100644 --- a/apps/files_trashbin/js/filelist.js +++ b/apps/files_trashbin/js/filelist.js @@ -113,7 +113,7 @@ if (params) { q = '?' + OC.buildQueryString(params); } - return OC.filePath('files_trashbin', 'ajax', action + '.php') + q; + return OC.generateUrl('/apps/files_trashbin/ajax/' + action) + q; }, setupUploadEvents: function() { @@ -205,7 +205,7 @@ }; } - $.post(OC.filePath('files_trashbin', 'ajax', 'undelete.php'), + $.post(OC.generateUrl('/apps/files_trashbin/ajax/undelete'), params, function(result) { if (allFiles) { @@ -255,7 +255,7 @@ } } - $.post(OC.filePath('files_trashbin', 'ajax', 'delete.php'), + $.post(OC.generateUrl('/apps/files_trashbin/ajax/delete'), params, function(result) { if (allFiles) { @@ -275,7 +275,7 @@ }, generatePreviewUrl: function(urlSpec) { - return OC.generateUrl('/index.php/apps/files_trashbin/ajax/preview.php') + '?' + $.param(urlSpec); + return OC.generateUrl('/apps/files_trashbin/ajax/preview') + '?' + $.param(urlSpec); }, getDownloadUrl: function() { diff --git a/apps/files_trashbin/tests/js/filelistSpec.js b/apps/files_trashbin/tests/js/filelistSpec.js index 277c3e2811b3..042aa70f946b 100644 --- a/apps/files_trashbin/tests/js/filelistSpec.js +++ b/apps/files_trashbin/tests/js/filelistSpec.js @@ -121,7 +121,7 @@ describe('OCA.Trashbin.FileList tests', function() { permissions: 1 } }; - fakeServer.respondWith(/\/index\.php\/apps\/files_trashbin\/ajax\/list.php\?dir=%2Fsubdir/, [ + fakeServer.respondWith(/\/index\.php\/apps\/files_trashbin\/ajax\/list\?dir=%2Fsubdir/, [ 200, { "Content-Type": "application/json" }, @@ -266,7 +266,7 @@ describe('OCA.Trashbin.FileList tests', function() { $('.selectedActions .delete-selected').click(); expect(fakeServer.requests.length).toEqual(1); request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/delete.php'); + expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/delete'); expect(OC.parseQueryString(request.requestBody)) .toEqual({'dir': '/', files: '["One.txt.d11111","Three.pdf.d33333","somedir.d99999"]'}); fakeServer.requests[0].respond( @@ -296,7 +296,7 @@ describe('OCA.Trashbin.FileList tests', function() { $('.selectedActions .delete-selected').click(); expect(fakeServer.requests.length).toEqual(1); request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/delete.php'); + expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/delete'); expect(OC.parseQueryString(request.requestBody)) .toEqual({'dir': '/', files: '["One.txt.d11111","somedir.d99999"]'}); fakeServer.requests[0].respond( @@ -323,7 +323,7 @@ describe('OCA.Trashbin.FileList tests', function() { $('.selectedActions .delete-selected').click(); expect(fakeServer.requests.length).toEqual(1); request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/delete.php'); + expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/delete'); expect(OC.parseQueryString(request.requestBody)) .toEqual({'dir': '/', allfiles: 'true'}); fakeServer.requests[0].respond( @@ -340,7 +340,7 @@ describe('OCA.Trashbin.FileList tests', function() { $('.selectedActions .undelete').click(); expect(fakeServer.requests.length).toEqual(1); request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/undelete.php'); + expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/undelete'); expect(OC.parseQueryString(request.requestBody)) .toEqual({'dir': '/', files: '["One.txt.d11111","Three.pdf.d33333","somedir.d99999"]'}); fakeServer.requests[0].respond( @@ -370,7 +370,7 @@ describe('OCA.Trashbin.FileList tests', function() { $('.selectedActions .undelete').click(); expect(fakeServer.requests.length).toEqual(1); request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/undelete.php'); + expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/undelete'); expect(OC.parseQueryString(request.requestBody)) .toEqual({'dir': '/', files: '["One.txt.d11111","somedir.d99999"]'}); fakeServer.requests[0].respond( @@ -397,7 +397,7 @@ describe('OCA.Trashbin.FileList tests', function() { $('.selectedActions .undelete').click(); expect(fakeServer.requests.length).toEqual(1); request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/undelete.php'); + expect(request.url).toEqual(OC.webroot + '/index.php/apps/files_trashbin/ajax/undelete'); expect(OC.parseQueryString(request.requestBody)) .toEqual({'dir': '/', allfiles: 'true'}); fakeServer.requests[0].respond( diff --git a/changelog/unreleased/41742 b/changelog/unreleased/41742 new file mode 100644 index 000000000000..3ec6581d6de5 --- /dev/null +++ b/changelog/unreleased/41742 @@ -0,0 +1,23 @@ +Bugfix: Rewire legacy ajax routes to extension-free urls + +The front controller rewrite only forwards a request to index.php when the +requested path does not exist on disk. A legacy route whose declared url was +itself a real file - for example /settings/ajax/setlanguage.php - therefore +never reached the router: the web server executed the script directly, without +the bootstrap index.php would have performed, and the request died with +`Class "OC" not found` (HTTP 500). Changing the personal language setting, the +share dialog e-mail lookup, the trashbin and public link preview thumbnails and +the Google Drive OAuth entry point were affected. + +Every such route url has lost its .php suffix so that no route url resolves to +a file on disk any more, and the affected javascript callers now build routed +urls through OC.generateUrl(). Route names are unchanged, so linkToRoute() +callers keep working. The legacy .php urls are gone and no alias is provided - +an alias would re-introduce the very shadowing this change removes. +/core/ajax/update.php keeps its suffix by design: it bootstraps itself and is +excluded from the rewrite. Five routes whose include target no longer existed +were removed. Third-party apps posting to their own shadowed .php route urls +need the same treatment. + +https://github.com/owncloud/core/issues/41740 +https://github.com/owncloud/core/pull/41742 diff --git a/core/js/js.js b/core/js/js.js index e8c51a37b60a..c416f3cb1400 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -43,7 +43,7 @@ function escapeHTML(s) { * @deprecated use Files.getDownloadURL() instead */ function fileDownloadPath(dir, file) { - return OC.filePath('files', 'ajax', 'download.php') + '?files=' + encodeURIComponent(file) + '&dir=' + encodeURIComponent(dir); + return OC.generateUrl('/apps/files/ajax/download') + '?files=' + encodeURIComponent(file) + '&dir=' + encodeURIComponent(dir); } /** @namespace */ diff --git a/core/js/oc-dialogs.js b/core/js/oc-dialogs.js index 74f5fbe0b4e2..6b564fb41177 100644 --- a/core/js/oc-dialogs.js +++ b/core/js/oc-dialogs.js @@ -745,7 +745,7 @@ var OCdialogs = { } return $.getJSON( - OC.filePath('files', 'ajax', 'list.php'), + OC.generateUrl('/apps/files/ajax/list'), { dir: dir, mimetypes: JSON.stringify(mimeType) diff --git a/core/js/share.js b/core/js/share.js index d35331cfc0ac..798302cb85c6 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -299,7 +299,7 @@ OC.Share = _.extend(OC.Share || {}, { } else { var checkShares = true; } - $.ajax({type: 'GET', url: OC.filePath('core', 'ajax', 'share.php'), data: { fetch: 'getItem', itemType: itemType, itemSource: itemSource, checkReshare: checkReshare, checkShares: checkShares }, async: async, success: function(result) { + $.ajax({type: 'GET', url: OC.generateUrl('/core/ajax/share'), data: { fetch: 'getItem', itemType: itemType, itemSource: itemSource, checkReshare: checkReshare, checkShares: checkShares }, async: async, success: function(result) { if (result && result.status === 'success') { data = result.data; } else { @@ -327,7 +327,7 @@ OC.Share = _.extend(OC.Share || {}, { ); } - return $.post(OC.filePath('core', 'ajax', 'share.php'), + return $.post(OC.generateUrl('/core/ajax/share'), { action: 'share', itemType: itemType, @@ -357,7 +357,7 @@ OC.Share = _.extend(OC.Share || {}, { ); }, unshare:function(itemType, itemSource, shareType, shareWith, callback) { - $.post(OC.filePath('core', 'ajax', 'share.php'), { action: 'unshare', itemType: itemType, itemSource: itemSource, shareType: shareType, shareWith: shareWith }, function(result) { + $.post(OC.generateUrl('/core/ajax/share'), { action: 'unshare', itemType: itemType, itemSource: itemSource, shareType: shareType, shareWith: shareWith }, function(result) { if (result && result.status === 'success') { if (callback) { callback(); diff --git a/core/js/sharedialogmailview.js b/core/js/sharedialogmailview.js index d7fa155c9474..490fca0d1455 100644 --- a/core/js/sharedialogmailview.js +++ b/core/js/sharedialogmailview.js @@ -208,7 +208,7 @@ if (this.xhr != null) this.xhr.abort(); - var xhr = $.get(OC.generateUrl('/index.php/core/ajax/share.php'), { + var xhr = $.get(OC.generateUrl('/core/ajax/share'), { 'fetch' : 'getShareWithEmail', 'search': query.term }).done(function(result) { diff --git a/core/routes.php b/core/routes.php index b381a64ae4cc..c3e6c464c532 100644 --- a/core/routes.php +++ b/core/routes.php @@ -87,15 +87,18 @@ $this->create('search_ajax_search', '/core/search') ->actionInclude('core/search/ajax/search.php'); // AppConfig -$this->create('core_ajax_appconfig', '/core/ajax/appconfig.php') +$this->create('core_ajax_appconfig', '/core/ajax/appconfig') ->actionInclude('core/ajax/appconfig.php'); // Share -$this->create('core_ajax_share', '/core/ajax/share.php') +$this->create('core_ajax_share', '/core/ajax/share') ->actionInclude('core/ajax/share.php'); // oC JS config $this->create('js_config', '/core/js/oc.js') ->actionInclude('core/js/config.php'); // Routing +// This url intentionally keeps the .php suffix: update.php bootstraps itself and +// is excluded from the front controller rewrite, see \OC\Setup::updateHtaccess() +// and OC::checkMaintenanceMode(). $this->create('core_ajax_update', '/core/ajax/update.php') ->actionInclude('core/ajax/update.php'); diff --git a/settings/js/admin-apps.js b/settings/js/admin-apps.js index 56603ecaf922..5134752f9db9 100644 --- a/settings/js/admin-apps.js +++ b/settings/js/admin-apps.js @@ -270,7 +270,7 @@ OC.Settings.Apps = OC.Settings.Apps || { var appItem = $('div#app-'+appId+''); element.val(t('settings','Please wait....')); if(active && !groups.length) { - $.post(OC.filePath('settings','ajax','disableapp.php'),{appid:appId},function(result) { + $.post(OC.generateUrl('/settings/ajax/disableapp'),{appid:appId},function(result) { if(!result || result.status !== 'success') { if (result.data && result.data.message) { OC.Settings.Apps.showErrorMessage(appId, result.data.message); @@ -296,7 +296,7 @@ OC.Settings.Apps = OC.Settings.Apps || { } else { // TODO: display message to admin to not refresh the page! // TODO: lock UI to prevent further operations - $.post(OC.filePath('settings','ajax','enableapp.php'),{appid: appId, groups: groups},function(result) { + $.post(OC.generateUrl('/settings/ajax/enableapp'),{appid: appId, groups: groups},function(result) { if(!result || result.status !== 'success') { if (result.data && result.data.message) { OC.Settings.Apps.showErrorMessage(appId, result.data.message); @@ -369,30 +369,10 @@ OC.Settings.Apps = OC.Settings.Apps || { } }, - updateApp:function(appId, element) { - var oldButtonText = element.val(); - element.val(t('settings','Updating....')); - OC.Settings.Apps.hideErrorMessage(appId); - $.post(OC.filePath('settings','ajax','updateapp.php'),{appid:appId},function(result) { - if(!result || result.status !== 'success') { - if (result.data && result.data.message) { - OC.Settings.Apps.showErrorMessage(appId, result.data.message); - } else { - OC.Settings.Apps.showErrorMessage(appId, t('settings','Error while updating app')); - } - element.val(oldButtonText); - } - else { - element.val(t('settings','Updated')); - element.hide(); - } - },'json'); - }, - uninstallApp:function(appId, element) { OC.Settings.Apps.hideErrorMessage(appId); element.val(t('settings','Uninstalling ....')); - $.post(OC.filePath('settings','ajax','uninstallapp.php'),{appid:appId},function(result) { + $.post(OC.generateUrl('/settings/ajax/uninstallapp'),{appid:appId},function(result) { if(!result || result.status !== 'success') { OC.Settings.Apps.showErrorMessage(appId, t('settings','Error while uninstalling app')); element.val(t('settings','Uninstall')); @@ -406,7 +386,7 @@ OC.Settings.Apps = OC.Settings.Apps || { }, rebuildNavigation: function() { - $.getJSON(OC.filePath('settings', 'ajax', 'navigationdetect.php')).done(function(response){ + $.getJSON(OC.generateUrl('/settings/ajax/navigationdetect')).done(function(response){ if(response.status === 'success'){ var idsToKeep = {}; var navEntries=response.nav_entries; @@ -621,13 +601,6 @@ OC.Settings.Apps = OC.Settings.Apps || { OC.Settings.Apps.uninstallApp(appId, element); }); - $(document).on('click', '#apps-list input.update', function () { - var appId = $(this).data('appid'); - var element = $(this); - - OC.Settings.Apps.updateApp(appId, element); - }); - $(document).on('change', '#group_select', function() { var element = $(this).parent().find('input.enable'); var groups = $(this).val(); diff --git a/settings/js/panels/profile.js b/settings/js/panels/profile.js index 22c9426f1103..9e8ef552596f 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(OC.generateUrl('/index.php/settings/ajax/setlanguage.php'), post, function (data) { + $.post(OC.generateUrl('/settings/ajax/setlanguage'), post, function (data) { if (data.status === "success") { location.reload(); } diff --git a/settings/js/users/users.js b/settings/js/users/users.js index 968173332d83..73864a1ae64e 100644 --- a/settings/js/users/users.js +++ b/settings/js/users/users.js @@ -437,7 +437,7 @@ var UserList = { return false; } $.post( - OC.filePath('settings', 'ajax', 'togglegroups.php'), + OC.generateUrl('/settings/ajax/togglegroups'), { username: user, group: group @@ -486,7 +486,7 @@ var UserList = { return false; } $.post( - OC.filePath('settings', 'ajax', 'togglesubadmins.php'), + OC.generateUrl('/settings/ajax/togglesubadmins'), { username: user, group: group @@ -549,7 +549,7 @@ var UserList = { */ _updateQuota: function(uid, quota, ready) { $.post( - OC.filePath('settings', 'ajax', 'setquota.php'), + OC.generateUrl('/settings/ajax/setquota'), {username: uid, quota: quota}, function (result) { if (ready) { diff --git a/settings/routes.php b/settings/routes.php index 21868b4f339f..75b425751562 100644 --- a/settings/routes.php +++ b/settings/routes.php @@ -89,34 +89,27 @@ // Settings ajax actions // users -$this->create('settings_ajax_setquota', '/settings/ajax/setquota.php') +$this->create('settings_ajax_setquota', '/settings/ajax/setquota') ->actionInclude('settings/ajax/setquota.php'); -$this->create('settings_ajax_togglegroups', '/settings/ajax/togglegroups.php') +$this->create('settings_ajax_togglegroups', '/settings/ajax/togglegroups') ->actionInclude('settings/ajax/togglegroups.php'); -$this->create('settings_ajax_togglesubadmins', '/settings/ajax/togglesubadmins.php') +$this->create('settings_ajax_togglesubadmins', '/settings/ajax/togglesubadmins') ->actionInclude('settings/ajax/togglesubadmins.php'); $this->create('settings_users_changepassword', '/settings/users/changepassword') ->post() ->action('OC\Settings\ChangePassword\Controller', 'changeUserPassword'); -$this->create('settings_ajax_changegroupname', '/settings/ajax/changegroupname.php') - ->actionInclude('settings/ajax/changegroupname.php'); // personal $this->create('settings_personal_changepassword', '/settings/personal/changepassword') ->post() ->action('OC\Settings\ChangePassword\Controller', 'changePersonalPassword'); -$this->create('settings_ajax_setlanguage', '/settings/ajax/setlanguage.php') +$this->create('settings_ajax_setlanguage', '/settings/ajax/setlanguage') ->actionInclude('settings/ajax/setlanguage.php'); // apps -$this->create('settings_ajax_enableapp', '/settings/ajax/enableapp.php') +$this->create('settings_ajax_enableapp', '/settings/ajax/enableapp') ->actionInclude('settings/ajax/enableapp.php'); -$this->create('settings_ajax_disableapp', '/settings/ajax/disableapp.php') +$this->create('settings_ajax_disableapp', '/settings/ajax/disableapp') ->actionInclude('settings/ajax/disableapp.php'); -$this->create('settings_ajax_updateapp', '/settings/ajax/updateapp.php') - ->actionInclude('settings/ajax/updateapp.php'); -$this->create('settings_ajax_uninstallapp', '/settings/ajax/uninstallapp.php') +$this->create('settings_ajax_uninstallapp', '/settings/ajax/uninstallapp') ->actionInclude('settings/ajax/uninstallapp.php'); -$this->create('settings_ajax_navigationdetect', '/settings/ajax/navigationdetect.php') +$this->create('settings_ajax_navigationdetect', '/settings/ajax/navigationdetect') ->actionInclude('settings/ajax/navigationdetect.php'); -// admin -$this->create('settings_ajax_excludegroups', '/settings/ajax/excludegroups.php') - ->actionInclude('settings/ajax/excludegroups.php'); diff --git a/tests/acceptance/features/bootstrap/Sharing.php b/tests/acceptance/features/bootstrap/Sharing.php index f1bd50755761..32b1507eb5f6 100644 --- a/tests/acceptance/features/bootstrap/Sharing.php +++ b/tests/acceptance/features/bootstrap/Sharing.php @@ -3484,7 +3484,7 @@ private function getAllSharesSharedWithUser(string $user, ?string $state = "all" */ public function getPublicPreviewOfFile(string $fileName, string $token):void { $url = $this->getBaseUrl() . - "/index.php/apps/files_sharing/ajax/publicpreview.php" . + "/index.php/apps/files_sharing/ajax/publicpreview" . "?file=$fileName&t=$token"; $resp = HttpRequestHelper::get( $url, diff --git a/tests/lib/Route/RouteShadowingTest.php b/tests/lib/Route/RouteShadowingTest.php new file mode 100644 index 000000000000..49d25546660a --- /dev/null +++ b/tests/lib/Route/RouteShadowingTest.php @@ -0,0 +1,223 @@ + + * + * @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 file is therefore unreachable through the + * router - the web server executes that script directly, without the bootstrap + * base.php/index.php would have done, and the request dies with + * 'Class "OC" not found'. That is why route urls must not carry a .php suffix + * that matches the script they include, e.g. /settings/ajax/setlanguage for + * settings/ajax/setlanguage.php. + */ +class RouteShadowingTest extends \Test\TestCase { + /** + * core/ajax/update.php bootstraps itself, is excluded from the rewrite in + * \OC\Setup::updateHtaccess() and is requested by its real path in + * core/js/update.js - so it is allowed to be served from disk. + */ + private const ALLOWED_TO_BE_SERVED_FROM_DISK = [ + '/core/ajax/update.php', + ]; + + /** + * Every routes file shipped with core, as + * [ => [, ]]. + * + * @return array + */ + public function providesRoutingFiles(): array { + $files = [ + 'core' => ['core/routes.php', null], + 'settings' => ['settings/routes.php', null], + ]; + foreach (\glob(\OC::$SERVERROOT . '/apps/*/appinfo/routes.php') as $file) { + $app = \basename(\dirname(\dirname($file))); + $files[$app] = ['apps/' . $app . '/appinfo/routes.php', $app]; + } + return $files; + } + + /** + * @dataProvider providesRoutingFiles + * @param string $routesFile + * @param string|null $app + */ + public function testNoRouteUrlIsShadowedByAFileOnDisk($routesFile, $app): void { + $shadowed = []; + foreach ($this->getRouteUrls($routesFile, $app) as $url) { + if (\in_array($url, self::ALLOWED_TO_BE_SERVED_FROM_DISK, true)) { + continue; + } + if (\is_file(\OC::$SERVERROOT . $url)) { + $shadowed[] = $url; + } + } + + $this->assertSame( + [], + $shadowed, + "$routesFile declares route urls which are real files on disk. The " . + 'front controller rewrite skips existing files, so these endpoints ' . + 'would be executed without a bootstrap and fail with "Class OC not ' . + 'found". Drop the file extension from the url - the include target ' . + 'stays as it is.' + ); + } + + /** + * A route which includes a script that no longer exists is dead weight and + * responds with a fatal error rather than a 404. + * + * @dataProvider providesRoutingFiles + * @param string $routesFile + * @param string|null $app + */ + public function testEveryActionIncludeTargetExists($routesFile, $app): void { + $missing = []; + foreach ($this->getIncludeTargets($routesFile) as $target) { + if ($this->resolveOnIncludePath($target) === null) { + $missing[] = $target; + } + } + + $this->assertSame( + [], + $missing, + "$routesFile includes scripts which do not exist any more. Remove " . + 'the route together with the callers of it.' + ); + } + + /** + * Urls the js of the web ui posts to, which therefore have to survive the + * front controller rewrite. + */ + public function providesUiEndpoints(): array { + return [ + 'personal language' => ['/settings/ajax/setlanguage'], + 'personal password' => ['/settings/personal/changepassword'], + 'share' => ['/core/ajax/share'], + 'trashbin preview' => ['/apps/files_trashbin/ajax/preview'], + 'public preview' => ['/apps/files_sharing/ajax/publicpreview'], + ]; + } + + /** + * @dataProvider providesUiEndpoints + * @param string $url + */ + public function testUiEndpointIsNotShadowedByAFileOnDisk($url): void { + $this->assertFileDoesNotExist( + \OC::$SERVERROOT . $url, + "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 testPersonalProfileJsDoesNotPostToARelativeScriptPath(): void { + $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/ajax/setlanguage')", + $js, + 'The language selector must post to the routed endpoint.' + ); + } + + /** + * All urls declared in a routes file, normalized the way the router does: + * a relative url of an app route is prefixed with /apps/, see + * \OC\Route\Router::loadRoutes(). + * + * @param string $routesFile + * @param string|null $app + * @return string[] + */ + private function getRouteUrls($routesFile, $app): array { + $content = \file_get_contents(\OC::$SERVERROOT . '/' . $routesFile); + + $urls = []; + // $this->create('name', 'url') + \preg_match_all( + "/create\\(\\s*'[^']+'\\s*,\\s*'([^']*)'/", + $content, + $matches + ); + $urls = $matches[1]; + // 'url' => '...' of the registerRoutes()/AppFramework style declarations + \preg_match_all("/'url'\\s*=>\\s*'([^']*)'/", $content, $matches); + $urls = \array_merge($urls, $matches[1]); + + return \array_map(function ($url) use ($app) { + $url = '/' . \ltrim($url, '/'); + if ($app !== null) { + $url = '/apps/' . $app . $url; + } + return $url; + }, $urls); + } + + /** + * @param string $routesFile + * @return string[] + */ + private function getIncludeTargets($routesFile): array { + $content = \file_get_contents(\OC::$SERVERROOT . '/' . $routesFile); + \preg_match_all("/actionInclude\\(\\s*'([^']+)'/", $content, $matches); + return $matches[1]; + } + + /** + * actionInclude() targets are required relative to the include path, which + * holds the server root and every configured apps directory, see + * OC::initPaths(). + * + * @param string $target + * @return string|null + */ + private function resolveOnIncludePath($target): ?string { + $roots = [\OC::$SERVERROOT]; + foreach (\OC::$APPSROOTS as $appsRoot) { + $roots[] = $appsRoot['path']; + } + foreach ($roots as $root) { + if (\is_file($root . '/' . $target)) { + return $root . '/' . $target; + } + } + return null; + } +}