Skip to content

[Bug]: Short-circuit evaluation bug in TaskManager #235

@lukehinds

Description

@lukehinds

What happened?

Unless I have misunderstood the intent, there maybe an unintended short-circuit evaluation in the conditional assignment of self.context_id

if not self.context_id and self.context_id != event.contextId:
self.context_id = event.contextId

  1. When self.context_id is falsy (None, empty string, etc.), then thenot clause for self.context_id evaluates to True

    • Due to short-circuit evaluation care of and, Python never evaluates the second part: self.context_id != event.contextId
    • The condition becomes ~~ True and = True. ~~
    • The code sets self.context_id = event.contextId (which is correct behavior), ok so far.
  2. When self.context_id has a value:

    • the not in self.context_id evaluates to False
    • Due to short-circuit evaluation, the entire condition becomes False
    • So context ID validation self.context_id != event.contextId never runs

So the current code will never detect mismatched context IDs because the validation check part is unreachable when self.context_id already has a value. Like I say though I might be wrong and forgive wasting time if so.

I think something like this should do the job:

         if not self.task_id:
             self.task_id = task_id_from_event
-        if not self.context_id and self.context_id != event.contextId:
+        if self.context_id and self.context_id != event.contextId:
+            raise ServerError(
+                error=InvalidParamsError(
+                    message=f"Context in event doesn't match TaskManager: {self.context_id} : {event.contextId}"
+                )
+            )
+        if not self.context_id:
             self.context_id = event.contextId

Relevant log output

Code of Conduct

  • I agree to follow this project's Code of Conduct

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions