Comment 1 Florian Best 2014-10-23 12:10:33 CEST

JPEG does not allow transparency and is larger for some files.

Comment 2 Alexander Kläser 2014-10-23 13:43:29 CEST

(In reply to Florian Best from comment #1)
> JPEG does not allow transparency and is larger for some files.

JPEG is more adapted to real-world images and it comes with an appropriate (lossy) compression for these. PNG comes with a lossless compression and is better suited for "artificial" graphics, i.e., with homogeneous coloured areas or sharp edges.

Therefore, I see technically no reason why to support PNG.

Comment 3 Stefan Gohmann 2014-12-03 09:53:23 CET

(In reply to Alexander Kläser from comment #2)
> Therefore, I see technically no reason why to support PNG.

I've received reports that tester tried to upload a PNG image. I think UMC should convert the PNG to a JPEG image.

Comment 4 Alexander Kläser 2014-12-03 15:49:48 CET

(In reply to Stefan Gohmann from comment #3)
> (In reply to Alexander Kläser from comment #2)
> > Therefore, I see technically no reason why to support PNG.
> 
> I've received reports that tester tried to upload a PNG image. I think UMC
> should convert the PNG to a JPEG image.

Good idea.

Comment 5 Alexander Kläser 2014-12-03 17:01:04 CET

For better usability, the correct file format needs to be verified directly when uploading the image.

Comment 6 Florian Best 2014-12-03 19:27:17 CET

Why converting? Why not accepting both formats?
Converting has the disadvantage of information loss. Converting maybe fails due to different PNG formats? Maybe also security issues.

Comment 7 Stefan Gohmann 2014-12-03 19:51:30 CET

(In reply to Florian Best from comment #6)
> Why converting? Why not accepting both formats?
> Converting has the disadvantage of information loss. Converting maybe fails
> due to different PNG formats? Maybe also security issues.

I think we are using the attribute jpegPhoto from the inetorgperson schema.

Comment 8 Alexander Kläser 2014-12-04 12:57:45 CET

(In reply to Stefan Gohmann from comment #7)
> I think we are using the attribute jpegPhoto from the inetorgperson schema.

Yep: https://tools.ietf.org/html/rfc2798#section-2.6

Comment 9 Florian Best 2015-01-08 15:51:11 CET

Created attachment 6583 [details]
patch

Attached a patch which uses PIL to convert PNG's to JPEG in memory.
It adapts the jpegPhoto syntax. Should we create a new syntax class for it?

Comment 10 Florian Best 2015-01-22 17:01:37 CET

Patch has been applied (with more error handling) in svn r57500.

YAML: 2015-01-22-univention-directory-manager-modules.yaml
Package: univention-directory-manager-modules
Version: 10.0.29-19.1283.201501221644
Branch: ucs_4.0-0
Scope: errata4.0-0
no cross dependencies.

Comment 11 Florian Best 2015-01-23 12:34:51 CET

In svn r57517 one debug line has been removed and IndexError is also catched as PIL raises this with an corrupted PNG file, too.
I tested for myself to upload valid PNG files, non-PNG files, PNG files with only the PNG header ('\x89PNG\r\n\x1a\n') and corrupted PNG files (somewhere in the middle cutted).

Comment 12 Drees Dormann 2015-01-26 14:47:23 CET

Upload of .png files (valid, wrong format, truncated file, header only) OK

YAML OK

Comment 13 Janek Walkenhorst 2015-01-29 11:45:08 CET

<http://errata.univention.de/ucs/4.0/65.html>

Comment 14 Florian Best 2015-07-07 17:34:17 CEST

I added a test script which tests setting of JPG, JPEG, PNG and unsetting in svn r61854.