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

Embedded non-free ICC profile in person.jpg(.gz) #5749

Closed
kapouer opened this issue Mar 16, 2016 · 10 comments
Closed

Embedded non-free ICC profile in person.jpg(.gz) #5749

kapouer opened this issue Mar 16, 2016 · 10 comments
Labels
meta Issues and PRs related to the general management of the project. test Issues and PRs related to the tests.

Comments

@kapouer
Copy link
Contributor

kapouer commented Mar 16, 2016

In nodejs 5.9.0,

identify -verbose test/fixtures/person.jpg | grep icc
   icc:copyright: Copyright (c) 1998 Hewlett-Packard Company
   icc:description: sRGB IEC61966-2.1
   icc:manufacturer: IEC http://www.iec.ch
   icc:model: IEC 61966-2.1 Default RGB colour space - sRGB
   Profile-icc: 3144 bytes

The profile is actually embedded - it's not only a reference to it.
IEC 61966-2.1 should have a free and open-source license, right ?
Sadly, no; see bottom of page http://www.color.org/srgbprofiles.xalter, quoting:

...permission to use, copy and distribute these file for any purpose
is hereby granted without fee, provided that the file is not changed
including the ICC copyright notice tag...

Which is in direct contradiction of chapter 3 of https://opensource.org/osd

To fix this: convert person.jpg +profile "icc" person.jpg
(do not forget to do it on person.jpg.gz as well).
Though the tests using person.jpg seems to assume its file size, so stripping several kilobytes from it might hurt a test.

I know, it's crazy - always strip and convert images before redistribution.

@mscdex mscdex added the test Issues and PRs related to the tests. label Mar 16, 2016
@Fishrock123
Copy link
Contributor

What does this realistically mean? The codec is copywrited? (Does that even matter?)

@kapouer
Copy link
Contributor Author

kapouer commented Mar 17, 2016

Since i don't know where exactly you don't get why it does matter, i'm trying to explain
a bit differently:

  • if the file is not redistributed in the nodejs tarball, there is practically no issue
  • copyright != license; everything has a copyright, not everything has a license to use it
  • here there is a binary blob in a file that turns out to be a complete sRGB profile with a
    copyright (all right) and a license explaining it is free to use it, free to redistribute it,
    but that one cannot modify that portion of the file - at all, ever - and redistribute it later.
  • now imagine there's a bug in the sRGB profile that would need to be fixed and redistributed;
    it would be forbidden by the license. But since most developers won't know that license
    issue they will make the modification, thus violating the license, thus exposing them to
    legal issues.

@MylesBorins
Copy link
Contributor

/cc @mikeal

@mikeal
Copy link
Contributor

mikeal commented Mar 17, 2016

While this is less important because it isn't in the distro tarball we should, as a matter of policy, restrict the licenses of files landing in the code base.

Can we replace with this a CC licensed file? We have a provision for CC so that docs can be CC licensed if necessary.

@MylesBorins
Copy link
Contributor

@kapouer what operating system are you on and what package is you identify coming from?

I ran the suggested command identify -verbose test/fixtures/person.jpg | grep icc on both OSX and Ubuntu and I'm not seeing the same output...

thealphanerd@thealphanerd-VirtualBox:~/temp$ identify -verbose person.jpg | grep icc
    Profile-icc: 3144 bytes

That is all I'm getting

@kenany
Copy link
Contributor

kenany commented Mar 17, 2016

@thealphanerd If it helps, I see it in ArchLinux:

$ identify -version
Version: ImageMagick 6.9.3-7 Q16 x86_64 2016-03-08 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2016 ImageMagick Studio LLC
License: http://www.imagemagick.org/script/license.php
Features: Cipher DPC HDRI Modules OpenCL OpenMP 
Delegates (built-in): bzlib cairo fontconfig freetype gslib jng jp2 jpeg lcms lqr ltdl lzma openexr pangocairo png ps rsvg tiff webp wmf x xml zlib

$ identify -verbose test/fixtures/person.jpg | grep icc
    icc:copyright: Copyright (c) 1998 Hewlett-Packard Company
    icc:description: sRGB IEC61966-2.1
    icc:manufacturer: IEC http://www.iec.ch
    icc:model: IEC 61966-2.1 Default RGB colour space - sRGB
    Profile-icc: 3144 bytes

@kapouer
Copy link
Contributor Author

kapouer commented Mar 17, 2016

@thealphanerd the one from ImageMagick 6.9.2-10 in debian/sid. You can try exiftool too - it should output similar information.

@MylesBorins
Copy link
Contributor

Homebrew is serving Version: ImageMagick 6.9.3-6 and Ubuntu has Version: ImageMagick 6.7.7-10

Since I am unable to reproduce would either of you be up for sending a PR that replaces those two files and updates the tests accordingly?

@Fishrock123 Fishrock123 added the meta Issues and PRs related to the general management of the project. label Mar 17, 2016
@Fishrock123
Copy link
Contributor

idea: can we just make these tests use the node.js logos in doc/?

@jbergstroem
Copy link
Member

@Fishrock123 ..or at least copy them?

kapouer added a commit to kapouer/node that referenced this issue Mar 20, 2016
Fishrock123 pushed a commit that referenced this issue Mar 22, 2016
Fixes: #5749
PR-URL: #5813
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
MylesBorins pushed a commit that referenced this issue Mar 30, 2016
Fixes: #5749
PR-URL: #5813
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
MylesBorins pushed a commit that referenced this issue Mar 30, 2016
Fixes: #5749
PR-URL: #5813
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

7 participants