Skip to content

Fix log exceptions for mp3 preview #41153

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.

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

Merged
merged 3 commits into from
Jan 17, 2024
Merged

Conversation

pako81
Copy link

@pako81 pako81 commented Jan 4, 2024

Description

Fix log exceptions for mp3 preview

Related Issue

Motivation and Context

Currently, log exceptions are generated when previews for MP3 files are being created, like:

{"reqId":"8q3EycderbaJ2eO5Hyc9","level":3,"time":"2024-01-04T21:19:44+00:00","remoteAddr":"xx.x.xxx.xxx","user":"admin","app":"PHP","method":"GET","url":"\/owncloud10134\/remote.php\/dav\/files\/admin\/file_example_MP3_1MG.mp3?c=faaf02aa4049f8cd054e634034dcdb58&x=64&y=64&forceIcon=0&preview=1","message":"file_get_contents(\/var\/www\/html\/owncloud10134\/data\/admin\/files\/var\/www\/html\/owncloud10134\/core\/img\/filetypes\/audio.svg): failed to open stream: No such file or directory at \/var\/www\/html\/owncloud10134\/lib\/private\/Files\/Storage\/Local.php#227"}
{"reqId":"8q3EycderbaJ2eO5Hyc9","level":3,"time":"2024-01-04T21:19:44+00:00","remoteAddr":"xx.xx.xxx.xxx","user":"admin","app":"PHP","method":"GET","url":"\/owncloud10134\/remote.php\/dav\/files\/admin\/file_example_MP3_1MG.mp3?c=faaf02aa4049f8cd054e634034dcdb58&x=64&y=64&forceIcon=0&preview=1","message":"imagecreatefromstring(): Empty string or invalid image at \/var\/www\/html\/owncloud10134\/lib\/private\/legacy\/image.php#702"}

How Has This Been Tested?

  • Manually, by making sure that such exceptions are no longer logged.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised
  • Changelog item

@pako81 pako81 added this to the development milestone Jan 4, 2024
@pako81 pako81 self-assigned this Jan 4, 2024
@pako81 pako81 force-pushed the fix_log_exceptions_for_mp3_preview branch from e52091b to 47b0c0e Compare January 4, 2024 22:31
@pako81 pako81 force-pushed the fix_log_exceptions_for_mp3_preview branch from 47b0c0e to 3710195 Compare January 4, 2024 22:33
@owncloud owncloud deleted a comment from update-docs bot Jan 5, 2024
@jvillafanez
Copy link
Member

The problem might be deeper.... It seems there is inconsistencies about how the image is loaded with the loadFromFile method, and we should probably recheck the rest of the methods too.

We're checking the filetype of the target file /var/www/html/owncloud10134/core/img/filetypes/audio.svg. The problem is that the SVG format isn't part of the filetypes we're handling in the loadFromFile, so it goes through the default method, and the default method uses the ownCloud's FS, so we're searching the file inside the user's primary storage. This is why the error appears.
Basically, using loadFromFile with SVG files is discouraged. I'm not sure if it works.

Using loadFromFileHandle should work. We shouldn't need to use any Imagick class on our own, and let our Image do the loading.

@pako81
Copy link
Author

pako81 commented Jan 17, 2024

Switched to loadFromFileHandle and avoided using a Imagick class.

Copy link

@pako81 pako81 merged commit cb5e983 into master Jan 17, 2024
@delete-merged-branch delete-merged-branch bot deleted the fix_log_exceptions_for_mp3_preview branch January 17, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants