Fix GH-17658: COMPersistHelper::LoadFromStream() can segfault #17659
Conversation
This file contains hidden or 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
The actual fix is trivial, but to be able to test the behavior we have to introduce an own COM object, since existing persistable objects likely implement `IPersistInit`, not only `IPersist`. We also want to avoid further test dependencies on possibly unavailable objects, such as `Word.Application`. To this purposes, we add a small COM in-process server, which may be extended for other testing purposes. We keep it simple by implementing it in C++, but without using any more sophisticated frameworks like ATL. This component needs to be built explicitly (`nmake comtest.dll`), and also needs to be explicitly registered (`nmake register_comtest`). When no longer needed, it is possible to unregister the component (`nmake unregister_comtest`).
devnexen
reviewed
Feb 1, 2025
devnexen
reviewed
Feb 1, 2025
`$(LINK)` may contain spaces, clang-cl does not recognize `/Fo:`, and the output folder may already exist.
This can be useful for debugging.
|
Noticed that we may swallow a helpful php-src/ext/com_dotnet/com_com.c Line 145 in 0d8aebf Will have a look into that in a different PR. |
devnexen
reviewed
Feb 2, 2025
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.
The actual fix is trivial, but to be able to test the behavior we have to introduce an own COM object, since existing persistable objects likely implement
IPersistInit, not onlyIPersist. We also want to avoid further test dependencies on possibly unavailable objects, such asWord.Application.To this purposes, we add a small COM in-process server, which may be extended for other testing purposes. We keep it simple by implementing it in C++, but without using any more sophisticated frameworks like ATL.
This component needs to be built explicitly (
nmake comtest.dll), and also needs to be explicitly registered (nmake register_comtest). When no longer needed, it is possible to unregister the component (nmake unregister_comtest).TODO
COMPersistHelper::InitNew()in the test case to ensure thatIPersistInitis not implemented (might later happen by accident, rendering the regression test useless)