Skip to content

Mark all existing options as non-updateable - #377

Open
sirzooro wants to merge 1 commit into
pion:mainfrom
sirzooro:options_not_updateable
Open

Mark all existing options as non-updateable#377
sirzooro wants to merge 1 commit into
pion:mainfrom
sirzooro:options_not_updateable

Conversation

@sirzooro

Copy link
Copy Markdown
Contributor

Initial changes to allow changing selected options at runtime. This is needed for Cryptex implementation.

Initial changes to allow changing selected options at runtime. This is
needed for Cryptex implementation.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a constructed flag on Context and uses it to make all existing ContextOptions reject mutation after context construction, establishing a baseline for future “selectively updatable options” work (notably for Cryptex).

Changes:

  • Add Context.constructed and set it during CreateContext to gate option updates.
  • Make all existing options in option.go return ErrContextOptionNotUpdatable after construction.
  • Add/adjust tests to validate the new non-updatability behavior and update a test helper to mark contexts as constructed.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
context.go Adds the constructed flag to Context and sets it during CreateContext.
errors.go Introduces ErrContextOptionNotUpdatable for post-construction option mutation attempts.
option.go Adds constructed-state checks to all existing ContextOptions.
option_test.go New test coverage for option “set during construction only” behavior.
srtp_cipher_utils_test.go Marks test-created contexts as constructed to align with new option gating.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread option.go
Comment on lines 14 to +18
func SRTPReplayProtection(windowSize uint) ContextOption { // nolint:revive
return func(c *Context) error {
if c.constructed {
return ErrContextOptionNotUpdatable
}
Comment thread option_test.go
Comment on lines +58 to +62
return func(c *Context) error {
c.newSRTPReplayDetector = func() replaydetector.ReplayDetector { return &testReplayDetector{} }

return SRTPNoReplayProtection()(c)
}
Comment thread option_test.go
Comment on lines +74 to +78
return func(c *Context) error {
c.newSRTCPReplayDetector = func() replaydetector.ReplayDetector { return &testReplayDetector{} }

return SRTCPNoReplayProtection()(c)
}
Comment thread option_test.go
Comment on lines +122 to +126
return func(c *Context) error {
c.encryptSRTP = false

return SRTPEncryption()(c)
}
Comment thread option_test.go
Comment on lines +136 to +140
return func(c *Context) error {
c.encryptSRTP = true

return SRTPNoEncryption()(c)
}
Comment thread option_test.go
Comment on lines +150 to +154
return func(c *Context) error {
c.encryptSRTCP = false

return SRTCPEncryption()(c)
}
Comment thread option_test.go
Comment on lines +164 to +168
return func(c *Context) error {
c.encryptSRTCP = true

return SRTCPNoEncryption()(c)
}
Comment thread context.go
Comment on lines +83 to +85
// constructed is set to true after the Context is fully initialized.
// Options can check this flag to reject updates that are only valid during construction.
constructed bool
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants