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.

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

[Bug]: PDF previews don't work after upgrade to 27 - Error 400 #38911

Closed
6 of 8 tasks
Chartman123 opened this issue Jun 20, 2023 · 33 comments
Closed
6 of 8 tasks

[Bug]: PDF previews don't work after upgrade to 27 - Error 400 #38911

Chartman123 opened this issue Jun 20, 2023 · 33 comments

Comments

@Chartman123
Copy link

Chartman123 commented Jun 20, 2023

⚠️ This issue respects the following points: ⚠️

Bug description

On one of my instances PDF previews do no longer work after upgrading to NC 27. On both my other instances it works as expected.

The failing one is using the official Docker images. The other two instances are classic installations at shared web hosters. Other differences: DB (pgsql <-> mariadb, redis <-> non redis)

PDF previews work with Imaginary disabled, so I think it's related to Imaginary.

Also, preview generation works for x >= 257 and y >= 257

Steps to reproduce

  1. Browse to folder with PDF files
  2. No previews, just the PDF logo

Expected behavior

See previews of files

Installation method

Community Docker image

Nextcloud Server version

27

Operating system

Other

PHP engine version

PHP 8.2

Web server

Apache (supported)

Database engine version

PostgreSQL

Is this bug present after an update or on a fresh install?

Upgraded to a MAJOR version (ex. 22 to 23)

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

{
    "system": {
        "htaccess.RewriteBase": "\/",
        "memcache.local": "\\OC\\Memcache\\APCu",
        "apps_paths": [
            {
                "path": "\/var\/www\/html\/apps",
                "url": "\/apps",
                "writable": false
            },
            {
                "path": "\/var\/www\/html\/custom_apps",
                "url": "\/custom_apps",
                "writable": true
            }
        ],
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "cloud.tsmd.de"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "pgsql",
        "version": "27.0.0.8",
        "overwrite.cli.url": "https:\/\/cloud.tsmd.de",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "app_install_overwrite": [
            "occweb",
            "forms",
            "groupfolders",
            "announcementcenter",
            "metadata",
            "polls",
            "integration_whiteboard",
            "bruteforcesettings",
            "issuetemplate",
            "onlyoffice",
            "suspicious_login",
            "collectives",
            "impersonate",
            "approval"
        ],
        "maintenance": false,
        "loglevel": "2",
        "theme": "",
        "mail_smtpmode": "smtp",
        "mail_smtpsecure": "ssl",
        "mail_sendmailmode": "smtp",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpauthtype": "LOGIN",
        "mail_smtpauth": 1,
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "465",
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***",
        "forwarded_for_headers": [
            "X-Forwarded-For",
            "HTTP_X_FORWARDED_FOR"
        ],
        "overwritehost": "cloud.tsmd.de",
        "overwriteprotocol": "https",
        "default_language": "de",
        "default_locale": "de_DE",
        "trusted_proxies": "***REMOVED SENSITIVE VALUE***",
        "skeletondirectory": "\/var\/www\/html\/data\/skeleton",
        "updater.release.channel": "stable",
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": 6379
        },
        "trashbin_retention_obligation": "auto,30",
        "versions_retention_obligation": "auto,30",
        "default_phone_region": "DE",
        "simpleSignUpLink.shown": false,
        "data-fingerprint": "681e3987b71dbcf8a80c491511dce9f8",
        "ldapProviderFactory": "OCA\\User_LDAP\\LDAPProviderFactory",
        "onlyoffice": {
            "jwt_header": "Jwt-Authorization"
        },
        "activity_use_cached_mountpoints": true,
        "enabledPreviewProviders": [
            "OC\\Preview\\PNG",
            "OC\\Preview\\JPEG",
            "OC\\Preview\\GIF",
            "OC\\Preview\\BMP",
            "OC\\Preview\\XBitmap",
            "OC\\Preview\\MP3",
            "OC\\Preview\\TXT",
            "OC\\Preview\\MarkDown",
            "OC\\Preview\\OpenDocument",
            "OC\\Preview\\Krita",
            "OC\\Preview\\Imaginary"
        ],
        "preview_imaginary_url": "http:\/\/imaginary:9000",
        "debug": false
    }
}

List of activated Apps

Enabled:
  - activity: 2.19.0
  - announcementcenter: 6.6.1
  - approval: 1.0.12
  - bruteforcesettings: 2.7.0
  - calendar: 4.4.2
  - circles: 27.0.0
  - cloud_federation_api: 1.10.0
  - collectives: 2.5.0
  - comments: 1.17.0
  - contacts: 5.3.1
  - dashboard: 7.7.0
  - dav: 1.27.0
  - deck: 1.10.0
  - federatedfilesharing: 1.17.0
  - federation: 1.17.0
  - files: 1.22.0
  - files_automatedtagging: 1.17.0
  - files_pdfviewer: 2.8.0
  - files_rightclick: 1.6.0
  - files_sharing: 1.19.0
  - files_trashbin: 1.17.0
  - files_versions: 1.20.0
  - firstrunwizard: 2.16.0
  - forms: 3.3.0
  - groupfolders: 15.0.0
  - impersonate: 1.14.0
  - integration_giphy: 1.0.6
  - integration_openstreetmap: 1.0.5
  - integration_whiteboard: 0.0.14
  - integration_youtube: 0.1.4
  - logreader: 2.12.0
  - lookup_server_connector: 1.15.0
  - mail: 3.2.2
  - nextcloud_announcements: 1.16.0
  - notifications: 2.15.0
  - notify_push: 0.6.3
  - oauth2: 1.15.0
  - onlyoffice: 8.1.0
  - password_policy: 1.17.0
  - polls: 5.0.5
  - privacy: 1.11.0
  - provisioning_api: 1.17.0
  - recommendations: 1.6.0
  - related_resources: 1.2.0
  - serverinfo: 1.17.0
  - settings: 1.9.0
  - sharebymail: 1.17.0
  - spreed: 17.0.0
  - survey_client: 1.15.0
  - suspicious_login: 5.0.0
  - systemtags: 1.17.0
  - tables: 0.5.1
  - text: 3.8.0
  - theming: 2.2.0
  - twofactor_backupcodes: 1.16.0
  - twofactor_nextcloud_notification: 3.7.0
  - twofactor_totp: 9.0.0
  - updatenotification: 1.17.0
  - user_ldap: 1.17.0
  - user_status: 1.7.0
  - viewer: 2.1.0
  - weather_status: 1.7.0
  - workflowengine: 2.9.0
Disabled:
  - admin_audit: 1.17.0
  - contactsinteraction: 1.8.0 (installed 1.7.0)
  - encryption: 2.15.0
  - files_external: 1.19.0
  - issuetemplate: 0.7.0 (installed 0.7.0)
  - libresign: 7.1.1 (installed 7.1.1)
  - photos: 2.3.0 (installed 1.0.0)
  - support: 1.10.0 (installed 1.1.0)

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

Couldn't find anything in the logs, even with Debug mode

Additional info

https://cloud.domain.tld/core/preview?fileId=280976&c=bb75d279726eb8ef826eb9764852a2a1&x=250&y=250&forceIcon=0&a=1

Response code: 400
Response: []

@Chartman123 Chartman123 added bug 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jun 20, 2023
@muchachagrande
Copy link

I'm having this issue but on a Hansson IT VM.
I can see that the preview is being generated because it is visible on the details panel of the file, but on the file list a PDF default icon is shown.

@muchachagrande
Copy link

muchachagrande commented Jul 20, 2023

I think that the problem is with the "preview" parameters "x" and "y".

When I enter a folder with PDFs Nextcloud tries to get a preview icon for each PDF.

The call is this:
GET /core/preview?fileId=3313142&c=628d0015278b2&x=250&y=250&forceIcon=0&a=1

If I test this call manually I get an empty json content-type.

But if I execute the call changing "x" and "y" for some bigger number like 512 x 512 or more I get the right icon with image/png content-type.

Also if you open the PDF detail view of the PDF it shows the correct preview image in the upper corner.
The call is different:
GET /core/preview?fileId=3313142&x=1920&y=1080&a=true

This call has x=1920 and y=1080 and it works fine.

@FarisZR
Copy link

FarisZR commented Aug 2, 2023

I am also facing this issue with the official docker image.
Previews show up in the info panel, just not while browsing on the web or in apps, can confirm @muchachagrande analysis, just upping the size 512 fixes it.

I am using imaginary if that affects anything.

@Chartman123
Copy link
Author

I just tried to find the numbers for x and y where the preview starts to work: its 257.

However, the image file in the response always has 793 x 793 pixels, regardless of the provided x and y values.

@JanisPlayer
Copy link
Contributor

Yes, the previews of .HEIC files also do not load with the parameters &x=250&y=250&forceIcon=0&a=1.
.WebP also does not load without my PR changes to OC_Image.php.
This is likely because a different preview generator is being used for these files, not DG.
I was completely surprised by this bug and even reinstalled Nextcloud to then realize that the problem existed before.

@Chartman123
Copy link
Author

At least for me this only happens with Imaginary enabled (see #40652 (comment))

@JanisPlayer
Copy link
Contributor

At least for me this only happens with Imaginary enabled (see #40652 (comment))

Yes, I can confirm. Thank you for pointing me in the right direction. I checked my standard config again, and I forgot to remove "preview_imaginary_url" . Everything is working again now. Thanks, you've been a great help; I wouldn't have looked so closely otherwise.

@solracsf
Copy link
Member

What are your Imaginary docker compose params?

@Chartman123
Copy link
Author

Chartman123 commented Sep 27, 2023

@solracsf

  imaginary:
    image: nextcloud/aio-imaginary:latest
    deploy:
      update_config:
        order: start-first
    networks:
      - nextcloud

Just the basics. So the Entrypoint from the Dockerfile. I just added the other recommendations from your other post:

    entrypoint:
      - "imaginary"
      - "-return-size"
      - "-max-allowed-resolution"
      - "222.2"
      - "-concurrency" 
      - "50"
      - "-enable-url-source"
      - "log-level" 
      - "debug"
    cap_add:
      - sys_nice

I'm now testing it...

Edit:

No changes... As soon as I re-enable Imaginary in my config, the calls for the previews start to fail again with error code 400. In my debug output of the imaginary service, I don't see any requests.

@enoch85
Copy link
Member

enoch85 commented Sep 27, 2023

@solracsf where's the function/setting in the code for generating 250x250? Since 257 works, maybe we/you should change to that?

Sorry, I don't know any PHP.

@Chartman123
Copy link
Author

Shouldn't we try to fix the origin of this issue instead of just working around? What if the problem starts to show with values smaller 260??

@solracsf
Copy link
Member

solracsf commented Sep 27, 2023

Sure, but a good starting point is finding out why it works for some, but not for all.
It works for me on a fresh install of v27.1.1.

brave_cOpArTdl9I.mp4

@muchachagrande
Copy link

muchachagrande commented Sep 27, 2023

May be I found something. if I enter a folder with some PDF, then looking into the Imaginary log, it shows nothing, but when I open file details, Imaginary shows the operation results in the log.
May be Imaginary is not even being used when dimensions are below 256x256 but only with 257x257 and above.

EDIT: almost confirmed. I've made a little change and now I have PDF preview working on Imaginary.

Look at this:

&& ($specifications[0]['width'] <= 256 || $specifications[0]['height'] <= 256)

I've changed 256 to 249 on both dimensions and now calling to Imaginary with 250 x 250 is working.
I can see the conversions in the Imaginary docker log.

@Chartman123
Copy link
Author

Sure, but a good starting point is finding out why it works for some, but not for all.
It works for me on a fresh install of v27.1.1.

brave_cOpArTdl9I.mp4

@solracsf Does it also work if you use the list view?

@solracsf
Copy link
Member

For those having the problem, can you patch this line:

} catch (NotFoundException $e) {

so it reads

} catch (NotFoundException | \InvalidArgumentException $e) {

@enoch85
Copy link
Member

enoch85 commented Sep 28, 2023

@solracsf That solves it for me!

enoch85 added a commit that referenced this issue Sep 28, 2023
This solves #38911

cc @szaimen @solracsf 

Signed-off-by: Daniel Hansson <[email protected]>
@enoch85 enoch85 added 3. to review Waiting for reviews and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Sep 28, 2023
@Chartman123
Copy link
Author

@solracsf @enoch85 Yes this works for me, too. However, I still don't see any calls in imaginary for these previews. Is this intended?

@solracsf
Copy link
Member

solracsf commented Sep 28, 2023

How are your PDF previews generated than? 🤔

I can see the Imaginary calls in my instance.
The better way to reproduce this is, without the patch, delete PDF files and go to the Trash.
With the patch you'll get the previews, without it, errors 400.

As you can see, the x and y paramteters are lower than 256.

/apps/files_trashbin/preview?fileId=776&x=32&y=32&a=1

@Chartman123
Copy link
Author

Chartman123 commented Sep 28, 2023

Perhaps with Onlyoffice? As soon as I remove the Onlyoffice checkbox to generate the previews I see more logging in my Imaginary container...

image

But then of course no more previews for Office files 😉

@solracsf
Copy link
Member

solracsf commented Sep 28, 2023

Well, if you have different previews generators it can get hard to debug your setup 😆
Disable your apps, be sure to have only Imaginary working, and retry.

@Chartman123
Copy link
Author

So in the end your proposed change fixes the problem also for this mixed setup. Which is good to know... Why it only appears if Imaginary is configured even if it's not used for PDF previews when Onlyoffice generation is enabled, still isn't clear to me. But as long as everything works, I'm fine with that 👍🏻

@enoch85
Copy link
Member

enoch85 commented Sep 28, 2023

How are your PDF previews generated than? 🤔

I can see the Imaginary calls in my instance. The better way to reproduce this is, without the patch, delete PDF files and go to the Trash. With the patch you'll get the previews, without it, errors 400.

As you can see, the x and y paramteters are lower than 256.

/apps/files_trashbin/preview?fileId=776&x=32&y=32&a=1

I just tested.

  1. Create a new folder in Nextcloud
  2. Upload a bunch of PDFs to it
  3. See that the imaginary docker log spits out a lot of info on the new previews
  4. Put the new folder in trash
  5. Same as # 3.

In other words, the fix works for Imaginary. I also noticed TIF previews are now working which wasn't working before.

@enoch85
Copy link
Member

enoch85 commented Sep 28, 2023

One thing I noticed now though, previews in the activity pane are still not generated:

https://cloud.whatever.nu/apps/theming/img/core/filetypes/application-pdf.svg?v=7e8a4c67

image

@Chartman123
Copy link
Author

No problems there for me

@muchachagrande
Copy link

Try this #38911 (comment)

@solracsf
Copy link
Member

solracsf commented Sep 28, 2023

Please note, the code only fixes Imaginary, well, because it only targets it.

In other words, developpers had put a condition that, if Imaginary is installed (correctly) and the asked preview is less or equal than 256, it should only generate that size, instead of all sizes normally generated when a preview is asked (by the comments on that file, for "performances reasons").

There is a problem in the code because it first try to get a cache (an already generated preview in other words), which doesn't exist for a first asked preview (obviously), but it returns 400 instead of 404 because $width and $height are not in the expected range.

So, the fix bypasses not only a 404 (expected) but also a 400 (possible) and generate a new preview as cache returns "false". The same logic is already present on that same file if you read the code.

For me, the best solution would be to remove that "performance" condition and just let Imaginary generate all sizes, just like any other provider. 👍 Less error prone.

@enoch85
Copy link
Member

enoch85 commented Sep 28, 2023

Try this #38911 (comment)

That didn't help, tried to set as low as 1 X 1, but nothing.

@enoch85
Copy link
Member

enoch85 commented Sep 28, 2023

Please note, the code only fixes Imaginary, well, because it only targets it.

In other words, developpers had put a condition that, if Imaginary is installed (correctly) and the asked preview is less or equal than 256, it should only generate that size, instead of all sizes normally generated when a preview is asked (by the comments on that file, for "performances reasons").

There is a problem in the code because it first try to get a cache (an already generated preview in other words), which doesn't exist for a first asked preview (obviously), but it returns 400 instead of 404 because $width and $height are not in the expected range.

So, the fix bypasses not only a 404 (expected) but also a 400 (possible) and generate a new preview as cache returns "false". The same logic is already present on that same file if you read the code.

For me, the best solution would be to remove that "performance" condition and just let Imaginary generate all sizes, just like any other provider. 👍 Less error prone.

Fine with me, and a better solution also in general. That's how preview generator works IIRC.

@Chartman123
Copy link
Author

For me, the best solution would be to remove that "performance" condition and just let Imaginary generate all sizes, just like any other provider. 👍 Less error prone.

@solracsf So, should this be done right now in the linked PR by @enoch85 or later in a second PR?

@enoch85
Copy link
Member

enoch85 commented Sep 29, 2023

I vote for a second PR.

@solracsf
Copy link
Member

solracsf commented Sep 30, 2023

I'll let core developpers take decisions here ;)

This is the code I'm talking about (to be eventually removed):

// If imaginary is enabled, and we request a small thumbnail,
// let's not generate the max preview for performance reasons
if (count($specifications) === 1
&& ($specifications[0]['width'] <= 256 || $specifications[0]['height'] <= 256)
&& preg_match(Imaginary::supportedMimeTypes(), $mimeType)
&& $this->config->getSystemValueString('preview_imaginary_url', 'invalid') !== 'invalid') {
$crop = $specifications[0]['crop'] ?? false;
$preview = $this->getSmallImagePreview($previewFolder, $previewFiles, $file, $mimeType, $previewVersion, $crop);
if ($preview->getSize() === 0) {
$preview->delete();
throw new NotFoundException('Cached preview size 0, invalid!');
}
return $preview;
}

enoch85 added a commit that referenced this issue Sep 30, 2023
A follow up on #40670

Based on discussions here: #38911 (comment)

This fixes the case were not all previews are generated, for example in the activity view: #38911 (comment)



Signed-off-by: Daniel Hansson <[email protected]>
@enoch85
Copy link
Member

enoch85 commented Sep 30, 2023

Now it's in a separate PR since it's two different issues.

@enoch85
Copy link
Member

enoch85 commented Oct 9, 2023

A fix has been merged, this issue can be closed.

@enoch85 enoch85 closed this as completed Oct 9, 2023
nfebe pushed a commit that referenced this issue Oct 9, 2023
A follow up on #40670

Based on discussions here: #38911 (comment)

This fixes the case were not all previews are generated, for example in the activity view: #38911 (comment)



Signed-off-by: Daniel Hansson <[email protected]>
zak39 pushed a commit to zak39/server that referenced this issue Dec 19, 2023
A follow up on nextcloud#40670

Based on discussions here: nextcloud#38911 (comment)

This fixes the case were not all previews are generated, for example in the activity view: nextcloud#38911 (comment)



Signed-off-by: Daniel Hansson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants