JPEG 2000 codestream doesn't have a header and guessing the color space
in particular is not foolproof (especially when 3 or 4 components, which
can be many spaces). Therefore the need of a parameter on the API.
Note that JP2 images should always have the color space information. In
interactive mode, I try to be a bit flexible to salvage broken JP2 with
no color space information in the header, but I am not adding a
parameter in file-jp2-load() (on purpose, since we are not going to add
in the API a parameter for a case not supposed to happen with properly
encoded files).
JPEG 2000 codestream (.j2k/.j2c) are only compressed code stream data,
without header. In particular we don't have color information, such as
the color space. So we need to open a dialog asking to set the color
space in interactive mode.
Note: according to OpenJPEG developers, a JP2 image (not codestream)
should always have a color space defined in its header. But just to be
flexible, the same dialog may get raised as well if we try to load a JP2
with no valid color space defined in header and no ICC profile embedded.
Maybe if such a thing happened, it means the image is corrupt, yet we
may as well try and salvage it anyway.
Note 2: I also removed a weird test which was setting some images as
being YUV color space by mistake. This actually fixes bug 794413 as a
side effect.
Even though I haven't seen working samples with this extension,
according to some references, this is a common extension for compressed
JPEG 2000 code stream. Also our old plug-in was listing this extension,
so let's do so now as well.
To this day, the only 2 extensions we used to list in the JasPer-based
plug-in and not in the OpenJPEG one are .jpf and .jpx (JPEG 2000 Part-2)
since OpenJPEG does not have support yet. But actually I think the old
plug-in may have simply been "lying" since JasPer website says the
library is meant to implement JPEG-2000 Part-1 standard.
So I believe we are now on par (and even better on many aspects) with
the former plug-in implementation based on libjasper.
After moving up the profile extraction, I was running
gimp_image_set_color_profile() with a non-existing image id, which was
obviously wrong. Reorder a bit the operations.
Also try to guess the color space from the profile not only with
OPJ_CLRSPC_UNSPECIFIED but also OPJ_CLRSPC_UNKNOWN images. Indeed I
encountered a case of .jp2 image with no color space in the header, but
with an embedded profile. And unlike the .j2c files I encountered
earlier, the color space was now *_UNKNOWN.
See https://github.com/uclouvain/openjpeg/issues/1103
Current OpenJPEG code only supported the base JP2 container. It now
supports also the JPEG 2000 codestream (which is usually contained
inside other formats, like the JP2 container format, but can also
sometimes be on its own).
The current magics and extension strings were also mixing all kind of
formats. This is now cleaned up a bit.
As explained in the previous commit, the color space is not always
properly declared, in particular with J2K files. If a profile is present
in such a case, try to deduct the color space from this information.
It seems that the color space is not necessarily declared for a JPEG2000
image. From tests, it looks like it especially happens with JPEG2000
codestream (.j2c or .j2k). This variant is apparently mostly designed to
be embedded (from what I read), which may explain why the color space is
not always set (I assume the embedding format would have the color space
information). Mostly a guess.
Rather than just assuming all non-gray images are RGB, do a bit more
robust check and reject unknown formats. Indeed even though I see we
took care of YUV, e-YCC and CMYK images above (and normally either
converted them to RGB or already exited with an error), I can see that
the OpenJPEG library could still return OPJ_CLRSPC_UNKNOWN or
OPJ_CLRSPC_UNSPECIFIED. Let's be thorough and not assume we got a SRGB
here.
Also add the alpha-variant tests inside their parent image type
respective test. This should not change anything by any logics, but
let's not leave anything for chance to strike us.
Finally minor coding style fixes:
- Add a space before "if|for" and parenthese.
- Remove some spaces after parentheses.
- Get rid of 2 trailing whitespaces.
- Align function call parameters, declarations, assignments…
Made plug-in support the RGB and grayscale with alpha.
Comment by Jehan: this makes the original branch work finally usable on
some JPEG 2000 images. Support of the format is not complete yet though
but at least the port to OpenJPEG is now in usable test.
Use the new gimp_get_pdb_status() to forward the error returned by
gimp_file_load(). Previous code was always returning
GIMP_PDB_EXECUTION_ERROR when the file load was failing, but this was
not granular enough. In particular when the file load is actually
interactively cancelled through Esc or the "Cancel" button, we don't
want to display an error message on screen. Therefore we forward the
actual error raised by the underlining plug-in.
While we are at it, let's just add RGBE exporting. It's just as easy.
Also rename s/file-gegl-load-rgbe/file-load-rgbe/. All formats just use
the "file-FORMAT-(load|save)" naming for their procedure, even when
implemented directly through "gegl:load|save".
It seems that various software use something different after the "#?",
and even Blender code just ended up only use these 2 characters as magic
number. See also bug 792453.
Various plug-ins exporting metadata should now follow preferences, which
would override any default. Of course these preferences can still be
overriden by saved settings (global parasite), previous run settings,
and finally through the GUI when interactive.
This is a privacy concern. Whereas importing metadata is usually a good
idea, exporting it should be a conscious action. A lot of private data
can be leaked through metadata and many people don't realize it (which
also usually means they don't need it). On the other hand, the people
who realize it are the ones who would explicitly edit the metadata and
check what they want to be exported or not.
This is only a first step. Some people may want to always export the
metadata and for these people, there should be abilities to change the
default.
After discussing with Mitch and understanding better the X bitmap/pixmap
history, I make the warning more specific to X bitmap cursors only (not
pixmap).
Also I can see our code always exports RGBA data, so I am not quite sure
if this warning even makes sense since X bitmaps are bicolor maps. On
the other hand, Mitch tells me that "these days gdk turns pixbufs into
bitmaps if the x server doesn't support rgba cursors", so maybe that can
still be of use to warn cursor designers for max compatibility.
Still that's pretty old compatibility stuff, so let's replace "may" by
"might".
Leading 0s have no special value, we use base 10 anyway. Removing
leading 0s allows to not trigger the 8-digit test, hence modify a valid
cursor size unecessarily.
We were basing our max export size on a macro value defined in
libXCursor code: MAX_BITMAP_CURSOR_SIZE. This macro is still defined in
libXCursor and still has the same value (64), yet it is unsure how far
or even where this is enforced since it seems we can get at least 96px
cursors in GNOME/X11.
As a consequence, this commit:
- still warns when cursor size is over this value, with more explicit
text, yet does not change the cursor size anymore! So it is now
possible to export bigger cursors, but you still get a warning.
- only changes the cursor size for the existing more-than-8-digits test
and I add a warning when it does so (we should never modify an image
silently!).
- adds the size 96 as not triggering the warning about GNOME Settings
since it definitely looks like this size is valid there (according to
my own empirical tests). Also since 96 is higher than the libXCursor
current MAX macro value, this really raises the question to where this
max is enforced and whether we should not just drop the first warning.
Note that it breaks a bit the string freeze since I modify one string
and adds one. Sorry for this!
It is possible to trigger a heap overflow with insanely large GBR
files with a deprecated file format on 32 bit systems.
The problem is that old versions of GBR allowed an additional pattern
after the brush data. These patterns have always 4 bytes per pixel,
but the initial size check is performed with the bytes per pixel of
the brush, which can be different.
If the brush has 1 byte per pixel and the dimensions are sufficiently
large, this can trigger a heap overflow with attacker-controlled
amount and content of data.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
This reverts commit 06d24a79af.
The CVE is still fixed but now in a different way. Commit 4fa0cd4dcf
passes instead the accurate string length when using the string, hence
making it work even when not NUL-terminated. This has the advantage of
having the GBR file loaded in the end, despite such file format error. I
am personally not persuaded this is the best path since a file with such
an error may either be corrupted, or worse may have been constructed on
purpose to be harmful, so rejecting it directly may be the safe choice.
Nevertheless I may also be too doubtful and maybe trying to save a
slightly corrupted file may be the nicest choice indeed.
The file formats GBR and PAT contain names which are supposed to be
NUL-terminated within the files. If no such terminating NUL byte
exists, the parsers of GBR and PAT trigger an out of boundary read
during utf-8 conversion.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Some g_message() stay when they are used as debug or warning message
(without actually cancelling load action). But all fatal errors now use
g_set_error().
... in PSP importer.
Check if declared block length is valid (i.e. within the actual file)
before going further.
Consider the file as broken otherwise and fail loading it.
According to some spec on the web, 16-bit RGB is also valid. In this
case, the last bit is simply ignored (at least that's how it is
implemented right now).
... TGA importer.
Be more thorough on valid TGA RGB and RGBA images.
In particular current TGA plug-in can import RGBA as 32 bits (8 bits per
channel) and 16 bits (5 bits per color channel and 1 bit for alpha), and
RGB as 15 and 24 bits.
Maybe there exist more variants, but if they do exist, we simply don't
support them yet.
Thanks to Hanno Böck for the report and a first patch attempt.
Jehan