Skip to content

EXIF stripping improvements#4405

Open
jekkos wants to merge 7 commits intomasterfrom
review-pr-4394
Open

EXIF stripping improvements#4405
jekkos wants to merge 7 commits intomasterfrom
review-pr-4394

Conversation

@jekkos
Copy link
Member

@jekkos jekkos commented Mar 6, 2026

Summary

This PR addresses all review comments from #4394:

✅ Completed Changes

  1. PSR-12 Compliance: Renamed strip_exif() to stripEXIF() (app/Libraries/Image_lib.php:7)

  2. Configuration Options: Added two new configuration keys:

    • exif_stripping_enabled: Toggle to enable/disable EXIF stripping (defaults to '0' for backward compatibility)
    • exif_fields_to_keep: Comma-separated list of EXIF fields to preserve (defaults to 'Copyright,Orientation')
  3. Conditional Stripping: Updated Config and Items controllers to only strip EXIF when enabled:

    • app/Controllers/Config.php:356-363
    • app/Controllers/Items.php:781-788
  4. Migration: Created migration to add new config keys to existing installations

📝 Design Decisions

Following the reviewer's "Good, Better, Best" framework:

  • ✅ EXIF stripping is now optional (addressing "Better" and "Best" requirements)
  • ✅ Users can opt-in via settings (backward compatible, default disabled)
  • ✅ Configurable field preservation for Copyright, Orientation, etc. (addressing "Best" requirement)
  • 🔜 For full "Best" implementation, a UI setting panel would be added in a future PR

🔒 Security Benefits

  • Privacy-conscious option to remove geolocation and device info
  • Preserves ethical copyright metadata when enabled
  • Gives admins control over their privacy approach

🔧 Testing

  • Syntax validated
  • Migration tested
  • Ready for integration with original PR

Closes #4394 review comments

@jekkos jekkos changed the base branch from fix/issue-4010-exif-stripping to master March 6, 2026 14:02
jekkos added 3 commits March 6, 2026 15:02
- 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
- 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)
- 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.
@jekkos jekkos changed the title Address review comments - EXIF stripping improvements EXIF stripping improvements Mar 6, 2026
@jekkos jekkos requested a review from objecttothis March 6, 2026 14:03
objecttothis
objecttothis previously approved these changes Mar 8, 2026
objecttothis
objecttothis previously approved these changes Mar 9, 2026
- Add checkbox to enable/disable EXIF stripping
- Add multiselect dropdown for EXIF fields to keep (Make, Model, Orientation, Copyright, Software, DateTime, GPS)
- Place UI in General Settings after Image Restrictions section
- Add JavaScript to enable/disable multiselect based on checkbox state
- Follows existing UI patterns (similar to image_allowed_types multiselect)
- Integrates with existing backend migration and Image_lib implementation

Closes #4405 - EXIF stripping improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants