socketio / socketio/socket.io

Namespaces are contravariant over SocketData because `Namespace['_fns']` is not `private`

Open
#5,179 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement package:socket.io
Dominant language
TypeScript
Stars
63.2k
Forks
10.3k
Avg merge
11d 20h
Merged PRs (30d)
2

Description

Describe the bug

I have a package that exports a function requiring a typed Namespace to do its thing. This Namespace has a SocketData value of { connectionId: string }. In my actual application I then set up such a namespace to provide said setup function with a such a namespace.

However, because the actual namespace I set up has additional properties in SocketData, for other parts of the application. This results in a TypeScript compiler error when trying to apply said actual namespace to that setup function.

The error stems from the fact that the _fns property on Namespace is contravariant over SocketData.

To Reproduce

Server

import type { Namespace } from "socket.io";
import { Server } from "socket.io";

const io = new Server(3000, {});
const ns = io.of('/my-ns') as Namespace<any, any, any, { connectionId: string; otherStuff: string }>;

const nsForMyPackage: Namespace<any, any, any, { connectionId: string }> = ns; // <- THIS ERRORS

Additional context

In theory, because _fns is not marked private, I can do something like this:

declare const nsA: Namespace<any, any, any, { foo: string; bar: string }>;

nsA.use((socket, next) => {
  console.log(socket.data.bar.toUpperCase());
  next();
});

const nsB: Namespace<any, any, any, { foo: string }> = nsA; // BAD ASSIGNMENT

const nsC: Namespace<any, any, any, { foo: string }>;

nsC.on('connection', (socketC) => {
  nsB._fns[0](socketC, () => {}); // BAD CALL
});

To the TypeScript compiler, BAD CALL looks fine. nsB._fns contains functions that require a Socket with SocketData of { foo: string }. nsC.on('connection', ...) provides just such a Socket. However, that middleware will try to call socket.data.bar.toUpperCase() and produce a TypeError: bar is undefined. That is why the compiler warns me at BAD ASSIGNMENT.

HOWEVER, I should never actually know anything about _fns, even less access it. That would make it impossible to do any such mischief. Then the assignment at BAD ASSIGNMENT would be safe. As even in that case, code like this would be sound:


declare const nsA: Namespace<any, any, any, { foo: string; bar: string }>;
nsA.use((socket, next) => {
  // setup socket data
  socket.data.foo = 'foo';
  socket.data.bar = 'bar';
});

const nsB: Namespace<any, any, any, { foo: string }> = nsA; // This would be a valid assignment if `_fns` was `private`

nsB.on('connection', (socket) => {
  console.log(socket.data.foo);
  console.log(socket.data.bar); // This would be a type error, but still work at runtime
});

I took a quick glance at the Namespace source and it looks like you're keeping it public (with a /** @private */ JSDoc comment) because you need to access it in ParentNamespace.createChild. Two suggestions to deal with that after making _fns private:

  1. Use namespace['_fns'] = this._fns.slice() in createChild
  2. Provide a protected static setFns(ns: Namespace, fns: Array<...>) { ns._fns = fns; } in Namespace. This would have the added benefit of working even with JS private fields (#_fns), and because it's protected will only be accessible to Namespace or it's subclasses.

Let me know if one of those options sounds good to you and you want me to make a PR.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the Namespace source and inspect the public _fns property, then trace how ParentNamespace.createChild accesses it. Make the internal function storage private while preserving child namespace creation, and verify that the unsafe Namespace assignment described in the reproduction is accepted without exposing _fns.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend-api-design
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.