Skip to content

Conversation

@come-nc
Copy link
Contributor

@come-nc come-nc commented Apr 27, 2023

Summary

In File::getContent, which must return a string, throw an Exception instead of returning false.

This avoids some TypeError that I’ve seen reported recently, either through SimpleFile which has a strong return type, or through a call to getimagesizefromstring which expects a string.

Checklist

In File::getContent, which must return a string, throw an Exception
 instead of returning false.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc added bug 3. to review Waiting for reviews labels Apr 27, 2023
@come-nc come-nc added this to the Nextcloud 27 milestone Apr 27, 2023
@come-nc come-nc self-assigned this Apr 27, 2023
@come-nc come-nc added the pending documentation This pull request needs an associated documentation update label Apr 27, 2023
@come-nc come-nc requested review from a team, ArtificialOwl, ChristophWurst, blizzz, icewind1991 and nickvergessen and removed request for a team April 27, 2023 07:59
@blizzz blizzz added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels May 3, 2023
@blizzz blizzz merged commit 997efe4 into master May 3, 2023
@blizzz blizzz deleted the fix/fix-getcontent-return-type branch May 3, 2023 08:34
@DaphneMuller
Copy link

Hello!
Thank you for your work on this! I noticed that your PR still has the label 'documentation'. I'm not sure if you already had a look at the documentation for your PR, but at Nextcloud we strive to document changes that affect other app developers or other admins before the release takes place, and I'm pinging you with the hope to get this done.

Are you familiar with our documentation process already?
Changes that affect app developers should be documented here:
https://docs.nextcloud.com/server/27/developer_manual/app_publishing_maintenance/app_upgrade_guide/upgrade_to_27.html

Changes that affect administrators should be documented here:
https://docs.nextcloud.com/server/latest/admin_manual/release_notes/upgrade_to_27.html

While I understand sometimes it's handy to merge fast to get your changes in, it would be great if next time you could try to add the documentation before merging. You can find more information on that here:
https://docs.nextcloud.com/server/27/developer_manual/prologue/compatibility_app_ecosystem.html#documentation-procedures-of-changes-that-affect-app-developers

If all your documentation efforts are done, please remove the label 'documentation' and check the checkbox in your opening note and I'll stop bugging you :)

Many thanks in advance and thanks again for your work!

@come-nc come-nc removed the pending documentation This pull request needs an associated documentation update label Jun 1, 2023
@digidax
Copy link

digidax commented Jun 21, 2023

@come-nc
Copy link
Contributor Author

come-nc commented Jun 22, 2023

Problem still present in 26.0.3rc2

https://help.nextcloud.com/t/webdav-with-folder-sync-typeerror-getimagesizefromstring-internal-server-error/164529

This PR was not backported to 26.
Not sure it is a good idea to backport this as it touches types in OCP, even if only phpdoc.

@digidax
Copy link

digidax commented Jun 22, 2023

Have applied as workaround (until v27 is available) this patch from master and it's working now.

https://github.com/nextcloud/server/commit/997efe44ffd56959f198be2e3aa71687a2f11412

grafik

@MWL1701
Copy link

MWL1701 commented Aug 16, 2023

Hello, I made an update to 27.0.2 yesterday and since then I have the GenericFileException error. Unfortunately I do not know what to do. Can someone help me with this?
Here is my error-stack:
[core] Fehler: OCP\Files\GenericFileException: at <>

  1. /var/www/nextcloud/lib/private/Files/SimpleFS/SimpleFile.php line 74
    OC\Files\Node\File->getContent()
  2. /var/www/nextcloud/lib/private/Template/JSCombiner.php line 118
    OC\Files\SimpleFS\SimpleFile->getContent()
  3. /var/www/nextcloud/lib/private/Template/JSCombiner.php line 93
    OC\Template\JSCombiner->isCached()
  4. /var/www/nextcloud/lib/private/Template/JSResourceLocator.php line 145
    OC\Template\JSCombiner->process()
  5. /var/www/nextcloud/lib/private/Template/JSResourceLocator.php line 103
    OC\Template\JSResourceLocator->cacheAndAppendCombineJsonIfExist()
  6. /var/www/nextcloud/lib/private/Template/ResourceLocator.php line 73
    OC\Template\JSResourceLocator->doFind()
  7. /var/www/nextcloud/lib/private/TemplateLayout.php line 377
    OC\Template\ResourceLocator->find()
  8. /var/www/nextcloud/lib/private/TemplateLayout.php line 222
    OC\TemplateLayout::findJavascriptFiles()
  9. /var/www/nextcloud/lib/private/legacy/OC_Template.php line 182
    OC\TemplateLayout->__construct()
  10. /var/www/nextcloud/lib/private/Template/Base.php line 132
    OC_Template->fetchPage()
  11. /var/www/nextcloud/lib/private/legacy/OC_Template.php line 331
    OC\Template\Base->printPage()
  12. /var/www/nextcloud/index.php line 74
    OC_Template::printExceptionErrorPage()

GET /index.php/apps/files/
from by at 2023-08-16T06:37:02+00:00

@MWL1701
Copy link

MWL1701 commented Aug 16, 2023

Found the solution:
In the config.php there was a new entry "update-secret", which had to be removed. Then it worked again.

@gjanssens
Copy link

My nextcloud setup is tracking the stable branch and currently only offers 25.0.13 as most recent version. Any chance this fix will be backported to that version ? For now I have manually applied part of the patch to lib/private/Files/Node/File.php and that seems to work, but I hope I don't have to continue applying these until the stable channel offers me an upgrade to 27.0.x.

@come-nc
Copy link
Contributor Author

come-nc commented Nov 7, 2023

My nextcloud setup is tracking the stable branch and currently only offers 25.0.13 as most recent version. Any chance this fix will be backported to that version ? For now I have manually applied part of the patch to lib/private/Files/Node/File.php and that seems to work, but I hope I don't have to continue applying these until the stable channel offers me an upgrade to 27.0.x.

Are you on PHP 7.4 by any chance? That may explain why the update to 26 is not offered to you, you need to update PHP to a supported version first I think.

@gjanssens
Copy link

I'm indeed still on PHP7.4. I had seen the warnings in the admin panel but didn't realize those were actually preventing further upgrades. Thanks for pointing that out, I'll go and upgrade PHP first.

@gjanssens
Copy link

I'm indeed still on PHP7.4. I had seen the warnings in the admin panel but didn't realize those were actually preventing further upgrades. Thanks for pointing that out, I'll go and upgrade PHP first.

In case anyone else gets here with the same question I had, upgrading PHP did indeed result in nextcloud offering upgrades to newer releases again. Thanks @come-nc for the pointer.

kesselb added a commit that referenced this pull request May 13, 2025
File.getContent can throw a GenericFileException since #37943.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb kesselb mentioned this pull request May 13, 2025
4 tasks
mickenordin pushed a commit to mickenordin/server that referenced this pull request May 23, 2025
File.getContent can throw a GenericFileException since nextcloud#37943.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
AIlkiv pushed a commit to AIlkiv/server that referenced this pull request Jul 10, 2025
File.getContent can throw a GenericFileException since nextcloud#37943.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants