From 1406252fafe17dc306d7343c43bdf3a59dbd9408 Mon Sep 17 00:00:00 2001
From: killecaptron <203002577+killecaptron@users.noreply.github.com>
Date: Wed, 2 Sep 2026 15:09:50 +0200
Subject: [PATCH 1/2] Fix broken preview images for extension-less external
files
If no file extension can be extracted from the URL of an external
attachment, Part-DB assumed that it is a picture. That is a sensible
guess for image URLs, but it also applies to URLs which redirect to the
actual file, like the datasheet links of the TrustedParts provider
(https://www.trustedparts.com/productredirect?id=...).
Such datasheets were rendered as an ![]()
in the attachment lists
(showing a broken image) and the first one was automatically used as the
preview picture of the part by the AttachmentSubmitHandler, so the part
page showed a broken main image too.
The attachment type already knows which filetypes it may contain (the
"Datasheet" type created by the info provider system is restricted to
application/pdf), so use that information for the guess. Attachments
whose URL has a known picture extension are unaffected, as are
attachment types without a filetype filter.
Existing parts heal themselves: the AttachmentSubmitHandler already
removes a master picture attachment which is not a picture anymore, as
soon as the part is saved the next time.
---
src/Entity/Attachments/Attachment.php | 37 +++++++++++++++++++--
tests/Entity/Attachments/AttachmentTest.php | 25 ++++++++++++++
2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/src/Entity/Attachments/Attachment.php b/src/Entity/Attachments/Attachment.php
index 1f8ad2972..f3c561be9 100644
--- a/src/Entity/Attachments/Attachment.php
+++ b/src/Entity/Attachments/Attachment.php
@@ -289,13 +289,46 @@ public function isPicture(): bool
//Check if we can extract a file extension from the URL
$extension = pathinfo(parse_url($this->getExternalPath(), PHP_URL_PATH) ?? '', PATHINFO_EXTENSION);
- //If no extension is found or it is known picture extension, we assume that this is a picture extension
- return $extension === '' || in_array(strtolower($extension), static::PICTURE_EXTS, true);
+ if (in_array(strtolower($extension), static::PICTURE_EXTS, true)) {
+ return true;
+ }
+
+ //If no extension is found (e.g. for URLs which redirect to the actual file), we can only guess. We assume
+ //that it is a picture, unless the attachment type rules pictures out (like the "Datasheet" type, which only
+ //allows PDFs), as we would show a broken image otherwise.
+ return $extension === '' && $this->attachmentTypeAllowsPictures();
}
//File doesn't have an internal, nor an external copy. This shouldn't happen, but it certainly isn't a picture...
return false;
}
+ /**
+ * Checks whether the filetype filter of the attachment type allows pictures.
+ * If no attachment type is set, or it does not restrict the filetypes, true is returned, as we can not tell.
+ */
+ private function attachmentTypeAllowsPictures(): bool
+ {
+ $filter = trim($this->getAttachmentType()?->getFiletypeFilter() ?? '');
+
+ if ($filter === '') {
+ return true;
+ }
+
+ foreach (explode(',', $filter) as $allowed) {
+ $allowed = trim(strtolower($allowed));
+
+ if ($allowed === '*' || $allowed === '*/*' || str_starts_with($allowed, 'image/')) {
+ return true;
+ }
+
+ if (in_array(ltrim($allowed, '.'), static::PICTURE_EXTS, true)) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
/**
* Check if this attachment is a 3D model and therefore can be directly shown to user.
* If no internal copy exists, false is returned (3D Models must be internal).
diff --git a/tests/Entity/Attachments/AttachmentTest.php b/tests/Entity/Attachments/AttachmentTest.php
index bef2df168..7d4173ac4 100644
--- a/tests/Entity/Attachments/AttachmentTest.php
+++ b/tests/Entity/Attachments/AttachmentTest.php
@@ -171,6 +171,31 @@ public function testIsPicture(?string $internal_path, ?string $external_path, bo
$this->assertSame($expected, $attachment->isPicture());
}
+ public static function pictureFiletypeFilterDataProvider(): \Iterator
+ {
+ //An URL without a file extension is only assumed to be a picture, if the attachment type allows pictures
+ yield ['https://invalid.com/redirect?id=1234', '', true];
+ yield ['https://invalid.com/redirect?id=1234', 'image/*', true];
+ yield ['https://invalid.com/redirect?id=1234', '.jpg,.png', true];
+ yield ['https://invalid.com/redirect?id=1234', 'application/pdf', false];
+ yield ['https://invalid.com/redirect?id=1234', '.pdf,.txt', false];
+ //An URL with a picture extension is always a picture, no matter what the attachment type says
+ yield ['https://invalid.com/picture.jpeg', 'application/pdf', true];
+ }
+
+ #[DataProvider('pictureFiletypeFilterDataProvider')]
+ public function testIsPictureRespectsFiletypeFilter(string $external_path, string $filter, bool $expected): void
+ {
+ $attachment_type = new AttachmentType();
+ $attachment_type->setFiletypeFilter($filter);
+
+ $attachment = new PartAttachment();
+ $attachment->setAttachmentType($attachment_type);
+ $this->setProtectedProperty($attachment, 'external_path', $external_path);
+
+ $this->assertSame($expected, $attachment->isPicture());
+ }
+
public static function builtinDataProvider(): \Iterator
{
yield ['', false];
From c08fa3381f68430741ddda157ca5c51203f803bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20B=C3=B6hmer?=
Date: Sun, 6 Sep 2026 21:51:41 +0200
Subject: [PATCH 2/2] Moved allowsPicture logic to attachmentType
---
src/Entity/Attachments/Attachment.php | 31 ++-----------------
src/Entity/Attachments/AttachmentType.php | 26 ++++++++++++++++
.../Entity/Attachments/AttachmentTypeTest.php | 30 ++++++++++++++++++
3 files changed, 59 insertions(+), 28 deletions(-)
diff --git a/src/Entity/Attachments/Attachment.php b/src/Entity/Attachments/Attachment.php
index f3c561be9..7a4a170a5 100644
--- a/src/Entity/Attachments/Attachment.php
+++ b/src/Entity/Attachments/Attachment.php
@@ -285,6 +285,7 @@ public function isPicture(): bool
return in_array(strtolower($extension), static::PICTURE_EXTS, true);
}
+
if ($this->hasExternal()) {
//Check if we can extract a file extension from the URL
$extension = pathinfo(parse_url($this->getExternalPath(), PHP_URL_PATH) ?? '', PATHINFO_EXTENSION);
@@ -296,36 +297,10 @@ public function isPicture(): bool
//If no extension is found (e.g. for URLs which redirect to the actual file), we can only guess. We assume
//that it is a picture, unless the attachment type rules pictures out (like the "Datasheet" type, which only
//allows PDFs), as we would show a broken image otherwise.
- return $extension === '' && $this->attachmentTypeAllowsPictures();
- }
- //File doesn't have an internal, nor an external copy. This shouldn't happen, but it certainly isn't a picture...
- return false;
- }
-
- /**
- * Checks whether the filetype filter of the attachment type allows pictures.
- * If no attachment type is set, or it does not restrict the filetypes, true is returned, as we can not tell.
- */
- private function attachmentTypeAllowsPictures(): bool
- {
- $filter = trim($this->getAttachmentType()?->getFiletypeFilter() ?? '');
-
- if ($filter === '') {
- return true;
- }
-
- foreach (explode(',', $filter) as $allowed) {
- $allowed = trim(strtolower($allowed));
-
- if ($allowed === '*' || $allowed === '*/*' || str_starts_with($allowed, 'image/')) {
- return true;
- }
-
- if (in_array(ltrim($allowed, '.'), static::PICTURE_EXTS, true)) {
- return true;
- }
+ return $extension === '' && ($this->getAttachmentType()?->allowsPictures() ?? true);
}
+ //File doesn't have an internal, nor an external copy. This shouldn't happen, but it certainly isn't a picture...
return false;
}
diff --git a/src/Entity/Attachments/AttachmentType.php b/src/Entity/Attachments/AttachmentType.php
index 03bb8031a..454ed67ff 100644
--- a/src/Entity/Attachments/AttachmentType.php
+++ b/src/Entity/Attachments/AttachmentType.php
@@ -191,6 +191,32 @@ public function setFiletypeFilter(string $filetype_filter): self
return $this;
}
+ /**
+ * Checks if this attachment type allows pictures (i.e. if the filetype filter allows image files).
+ * If no filetype filter is set, this method returns true, as it is assumed that all file types are allowed.
+ * @return bool
+ */
+ public function allowsPictures(): bool
+ {
+ if ($this->filetype_filter === '') {
+ return true;
+ }
+
+ foreach (explode(',', $this->filetype_filter) as $allowed) {
+ $allowed = strtolower(trim($allowed));
+
+ if ($allowed === '*' || $allowed === '*/*' || str_starts_with($allowed, 'image/')) {
+ return true;
+ }
+
+ if (in_array(ltrim($allowed, '.'), Attachment::PICTURE_EXTS, true)) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
/**
* Returns a list of allowed targets as class names (e.g. PartAttachment::class), where this attachment type can be assigned to. If null, there are no restrictions.
* @return class-string[]|null
diff --git a/tests/Entity/Attachments/AttachmentTypeTest.php b/tests/Entity/Attachments/AttachmentTypeTest.php
index c966d23f8..dad479981 100644
--- a/tests/Entity/Attachments/AttachmentTypeTest.php
+++ b/tests/Entity/Attachments/AttachmentTypeTest.php
@@ -26,6 +26,7 @@
use App\Entity\Attachments\PartAttachment;
use App\Entity\Attachments\UserAttachment;
use Doctrine\Common\Collections\Collection;
+use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
final class AttachmentTypeTest extends TestCase
@@ -83,4 +84,33 @@ public function testIsAllowedForTarget(): void
$this->assertTrue($attachmentType->isAllowedForTarget(PartAttachment::class));
$this->assertTrue($attachmentType->isAllowedForTarget(UserAttachment::class));
}
+
+ public static function allowsPictureDataProvider(): array
+ {
+ return [
+ ['', true],
+ ['image/*', true],
+ ['image/png,image/jpeg', true],
+ ['application/pdf', false],
+ ['text/plain', false],
+ ['image/gif,application/pdf', true],
+ ['audio/mpeg', false],
+ ['video/mp4', false],
+ ['image/svg+xml', true],
+ ['application/zip', false],
+ ['image/webp,text/html', true],
+ ['*', true],
+ ['.pdf,.png', true],
+ ['.jpeg,.gif', true],
+ ['.pdf,.xml', false],
+ ];
+ }
+
+ #[DataProvider('allowsPictureDataProvider')]
+ public function testAllowsPicture(string $filetype_filter, bool $expected): void
+ {
+ $attachmentType = new AttachmentType();
+ $attachmentType->setFiletypeFilter($filetype_filter);
+ $this->assertSame($expected, $attachmentType->allowsPictures());
+ }
}