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

[BUG] Empty PNG causes exception #3661

Closed
DougRogers opened this issue Nov 11, 2022 · 20 comments
Closed

[BUG] Empty PNG causes exception #3661

DougRogers opened this issue Nov 11, 2022 · 20 comments

Comments

@DougRogers
Copy link


    ImageSpec config;
    config["oiio:UnassociatedAlpha"] = 1;
    bool success                     = false;
    auto inp                         = ImageInput::open(filePath.toStdWString(), &config);

image

arrow-down.zip

@lgritz
Copy link
Collaborator

lgritz commented Nov 11, 2022

I'm not able to reproduce this. Here's what I know:

Despite it being a "png" file, it's 0 length and the debug screen capture you attached makes it clear that the actual exception in OpenEXR. (This is presumably because after the png reader fails to open the corrupt file, it will try to open it with all format readers before completely giving up, and along the way, the openexr reader hits an uncaught exception.)

The trace also makes it seem that you're using is labeled as OIIO 2.4.0, which is a fairly early point in the development history of 2.4 circa early in 2022, and also that the version of openexr you're using is 2.5 (hard to tell which release, exactly).

Since the openexr 2.5 days, a lot has improved -- much due to the OpenEXR team responding quickly to reports from people who fuzz test it mercilessly. I suspect that this is a long fixed issue on their side.

It's hard to be sure. I tried with OIIO top of master, as well as a couple different points along the early 2.4.0 development (I could only spot check a couple places, not sure if they correspond to your build), and also by building those against a build of OpenEXR 2.5 (just guessing which patch release to try, it may not be exactly what you're using), and I was unable to reproduce under any of those combinations.

I'd like to ask you to try on your end with a modern release version of OIIO 2.4 (the last release was 2.4.5.0) and if possible, build against a modern OpenEXR (the latest is 3.1.5 -- though it may be a lot to ask because if you have other components that need OpenEXR, it's not necessarily a simple task to jump from 2.x to 3.x).

If you can't reproduce after upgrading one or both of those, then let's chalk this up to a bug long ago fixed. Both OIIO and OpenEXR have been hardened against corrupt files pretty steadily lately.

If you can still reproduce with the current OIIO 2.4.5 or with the current master, then that's still progress, especially if you can confirm the specific tag or commit of OpenEXR you were using. Then at least I can test using the same code as you, and we'll see what the next step is to attack this.

@DougRogers
Copy link
Author

I will update both and retest. I am currently using the OpenEXR in vcpkg, but will put the current version and use that.

@DougRogers
Copy link
Author

DougRogers commented Nov 12, 2022

I am not able to build OIIO using cmake for Windows.

I download boost to this folder C:\Users\Roger\source\repos\External Projects\boost and used the bootstrap and b2 commands.
I set Boost_Root to C:\Users\Roger\source\repos\External Projects\boost, but that did not work.
I also tried several of the subdirectories in the boost dir, but they did not work either.

What am I missing?

�[31mBoost library not found �[m
�[31m    Boost_ROOT was: C:\Users\Roger\source\repos\External Projects\boost �[m
CMake Error at src/cmake/checked_find_package.cmake:177 (message):
  �[31mBoost is required, aborting.�[m
Call Stack (most recent call first):
  src/cmake/externalpackages.cmake:65 (checked_find_package)
  CMakeLists.txt:162 (include)

@lgritz
Copy link
Collaborator

lgritz commented Nov 12, 2022

I'm not sure you need to go to all that trouble.

I think that vcpkg has recently upgraded OpenEXR to 3.1:
https://github.com/microsoft/vcpkg/blob/master/ports/openexr/portfile.cmake
microsoft/vcpkg#26862

If you just upgrade the vcpkg version of openexr, that should solve half of the equation. Looks like the vcpkg port of OpenImageIO is still 2.3, but I would expect somebody to upgrade it to the new 2.4 relatively soon (it's only been the official release for a month).

@DougRogers
Copy link
Author

I can use the OpenEXR from vcpkg, but I can't get the cmake for oiio to recognize boost at all. I can't use vcpkg's version of oiio, because bugs have been fixed that I depend on.

So I am stuck trying to get oiio's make file to work and I can't get it to.

@DougRogers
Copy link
Author

I was able to locate the directory in vcpkg that that cmake was looking for, so I am past the boost error. I should be able to build from master now.

@lgritz
Copy link
Collaborator

lgritz commented Nov 12, 2022

Or just a newer 2.4 is fine, too. If it works with OpenEXR 3.1 and the current (2.4) release of OIIO, that will be an easier transition for you than moving to master and possibly having other compatibility issues to grapple with.

@DougRogers
Copy link
Author

DougRogers commented Nov 12, 2022

Unfortunately, I am still getting the boost error. I may have to hack the cmake file so it doesn't error on boost not found.

How do I sync to a specific version of oiio? I am not git savvy, so I just clone from the repo.

@DougRogers
Copy link
Author

I see, I just select the branch and clone that. What branch is the 2.4 release?

@lgritz
Copy link
Collaborator

lgritz commented Nov 12, 2022

"release" is always the latest release!

But the permanent tag for the latest release today, Nov 12, is "v2.4.5.0".

@lgritz
Copy link
Collaborator

lgritz commented Nov 12, 2022

Fresh clone: git clone https://github.com/OpenImageIO/oiio -b release

If you already have a repo checked out, then in it just do git checkout release

@DougRogers
Copy link
Author

Thank you. Pulling release. I will have another go at trying to get cmake to recognize boost again.

@DougRogers
Copy link
Author

I was able to use that command to pull release and to build, but the namespace is still v2_3. It looks like that is the version I pulled. Where can I look in the source to get the version info?

@DougRogers
Copy link
Author

Ignore that. I think it was the old vcpkg build

@DougRogers
Copy link
Author

DougRogers commented Nov 13, 2022

OpenEXR 3 throws on bad input

class OpenEXRInputStream final : public Imf::IStream {
public:
    OpenEXRInputStream(const char* filename, Filesystem::IOProxy* io)
        : Imf::IStream(filename)
        , m_io(io)
    {
        if (!io || io->mode() != Filesystem::IOProxy::Read)
            throw Iex::IoExc("File input failed.");
    }
    bool read(char c[], int n) override
    {
        OIIO_DASSERT(m_io);
        if (m_io->read(c, n) != size_t(n))
            throw Iex::IoExc("Unexpected end of file.");
        return n;
    }

@lgritz
Copy link
Collaborator

lgritz commented Nov 13, 2022

And do we not catch it?

@DougRogers
Copy link
Author

DougRogers commented Nov 13, 2022

Yes, it is caught by oiio. I am have not closely looked at what Iex::IoExc is, though.

bool
OpenEXRInput::valid_file(const std::string& filename,
                         Filesystem::IOProxy* io) const
{
    try {
        std::unique_ptr<Filesystem::IOProxy> local_io;
        if (!io) {
            io = new Filesystem::IOFile(filename, Filesystem::IOProxy::Read);
            local_io.reset(io);
        }
        OpenEXRInputStream IStream(filename.c_str(), io);
        return Imf::isOpenExrFile(IStream);
    } catch (const std::exception& e) {
        return false;
    }
}

@lgritz
Copy link
Collaborator

lgritz commented Nov 13, 2022

So is the current hypothesis that the version of OIIO you were using originally was sufficiently old that we didn't have the try/catch around this spot?

@DougRogers
Copy link
Author

I would have to go back to see what changed in OIIO and OpenEXR, but there is a proper throw/catch mechanism in place now.

@lgritz
Copy link
Collaborator

lgritz commented Nov 13, 2022

Good enough. If it's fixed in the modern supported versions of the two packages, that's really all we could have aimed for anyway.

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

No branches or pull requests

2 participants