NC20: Music app Ampache API broken because URL parameter 'action' can't be used

How to use GitHub

Please use the 👍 reaction to show that you are affected by the same issue.
Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
Subscribe to receive notifications on status change and new comments.

Steps to reproduce

Call any REST API endpoint and append GET query argument action=somecontent, e.g. https://domain.org/nextcloud/index.php/apps/someapp/api/somemethod?action=somecontent
Try to use the argument action in the controller class either through function argument injection public function someMethod($action) or by calling $this->request->getParam('action') or $this->request->getParams()['action'] or $this->request['action'].

Expected behaviour

The controller should be able to obtain the value somecontent of the URL argument action.

Actual behaviour

The value of the argument action is empty, no matter which method you use to obtain it. Arguments with any other name work as expected, but the name action is somehow special. Also, if you call an API endpoint without supplying the command line argument action, the array returned by $this->request->getParams() still contains the key action with an empty content.

This oddity prevents using the Ampache API of the Music app on Nextcloud 20, as reported in nc-music/oc-music#791. This issue is new in NC20, there has never been such problem on the NC versions 12 - 19.

(Side note: The Music app version 0.17.0 with support for NC20 has just been released.)

Server configuration

Operating system: Raspbian Linux 4.9.35-v7+ armv7l

Web server: nginx/1.6.2

Database: sqlite3 3.8.7.1

PHP version: 7.4

Nextcloud version: Nextcloud 20.0.0 RC2

Updated from an older Nextcloud/ownCloud or fresh install: fresh

Where did you install Nextcloud from: https://nextcloud.com/install/

Signing status:

Signing status

No errors have been found.

List of activated apps:

App list

Enabled:
  - accessibility: 1.6.0
  - activity: 2.13.1
  - bruteforcesettings: 2.0.1
  - cloud_federation_api: 1.3.0
  - comments: 1.10.0
  - contactsinteraction: 1.1.0
  - dashboard: 7.0.0
  - dav: 1.16.0
  - federatedfilesharing: 1.10.1
  - federation: 1.10.1
  - files: 1.15.0
  - files_pdfviewer: 2.0.1
  - files_rightclick: 0.17.0
  - files_sharing: 1.12.0
  - files_trashbin: 1.10.1
  - files_versions: 1.13.0
  - files_videoplayer: 1.9.0
  - firstrunwizard: 2.9.0
  - logreader: 2.5.0
  - lookup_server_connector: 1.8.0
  - music: 0.16.1-alpha1
  - nextcloud_announcements: 1.9.0
  - notifications: 2.8.0
  - oauth2: 1.8.0
  - password_policy: 1.10.1
  - photos: 1.2.0
  - privacy: 1.4.0
  - provisioning_api: 1.10.0
  - recommendations: 0.8.0
  - serverinfo: 1.10.0
  - settings: 1.2.0
  - sharebymail: 1.10.0
  - support: 1.3.0
  - survey_client: 1.8.0
  - systemtags: 1.10.0
  - text: 3.1.0
  - theming: 1.11.0
  - twofactor_backupcodes: 1.9.0
  - updatenotification: 1.10.0
  - user_status: 1.0.0
  - viewer: 1.4.0
  - weather_status: 1.0.0
  - workflowengine: 2.2.0
Disabled:
  - admin_audit
  - encryption
  - files_external
  - user_ldap

Nextcloud configuration:

Config report

{
    "system": {
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "***REMOVED SENSITIVE VALUE***",
            "***REMOVED SENSITIVE VALUE***",
            "***REMOVED SENSITIVE VALUE***"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "sqlite3",
        "version": "20.0.0.8",
        "overwrite.cli.url": "https:\/\/***REMOVED SENSITIVE VALUE***\/nextcloud-20",
        "installed": true,
        "loglevel": 1
    }
}

Are you using external storage, if yes which one: no

Are you using encryption: no

Are you using an external user-backend, if yes which one: no

Client configuration

Browser: N/A

Operating system: N/A

If I knew how to fix this I would 🙁
Pls fix <3

So does anyone have any idea, why is the URL argument action special for Nextcloud 20? And what has changed on this regard between NC19 and NC20?

@rullzer

I'm almost certain that this bug has been introduced as a side effect of the commit edc1c77. Before the commit, the execution went through this branch:

server/lib/private/Route/Router.php

Lines 291 to 297 in cd7a627

    
           if (isset($parameters['action'])) { 
        
           	$action = $parameters['action']; 
        
           	if (!is_callable($action)) { 
        
           		throw new \Exception('not a callable action'); 
        
           	} 
        
           	unset($parameters['action']); 
        
           	call_user_func($action, $parameters);

...where the key 'action' is removed from the $parameters returned by the UrlMatcher.

After the commit, this branch is executed instead:

server/lib/private/Route/Router.php

Lines 297 to 301 in f7fa292

    
           if (isset($parameters['caller'])) { 
        
           	$caller = $parameters['caller']; 
        
           	unset($parameters['caller']); 
        
           	$application = $this->getApplicationClass($caller[0]); 
        
           	\OC\AppFramework\App::main($caller[1], $caller[2], $application->getContainer(), $parameters);

Here, the $parameters contains the key 'action' with null value, and it is passed straight through to App::main which passes it to the Request object. Then, the array_merge here overwrites any URL arguments or POST parameters named 'action' with the null-valued key:

server/lib/private/AppFramework/Http/Request.php

Lines 169 to 175 in cd7a627

    
           public function setUrlParameters(array $parameters) { 
        
           	$this->items['urlParams'] = $parameters; 
        
           	$this->items['parameters'] = array_merge( 
        
           		$this->items['parameters'], 
        
           		$this->items['urlParams'] 
        
           	); 
        
           }

What do you think, @rullzer?

To add to my previous comment, it seems that the bug can be fixed by adding the call

unset($parameters['action']);

near the line

server/lib/private/Route/Router.php

Line 299 in f7fa292

unset($parameters['caller']);

But as I lack the knowledge of the big picture of the Nextcloud routing, I'm not certain if this is the best way to fix this, or if this could have some side effects on some case. But so far I have noticed nothing wrong when I made this experimental one line fix on my installation.

Hi Devs,

I would like to upvote this issue too.
Unfortunately it breaks Power Ampache app.

Thanks.

Hi Devs,

It works with Nextcloud 20.0.5.

Thanks!

Run behat tests with two different NC and PHP versions on github actions

Navigation Menu

Repository navigation

NC20: Music app Ampache API broken because URL parameter 'action' can't be used #23544

How to use GitHub

Steps to reproduce

Expected behaviour

Actual behaviour

Server configuration

Client configuration

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (isset($parameters['action'])) {
	$action = $parameters['action'];
	if (!is_callable($action)) {
	throw new \Exception('not a callable action');
	}
	unset($parameters['action']);
	call_user_func($action, $parameters);

	if (isset($parameters['caller'])) {
	$caller = $parameters['caller'];
	unset($parameters['caller']);
	$application = $this->getApplicationClass($caller[0]);
	\OC\AppFramework\App::main($caller[1], $caller[2], $application->getContainer(), $parameters);

	public function setUrlParameters(array $parameters) {
	$this->items['urlParams'] = $parameters;
	$this->items['parameters'] = array_merge(
	$this->items['parameters'],
	$this->items['urlParams']
	);
	}

NC20: Music app Ampache API broken because URL parameter 'action' can't be used #23544

Description

How to use GitHub

Steps to reproduce

Expected behaviour

Actual behaviour

Server configuration

Client configuration

Activity

mariusurbelis commented on Nov 8, 2020

paulijar commented on Nov 12, 2020

paulijar commented on Nov 29, 2020

paulijar commented on Nov 30, 2020

florom commented on Dec 16, 2020

florom commented on Jan 15, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions