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.
It never belonged inside "tools". Also rename its "pdb" subdirectory
to "groups". This had to happen before 2.10 so cherry-picking between
branches doesn't become a nightmare in the future.
Poppler and poppler-data are now hard dependencies.
PDF is a common-enough format nowadays that import support is likely
considered as a granted feature by everyone. Moreover the current
fallback to the PostScript plug-in for PDF support just gives a degraded
experience with less features (and probably a lot of bugs since
basically nobody uses this code).
Poppler-data is also considered mandatory because non-western language
support should never be considered an "option". People using non-western
languages are not second class citizens and therefore if we say that PDF
import is now a hard feature, it should also include PDF using CJK or
Cyrillic languages.
Since SRTM-1 and SRTM-3 data have a fixed image size, it is actually
very easy to auto-detect these by checking the file size. There is no
need to ask the user to select between the 2 variants (even though these
values are made quite obvious by official download links).
I still left the dropdown appear optionally if the detection fails for
some reason (we never know, at least that makes a fallback, for instance
to be able to load partial data!). Yet now by default, HGT file settings
should be fully auto-detected.
g_set_error() so that when samplespacing is not a valid argument for
file-hgt-load, the error is properly propagated.
Also add a g_warning() to get the error in the terminal as well.
