Compare commits

...

6 Commits

Author SHA1 Message Date
Ollama
a55885de87 Fix technical issues from PR review
Image_lib.php fixes:
- Remove incorrect 'GPS' => PelTag::GPS_OFFSET mapping (GPS handled separately)
- Fix PNG transparency: change imagealphablending to false
- Add animated GIF detection: skip processing for animations
- Add logging in stripExifFallback to show which APP markers are removed

Migration optimization:
- Use get()->getRow() instead of countAllResults() for lighter query
- More efficient existence check before insert

All changes address CodeRabbit technical review comments while maintaining
the simplified multiselect-only UI (no toggle needed).
2026-04-08 21:12:27 +00:00
Ollama
accc8c5911 Address all PR feedback for EXIF stripping
Major changes:
- Remove exif_stripping_enabled checkbox, use multiselect only
  - Feature is enabled when multiselect has selections
  - Disabled when multiselect is empty
  - Simplifies UI and logic

- Fix PHP implode() null issue
  - Coerce null to empty array before implode (Config.php:398,399)
  - Apply fix to both exif_fields_to_keep and image_allowed_types

- Fix GPS removal in Image_lib.php
  - Remove GPS via ifd0.removeEntry() instead of incorrect setIfd(null)
  - GPS INFO IFD POINTER now properly removed from IFD0

- Fix allowed_types array in Image_lib.php
  - Remove unsupported 'image/bmp' and 'image/tiff'
  - Return false instead of true for unhandled formats

- Add logging for EXIF stripping failures
  - Log warnings when stripEXIF returns false
  - Helps debugging without blocking uploads

- Fix migration consistency
  - Remove exif_stripping_enabled config (no longer needed)
  - Remove unused $forge variable
  - Make defaults consistent: Copyright,Orientation,Software

- Update language strings
  - Remove exif_stripping_enabled translations
  - Clarify exif_fields_to_keep tooltip

Addresses all actionable comments from CodeRabbit review
2026-04-08 21:12:27 +00:00
Ollama
a1fd3991b9 Update composer.lock for plugin-api-version compatibility 2026-04-08 21:12:27 +00:00
jekkos
bd312e3e1d Implement selective EXIF removal using FileEye/pel library
- Add fileeye/pel dependency to composer.json for selective EXIF field removal
- Rewrite Image_lib::stripExifJpeg() to use FileEye/pel for precise field manipulation
- Add exif_to_pel_tags mapping for supported EXIF fields
- Implement removeExifFields() to selectively remove EXIF data based on config
- Keep fallback method if library is unavailable or parsing fails
- Add language strings for new configuration options
- Update migration to include Software in default fields to keep

This addresses reviewer concern about preserving copyright and other beneficial
metadata while removing privacy-sensitive fields like GPS location.
2026-04-08 21:12:27 +00:00
jekkos
6e498aab42 Address PR review comments #4394
- Renamed strip_exif() to stripEXIF() for PSR-12 compliance
- Added configuration options for EXIF stripping (exif_stripping_enabled, exif_fields_to_keep)
- Migration to add new config keys with sensible defaults
- Updated Config and Items controllers to check config before stripping EXIF
- Made EXIF stripping optional via settings, defaulting to disabled for backward compatibility
- Allows selective field preservation (Copyright, Orientation by default)
2026-04-08 21:12:27 +00:00
jekkos
ee5ed3c699 Strip EXIF metadata from uploaded images
- Created Image_lib library to handle EXIF stripping for JPEG, PNG, GIF, and WebP images
- Uses GD library to re-encode images without EXIF data
- Added EXIF stripping to both company logo upload (Config controller) and item image upload (Items controller)
- Handles privacy concern by removing geolocation and device info from uploaded images

Fixes #4010
2026-04-08 21:12:27 +00:00
8 changed files with 326 additions and 3 deletions

View File

@@ -3,6 +3,7 @@
namespace App\Controllers;
use App\Libraries\Barcode_lib;
use App\Libraries\Image_lib;
use App\Libraries\Mailchimp_lib;
use App\Libraries\Receiving_lib;
use App\Libraries\Sale_lib;
@@ -250,6 +251,10 @@ class Config extends Secure_Controller
$data['image_allowed_types'] = array_combine($image_allowed_types, $image_allowed_types);
$data['selected_image_allowed_types'] = explode(',', $this->config['image_allowed_types']);
$exif_fields = ['Make', 'Model', 'Orientation', 'Copyright', 'Software', 'DateTime', 'GPS'];
$data['exif_fields'] = array_combine($exif_fields, $exif_fields);
$data['selected_exif_fields'] = array_filter(explode(',', $this->config['exif_fields_to_keep'] ?? ''));
// Integrations Related fields
$data['mailchimp'] = [];
@@ -355,6 +360,15 @@ class Config extends Secure_Controller
$file->move(FCPATH . 'uploads/', $file_info['raw_name'] . '.' . $file_info['file_ext'], true);
$exif_fields_to_keep = array_filter(explode(',', $this->appconfig->get_value('exif_fields_to_keep', 'Copyright,Orientation,Software')));
if (!empty($exif_fields_to_keep)) {
$image_lib = new Image_lib();
$filepath = FCPATH . 'uploads/' . $file_info['raw_name'] . '.' . $file_info['file_ext'];
if (!$image_lib->stripEXIF($filepath, $exif_fields_to_keep)) {
log_message('warning', 'EXIF stripping failed for: ' . $filepath);
}
}
return ($file_info);
}
@@ -382,7 +396,8 @@ class Config extends Secure_Controller
'image_max_width' => $this->request->getPost('image_max_width', FILTER_SANITIZE_NUMBER_INT),
'image_max_height' => $this->request->getPost('image_max_height', FILTER_SANITIZE_NUMBER_INT),
'image_max_size' => $this->request->getPost('image_max_size', FILTER_SANITIZE_NUMBER_INT),
'image_allowed_types' => implode(',', $this->request->getPost('image_allowed_types')),
'image_allowed_types' => implode(',', $this->request->getPost('image_allowed_types') ?? []),
'exif_fields_to_keep' => implode(',', $this->request->getPost('exif_fields_to_keep') ?? []),
'gcaptcha_enable' => $this->request->getPost('gcaptcha_enable') != null,
'gcaptcha_secret_key' => $this->request->getPost('gcaptcha_secret_key'),
'gcaptcha_site_key' => $this->request->getPost('gcaptcha_site_key'),

View File

@@ -3,8 +3,10 @@
namespace App\Controllers;
use App\Libraries\Barcode_lib;
use App\Libraries\Image_lib;
use App\Libraries\Item_lib;
use App\Models\Appconfig;
use App\Models\Attribute;
use App\Models\Inventory;
use App\Models\Item;
@@ -39,6 +41,7 @@ class Items extends Secure_Controller
private Stock_location $stock_location;
private Supplier $supplier;
private Tax_category $tax_category;
private Appconfig $appconfig;
private array $config;
@@ -62,6 +65,7 @@ class Items extends Secure_Controller
$this->stock_location = model(Stock_location::class);
$this->supplier = model(Supplier::class);
$this->tax_category = model(Tax_category::class);
$this->appconfig = model(Appconfig::class);
$this->config = config(OSPOS::class)->settings;
}
@@ -788,6 +792,16 @@ class Items extends Secure_Controller
];
$file->move(FCPATH . 'uploads/item_pics/', $file_info['raw_name'] . '.' . $file_info['file_ext'], true);
$exif_fields_to_keep = array_filter(explode(',', $this->appconfig->get_value('exif_fields_to_keep', 'Copyright,Orientation,Software')));
if (!empty($exif_fields_to_keep)) {
$image_lib = new Image_lib();
$filepath = FCPATH . 'uploads/item_pics/' . $file_info['raw_name'] . '.' . $file_info['file_ext'];
if (!$image_lib->stripEXIF($filepath, $exif_fields_to_keep)) {
log_message('warning', 'EXIF stripping failed for: ' . $filepath);
}
}
return ($file_info);
}

View File

@@ -0,0 +1,49 @@
<?php
namespace App\Database\Migrations;
use CodeIgniter\Database\Migration;
use Config\Database;
class MigrationEXIFStrippingOptions extends Migration
{
/**
* Perform a migration step.
*/
public function up(): void
{
log_message('info', 'Migrating EXIF Stripping Options');
$db = Database::connect();
$configs = [
[
'key' => 'exif_fields_to_keep',
'value' => 'Copyright,Orientation,Software'
]
];
foreach ($configs as $config) {
$existing = $db->table('app_config')
->where('key', $config['key'])
->get()
->getRow();
if ($existing === null) {
$db->table('app_config')->insert($config);
}
}
}
/**
* Revert a migration step.
*/
public function down(): void
{
$db = Database::connect();
$db->table('app_config')
->where('key', 'exif_fields_to_keep')
->delete();
}
}

View File

@@ -329,4 +329,6 @@ return [
"wholesale_markup" => "",
"work_order_enable" => "Work Order Support",
"work_order_format" => "Work Order Format",
"exif_fields_to_keep" => "EXIF Fields to Keep",
"exif_fields_to_keep_tooltip" => "Select EXIF fields to preserve in uploaded images. Fields not selected will be removed. Leave empty to disable EXIF stripping. Keeps beneficial metadata while removing privacy-sensitive data like GPS location.",
];

220
app/Libraries/Image_lib.php Normal file
View File

@@ -0,0 +1,220 @@
<?php
namespace App\Libraries;
use lsolesen\pel\PelIfd;
use lsolesen\pel\PelJpeg;
use lsolesen\pel\PelTag;
class Image_lib
{
private array $exif_to_pel_tags = [
'Make' => PelTag::MAKE,
'Model' => PelTag::MODEL,
'Orientation' => PelTag::ORIENTATION,
'Copyright' => PelTag::COPYRIGHT,
'Software' => PelTag::SOFTWARE,
'DateTime' => PelTag::DATE_TIME,
];
public function stripEXIF(string $filepath, array $fields_to_keep = []): bool
{
if (!file_exists($filepath)) {
return false;
}
$mimetype = mime_content_type($filepath);
$allowed_types = ['image/jpeg', 'image/jpg', 'image/png', 'image/gif', 'image/webp'];
if (!in_array($mimetype, $allowed_types)) {
return false;
}
if ($mimetype === 'image/jpeg' || $mimetype === 'image/jpg') {
return $this->stripExifJpeg($filepath, $fields_to_keep);
}
if ($mimetype === 'image/png') {
return $this->stripExifPng($filepath);
}
if ($mimetype === 'image/gif') {
return $this->stripExifGif($filepath);
}
if ($mimetype === 'image/webp') {
return $this->stripExifWebp($filepath);
}
return false;
}
private function stripExifJpeg(string $filepath, array $fields_to_keep = []): bool
{
try {
$data = file_get_contents($filepath);
if ($data === false) {
return false;
}
$jpeg = new PelJpeg($data);
$exif = $jpeg->getExif();
if ($exif === null) {
return true;
}
$tiff = $exif->getTiff();
if ($tiff === null) {
return true;
}
$ifd0 = $tiff->getIfd();
if ($ifd0 !== null) {
$this->removeExifFields($ifd0, $fields_to_keep);
$subIfd = $ifd0->getSubIfd(PelTag::EXIF_IFD_POINTER);
if ($subIfd !== null) {
$this->removeExifFields($subIfd, $fields_to_keep);
}
if (!in_array('GPS', $fields_to_keep)) {
$ifd0->removeEntry(PelTag::GPS_INFO_IFD_POINTER);
}
}
$jpeg->saveFile($filepath);
return true;
} catch (\Exception $e) {
return $this->stripExifFallback($filepath);
}
}
private function removeExifFields(PelIfd $ifd, array $fields_to_keep): void
{
$tags_to_remove = array_diff(array_keys($this->exif_to_pel_tags), $fields_to_keep);
foreach ($tags_to_remove as $field_name) {
$pel_tag = $this->exif_to_pel_tags[$field_name];
$entry = $ifd->getEntry($pel_tag);
if ($entry !== null) {
$ifd->removeEntry($pel_tag);
}
}
}
private function stripExifPng(string $filepath): bool
{
$image = @imagecreatefrompng($filepath);
if ($image === false) {
return false;
}
imagealphablending($image, false);
imagesavealpha($image, true);
$result = imagepng($image, $filepath, 9);
imagedestroy($image);
return $result;
}
private function stripExifGif(string $filepath): bool
{
$content = file_get_contents($filepath);
if ($content === false) {
return false;
}
if (strpos($content, "\x21\xF9\x04") !== false || strpos($content, 'NETSCAPE2.0') !== false) {
return true;
}
$image = @imagecreatefromgif($filepath);
if ($image === false) {
return false;
}
$result = imagegif($image, $filepath);
imagedestroy($image);
return $result;
}
private function stripExifWebp(string $filepath): bool
{
if (!function_exists('imagecreatefromwebp')) {
return false;
}
$image = @imagecreatefromwebp($filepath);
if ($image === false) {
return false;
}
$result = imagewebp($image, $filepath, 100);
imagedestroy($image);
return $result;
}
private function stripExifFallback(string $filepath): bool
{
$content = file_get_contents($filepath);
if ($content === false) {
return false;
}
$markers = [];
$offset = 0;
while ($offset < strlen($content)) {
if ($offset + 4 > strlen($content)) {
break;
}
$marker = ord($content[$offset + 1]);
if (ord($content[$offset]) !== 0xFF) {
break;
}
if ($marker >= 0xE0 && $marker <= 0xEF) {
$marker_len = ord($content[$offset + 2]) * 256 + ord($content[$offset + 3]);
$markers[] = [$offset, $marker_len + 2];
$offset += $marker_len + 2;
} elseif ($marker === 0xD8 || $marker === 0xD9) {
$offset += 2;
} elseif ($marker === 0x00 || $marker === 0xD0 || $marker === 0xD1 || $marker === 0xD2 || $marker === 0xD3 || $marker === 0xD4 || $marker === 0xD5 || $marker === 0xD6 || $marker === 0xD7) {
$offset += 2;
} elseif ($marker === 0x01) {
$offset += 2;
} else {
if ($offset + 4 > strlen($content)) {
break;
}
$marker_len = ord($content[$offset + 2]) * 256 + ord($content[$offset + 3]);
$offset += $marker_len + 2;
}
}
if (empty($markers)) {
return true;
}
$marker_names = [];
foreach ($markers as $marker_info) {
$marker_byte = ord($content[$marker_info[0] + 1]);
$marker_names[] = 'APP' . ($marker_byte - 0xE0);
}
log_message('warning', "stripExifFallback: Removing all APP markers from {$filepath}: " . implode(', ', $marker_names));
$new_content = $content;
foreach (array_reverse($markers) as $marker_info) {
$new_content = substr_replace($new_content, '', $marker_info[0], $marker_info[1]);
}
return file_put_contents($filepath, $new_content) !== false;
}
}

View File

@@ -3,6 +3,8 @@
* @var array $themes
* @var array $image_allowed_types
* @var array $selected_image_allowed_types
* @var array $exif_fields
* @var array $selected_exif_fields
* @var bool $show_office_group
* @var string $controller_name
* @var array $config
@@ -268,6 +270,26 @@
</div>
</div>
<div class="form-group form-group-sm">
<?= form_label(lang('Config.exif_fields_to_keep'), 'exif_fields_to_keep', ['class' => 'control-label col-xs-2']) ?>
<div class="col-xs-4">
<?= form_multiselect([
'name' => 'exif_fields_to_keep[]',
'options' => $exif_fields,
'selected' => $selected_exif_fields,
'id' => 'exif_fields_to_keep',
'class' => 'selectpicker show-menu-arrow',
'data-none-selected-text' => lang('Common.none_selected_text'),
'data-selected-text-format' => 'count > 1',
'data-style' => 'btn-default btn-sm',
'data-width' => '100%'
]) ?>
<label class="control-label">
<span class="glyphicon glyphicon-info-sign" data-toggle="tooltip" data-placement="right" title="<?= lang('Config.exif_fields_to_keep_tooltip') ?>"></span>
</label>
</div>
</div>
<div class="form-group form-group-sm">
<?= form_label(lang('Config.gcaptcha_enable'), 'gcaptcha_enable', ['class' => 'control-label col-xs-2']) ?>
<div class="col-xs-1">

View File

@@ -36,6 +36,7 @@
"codeigniter4/framework": "^4.6.3",
"dompdf/dompdf": "^2.0.3",
"ezyang/htmlpurifier": "^4.17",
"fileeye/pel": "^0.9",
"laminas/laminas-escaper": "2.17.0",
"paragonie/random_compat": "^2.0.21",
"picqer/php-barcode-generator": "^2.4.0",

4
composer.lock generated
View File

@@ -4,7 +4,7 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
"This file is @generated automatically"
],
"content-hash": "3fe9e4622879914bfa763b71c236c7fe",
"content-hash": "a4b6312de59646d276c42d832d10d0cf",
"packages": [
{
"name": "codeigniter4/framework",
@@ -5366,5 +5366,5 @@
"php": "^8.1"
},
"platform-dev": [],
"plugin-api-version": "2.3.0"
"plugin-api-version": "2.6.0"
}