Reset xmlXPathContext state variables before each evaluation #3386
+12
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[skip ci]
If we're re-using the xmlXPathContext object, there's a chance that the context variables will be trashed by recursive custom functions. In #3378 (comment), Nick advised: > Note that if you use a single XPath context and support custom XPath > extension functions, a custom function could evaluate XPath > expressions recursively which will lead to corruption of context > variables. This is mostly due to some design mistakes in libxml2. So let's set these context variables back to their default.
|
After chatting more with Nick, this approach is not right, closing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem is this PR intended to solve?
In #3378 we implemented re-used of xmlXPathContext objects for performance.
If we're re-using the xmlXPathContext object, there's a chance that the context variables will be trashed by recursive custom functions.
In #3378 (comment), @nwellnhof advised:
So let's set these context variables back to their default.
Have you included adequate test coverage?
Functional test coverage should be sufficient to show the features still work, but I did not add test coverage that would have revealed a bug.
Does this change affect the behavior of either the C or the Java implementations?
CRuby only fix