Skip to content

Throw DomException for DOM out-of-memory error conditions #7049

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

Closed
wants to merge 3 commits into from

Conversation

nikic
Copy link
Member

@nikic nikic commented May 26, 2021

A number of error conditions in DOM can only occur if libxml2 runs out of memory, at least as far as I can see. In such cases we currently do a silent "return false", which violates the DOM spec, and which code is very unlikely to handle sensibly.

Switch these to throw a DomException with INVALID_STATE_ERR type. This error type is chosen because we use for similar checks elsewhere, for example:

if (!nodep) {
php_dom_throw_error(INVALID_STATE_ERR, 1);
RETURN_THROWS();
}

This changes some of the more obvious cases I spotted, but there are probably more.

cc @kocsismate @beberlei

@nikic nikic changed the title Use E_ERROR for some DOM out-of-memory error conditions Throw DomException for DOM out-of-memory error conditions Jul 16, 2021
@nikic
Copy link
Member Author

nikic commented Jul 16, 2021

I've pivoted this to throw a custom DomException instead of a fatal error. I think doing that should be uncontroversial...

@nikic
Copy link
Member Author

nikic commented Jul 16, 2021

Okay, I ended up switching this to use INVALID_STATE_ERR instead, after I realized that we already have quite a few checks like these and they seem to pretty consistently use INVALID_STATE_ERR, so just stick with that. For example:

if (!nodep) {
php_dom_throw_error(INVALID_STATE_ERR, 1);
RETURN_THROWS();
}

@nikic nikic closed this in c2a58ab Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants