Skip to content
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

Add an explanation the need for Context's Restore operation. #490

Closed
carlosalberto opened this issue Feb 26, 2020 · 1 comment · Fixed by #563
Closed

Add an explanation the need for Context's Restore operation. #490

carlosalberto opened this issue Feb 26, 2020 · 1 comment · Fixed by #563
Assignees
Milestone

Comments

@carlosalberto
Copy link
Contributor

From #424 (comment)

  • Checking correctness - by having a restore that accepts a Token or just simply the previous Context you can check if the given value is the expected one (in other words you can see the calls to set as push to a stack and restore as pop where you check that the element you intend to pop is the expected one. This is critical and I've seen cases where auth tokens were leaked because of this issue (I don't think I can share too many things about this issue, but the root cause were people calling set without a paired restore).
  • This approach guarantees (at least tries to enforce because mistakes can still happen) that the Context is not "modified" by calling a function:
void bar() {
  Context current = Context.current();
  foo();
  // This should always be the case, otherwise for examples
  // credentials can be leaked here.
  assert(current == Context.current());
}

It can still happen if a Set is not followed by a Restore, but by having the Restore users can identify very soon this wrong behavior because the next Restore call will give a signal to the users (log, crash, based on a config).

So this API helps users use the Context API in an expected way.

@carlosalberto carlosalberto added this to the v0.4 milestone Feb 26, 2020
@dyladan
Copy link
Member

dyladan commented Feb 26, 2020

In JS we have something like:

// assume context here is ctx1

context.with(ctx2, () => {
  // here context.active() is ctx2
});

// here context.active() is ctx1 again

Do we need explicit set/restore with tokens in this case? This callback style is very idiomatic in JS and having a version where you have something like the following might be weird for our users:

// assume context here is ctx1

const restoreToken = context.set(ctx2);
// here context.active() is ctx2

context.restore(restoreToken);
// here context.active() is ctx1 again

edit:

with the callback style you can also have very easily nested contexts

context.with(context.active().set('a', 1), () => {
  context.active.get('a') // 1
  context.with(context.active().set('a', 2), () => {
    context.active.get('a') // 2
    context.with(context.active().set('a', 3), () => {
      context.active.get('a') // 3
    })
    context.active.get('a') // 2
  })
  context.active.get('a') // 1
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants