Global namespace collision "Error"

[eparisca]

As soon as I upgraded from ramsey/uuid 3.2 to 3.3 via composer, my servers started reporting the following PHP Fatal error:

PHP Fatal error: Call to undefined method Error::GetXML() in [script name]

Downgrading to 3.2, which also removes the prerequisite paragonie/random_compat v2.0.2, fixed the issue.

Issue appears to be in paragonie/random_compat extending Exception with the global name Error.

 // We can't really avoid making this extend Exception in PHP 5.
class Error extends Exception

See:
https://github.com/paragonie/random_compat/blob/master/lib/error_polyfill.php

Cheers.

[paragonie-scott]

What project defined an Error class?

[eparisca]

Our own software uses an Error class, which does not extend Exception.

[paragonie-scott]

Okay, the best solution I can think of that will fix the problem immediately is to make sure your Error class is defined before you include vendor/autoload.php. Note that your code won't work on PHP 7 with the current naming convention, irrespective of what random_compat does.

[eparisca]

@paragonie-scott thank you for the prompt response.

We use an autoloader to include the Error class only when needed. So including it prior to vendor/autoload.php goes against our internal development rules.

For now, we have downgraded ramsay/uuid to 3.2, which does not make use of paragonie/random_compat yet. This will also prevent the issue temporarily for us.

[paragonie-scott]

Note that 3.2 runs the risk of encountering this problem: ramsey/uuid#80

[eparisca]

Yes, thank you for pointing that out. That was the reason for upgrading, although we have not detected UUID v4 collisions, yet. We are nowhere near generating that amount of UUIDs in PHP yet. We leverage PostgreSQL's UUID generator as well as client-side UUID generators.

[ramsey]

If you're using PostgreSQL and client-side code to generate UUIDs, then you probably won't run into the collision problem. The collision problem occurs when you are using OpenSSL in PHP running in an forked environment (i.e. Apache, FPM) to generate UUIDs.

[paragonie-scott]

It sounds like this was resolved on your end, but I'd like to leave this issue open with a general commentary in case someone else encounters the same problem.

The function of any polyfill library is to expose the new way of doing things on platforms that don't have those features. The purpose of any polyfill library is to enable developers to write code the new way even in environments that don't currently offer the capability to do things the new way out of the box.

If your code defines an Error class in the global namespace, your code isn't PHP 7 compatible. This random_compat issue constitutes an early warning before you upgrade.

The workaround is to import random_compat after your custom Error class has been defined, but the best solution is one of:

• Rename your class to something that isn't reserved
• Toss it into a namespace, and them import it with use

[kelunik]

@paragonie-scott: You might want to check for Error with autoloading and check whether it extends Exception / Throwable.

[paragonie-scott]

Do you mean here? https://github.com/paragonie/random_compat/blob/master/lib/error_polyfill.php#L37

[kelunik]

Yes. Would ensure there's no custom Error class included before. Other libraries might polyfill Error as well, but then I think it's always extended from Exception.

[paragonie-scott]

e3753fe Does that look good?

[kelunik]

I think we should error out instead of extending TypeError directly from Exception in that case.

If you define a custom Error class, it's just not compatible with random_compat and additionally isn't compatible with PHP 7 either. So it's something the one defining Error differently has to fix anyway.

[kelunik]

I'd write it like that:

if (!class_exists('Error')) {
    class Error extends Exception { }
} else if (!is_subclass_of('Error', 'Exception')) {
    throw E_FATAL;
}

if (!class_exists('TypeError')) {
    class TypeError extends Error { }
} else if (!is_subclass_of('TypeError', 'Error')) {
    throw E_FATAL;
}

That should ensure it's always the same type hierarchy.

[paragonie-scott]

I've been thinking about this for a while, and I think the solution is "document this in the README" and not try to handle these corner cases.

[paragonie-scott]

e3753fe