Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(files): Don't throw an error when guests access the controller #37826

Merged
merged 1 commit into from
Apr 20, 2023
Merged

fix(files): Don't throw an error when guests access the controller #37826

merged 1 commit into from
Apr 20, 2023

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Apr 19, 2023 •
edited by RuudschMaHinda
Loading

Summary

Our log was spammed quite heavily with the following log message:

OCA\\Files\\Service\\TagService::__construct(): Argument #4 ($homeFolder) must be of type OCP\\Files\\Folder, null given, called in …/apps/files/lib/AppInfo/Application.php on line 111

Before

$ curl -k https://localhost/index.php/apps/files/api/v1/stats
<!DOCTYPE html>
<html class="ng-csp" data-placeholder-focus="false" lang="en" data-locale="en" translate="no" >
	…
					<div class="guest-box wide">
	<h2>Internal Server Error</h2>
	<p>The server was unable to complete your request.</p>
	<p>If this happens again, please send the technical details below to the server administrator.</p>
	<p>More details can be found in the server log.</p>

	<h3>Technical details</h3>
	<ul>
		<li>Remote Address: …</li>
		<li>Request ID: ZD_9oDM7JyuXBrh8F7kaqQAAAAw</li>
					<li>Type: TypeError</li>
			<li>Code: 0</li>
			<li>Message: OCA\Files\Service\TagService::__construct(): Argument #4 ($homeFolder) must be of type OCP\Files\Folder, null given, called in …/apps/files/lib/AppInfo/Application.php on line 113</li>
			<li>File: …/apps/files/lib/Service/TagService.php</li>
			<li>Line: 52</li>
			</ul>

			<br />
		<h3>Trace</h3>
		<pre>#0 …/apps/files/lib/AppInfo/Application.php(113): OCA\Files\Service\TagService-&gt;__construct()
#1 …/lib/private/AppFramework/Utility/SimpleContainer.php(171): OCA\Files\AppInfo\Application-&gt;OCA\Files\AppInfo\{closure}()
#2 …/3rdparty/pimple/pimple/src/Pimple/Container.php(122): OC\AppFramework\Utility\SimpleContainer-&gt;OC\AppFramework\Utility\{closure}()
#3 …/lib/private/AppFramework/Utility/SimpleContainer.php(138): Pimple\Container-&gt;offsetGet()
#4 …/lib/private/AppFramework/DependencyInjection/DIContainer.php(488): OC\AppFramework\Utility\SimpleContainer-&gt;query()
#5 …/lib/private/AppFramework/DependencyInjection/DIContainer.php(466): OC\AppFramework\DependencyInjection\DIContainer-&gt;queryNoFallback()
#6 …/lib/private/AppFramework/Utility/SimpleContainer.php(65): OC\AppFramework\DependencyInjection\DIContainer-&gt;query()
#7 …/apps/files/lib/AppInfo/Application.php(91): OC\AppFramework\Utility\SimpleContainer-&gt;get()
#8 …/lib/private/AppFramework/Utility/SimpleContainer.php(171): OCA\Files\AppInfo\Application-&gt;OCA\Files\AppInfo\{closure}()
#9 …/3rdparty/pimple/pimple/src/Pimple/Container.php(122): OC\AppFramework\Utility\SimpleContainer-&gt;OC\AppFramework\Utility\{closure}()
#10 …/lib/private/AppFramework/Utility/SimpleContainer.php(138): Pimple\Container-&gt;offsetGet()
#11 …/lib/private/AppFramework/DependencyInjection/DIContainer.php(488): OC\AppFramework\Utility\SimpleContainer-&gt;query()
#12 …/lib/private/AppFramework/DependencyInjection/DIContainer.php(466): OC\AppFramework\DependencyInjection\DIContainer-&gt;queryNoFallback()
#13 …/lib/private/AppFramework/Utility/SimpleContainer.php(65): OC\AppFramework\DependencyInjection\DIContainer-&gt;query()
#14 …/lib/private/AppFramework/App.php(148): OC\AppFramework\Utility\SimpleContainer-&gt;get()
#15 …/lib/private/Route/Router.php(315): OC\AppFramework\App::main()
#16 …/lib/base.php(1059): OC\Route\Router-&gt;match()
#17 …/index.php(36): OC::handleRequest()
#18 {main}</pre>	</div>
…
</html>

After

$ curl -k https://localhost/index.php/apps/files/api/v1/stats
{"message":"Current user is not logged in"}

TODO

  • ...

Checklist

@nickvergessen
fix(files): Don't throw an error when guests access the controller
8b31d82 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen added this to the Nextcloud 27 milestone Apr 19, 2023
@nickvergessen nickvergessen self-assigned this Apr 19, 2023
@nickvergessen
Copy link
Member Author

/backport to stable26

@come-nc
Copy link
Contributor

come-nc commented Apr 20, 2023

Maybe add a comment explaining that this happens for guest users. Otherwise it’s always hard to guess if it is possible or not to get null somewhere.

@nickvergessen
Copy link
Member Author

Well actually I'm more thinking about preventing injection of the homeFolder and just use the rootFolder and then handling has to be done inside the controller methods, but there we always have a user, so shouldn't be an issue

@nickvergessen nickvergessen merged commit 590c202 into master Apr 20, 2023
@nickvergessen nickvergessen deleted the bugfix/noid/no-error-for-guests branch April 20, 2023 11:55
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Apr 20, 2023
Merged
1 task
@solracsf solracsf mentioned this pull request Apr 22, 2023
Closed
9 tasks
@nickvergessen nickvergessen mentioned this pull request Apr 24, 2023
Merged
4 tasks
@kesselb kesselb mentioned this pull request Apr 25, 2023
Closed
9 tasks
@solracsf solracsf mentioned this pull request May 23, 2023
Closed
9 tasks
@joshtrichards joshtrichards mentioned this pull request Oct 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artonge artonge artonge approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@come-nc come-nc come-nc approved these changes

Assignees

@nickvergessen nickvergessen

Labels
3. to review Waiting for reviews bug feature: files
Projects
None yet
Milestone
Nextcloud 27
Development

Successfully merging this pull request may close these issues.
4 participants
@nickvergessen @come-nc @artonge @skjnldsv