Harden stream lifetime
Commit Message: made 2 interface changes to harden the life time interaction between codec server streams and HCM or between codec client streams and UpstreamRequest.
-
Added
StreamCallbacksRegistryinterface to cooperate withHttp::StreamCallbacksto cross reference each other so that when StreamCallbacks objects go out of life time, it will unregister itself from the registry implementation StreamCallbacksHelper and vice versa. -
Added a new
ObjectHandleinterface to wrap request/response encoder and decoder objects and a new functioncreateHandle()to Request/Response Encoder and Decoder interfaces to return a handle to themselves. When these encoder and decoders get destroyed, they will clear the reference in the handle object so that the owners of those handles can no longer access encoder and decoder objects. This is pretty much similar toweak_ptr, but the handle interface also provide the ability to cache the destruction state of the encoder or decoder for debugging purpose. The existingStreamCallbacksinterface together with conditionalrunResetCallbacks()should have already guaranteed that the decoder and encoder objects shouldn't be accessed after the end of the stream life time. But such contract and the condition can easily get wrong in both side and missing callbacks indeed happens in production. The destruction state in the handle could be used to debug how the callbacks are missed. And this PR also changed the code to switch to hold an ObjectHandle instead of the raw pointer at a few places, i.e. HCM and HTTP/3 and HTTP/2 codec.
Additional Description: this PR doesn't make change to HTTP/1 codec. And it only added destruction state to HTTP/3 codec.
Risk Level: Testing: Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional API Considerations:]
As a reminder, PRs marked as draft will not be automatically assigned reviewers, or be handled by maintainer-oncall triage.
Please mark your PR as ready when you want it to be reviewed!
@alyssawilk @RyanTheOptimist I'm planning to propose these interface changes to harden the stream lifetime interaction between codec and HCM. It added some implementation code into the encoder/decoder interfaces unfortunately, but that's where I found most appropriate. Let me know what you think.
I'm also debating if I should runtime guard this change, given the whole thing is to protect UAF.
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!
This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!
Still working on it.