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.

Sign up for GitHub

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

Jump to bottom

WIP: Implement class Frame #477

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

WIP: Implement class Frame #477

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1f26327
Implement class Frame
MarhiievHE Nov 6, 2023
ede8de9
fix decode
MarhiievHE Nov 7, 2023
7df26af
Rewrite class Frame
MarhiievHE Nov 8, 2023
509564f
Removed commented line
MarhiievHE Nov 8, 2023
302bd63
Fix flag masked
MarhiievHE Nov 9, 2023
e67f5bd
Reduced max payload size to the max safe integer in JS
MarhiievHE Nov 10, 2023
c474f02
Fix unmask logic
MarhiievHE Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
154 changes: 114 additions & 40 deletions lib/websocket.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,120 @@ const OPCODE_SHORT = 0x81;
const LEN_16_BIT = 126;
const MAX_16_BIT = 65536;
const LEN_64_BIT = 127;
const MAX_64_BIT = 0x7fffffffffffffffn + 1n;

const calcOffset = (frame, length) => {
if (length < LEN_16_BIT) return [2, 6];
if (length === LEN_16_BIT) return [4, 8];
return [10, 14];
const DEFAULT_OPTIONS = {
server: true,
};

const parseFrame = (frame) => {
const length = frame[1] ^ 0x80;
const [maskOffset, dataOffset] = calcOffset(frame, length);
const mask = frame.subarray(maskOffset, maskOffset + MASK_LENGTH);
const data = frame.subarray(dataOffset);
return { mask, data };
};
const xor = (cond1, cond2) => (cond1 || cond2) && !(cond1 && cond2);

class Frame {
Copy link
Contributor

Choose a reason for hiding this comment

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

Typically in a constructor you just assign some props for instance. Make some configuration. But you don't do some complex logic of how inputs should be mapped correctly to Frame class props. This is not the responsibility of the constructor. So we can create from method that will take this responsibility. See in the example below. Also we should not handle encode or decode logic in constructor. Because when you create frame with string or buffer there is no difference. new Frame('1234') and new Frame(buffer). Code becomes implicit and less readable. Instead you can do Frame.from(data).decode() and Frame.from(data).decode(). This makes clear what happens in this code. See in the example below. So, please use interface described in example below.

class Frame {
  #buffer = null

  constructor(buffer) {
    this.#buffer = buffer
  }


  encode() {
    ...
  }

  decode() {
    ...
  }


  static from(data) {
    if (Buffer.isBuffer(data)) {
      return new Frame(data)
    } 

    if (typeof data === 'string') {
      return new Frame(Buffer.from(data))
    }

    throw new Error('Unsupported')
  }
}

// USAGE

send(text) {
  const frame = Frame.from(text).encode()
  this.socket.write(frame.data);
}

receive(data) {
  console.log('data: ', data[0], data.length);
  if (data[0] !== OPCODE_SHORT) return;
  const frame = Frame.from(data).decode()
  const text = frame.toString();
  this.send(`Echo "${text}"`);
  console.log('Message:', text);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, with the factory, yeah, that's probably better. But creating a text frame of a protocol from a string and just saving the finished frame in a class field is not the same action. Then it is probably better to just accept a ready frame by constructor and create a protocol frame from input data by static method. Thanks.

#frame = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#frame = null;
#data = null;

#mask = null;
#dataOffset = 6;
#length = 0n;
#server = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why server variable is required? The task was about refactoring, not about adding new features.


constructor(data, options) {
if (!Buffer.isBuffer(data) && typeof data !== 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Always false. Because type of Buffer is object.
https://nodejs.org/dist/latest-v20.x/docs/api/buffer.html#static-method-bufferisbufferobj

> typeof Buffer.from('foo')
'object'

throw new Error('Unsupported');
}
const opt = {
...DEFAULT_OPTIONS,
...options,
};
this.#server = opt.server;

if (Buffer.isBuffer(data) && data.length !== 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Already checked isBuffer before

this.#frame = data;
this.#decode();
return;
}

const unmask = (buffer, mask) => {
const data = Buffer.allocUnsafe(buffer.length);
buffer.copy(data);
for (let i = 0; i < data.length; i++) {
data[i] ^= mask[i & 3];
if (typeof data === 'string' && data.length !== 0) {
this.#encode(data);
return;
}
}

#hasMask() {
if ((this.#frame[1] & 0x80) === 0x80) return true;
return false;
}

#decode() {
if (xor(this.#server, this.#hasMask()))
throw new Error('1002 (protocol error)');
const maskLength = this.#hasMask() ? MASK_LENGTH : 0;
this.#length = BigInt(this.#frame[1] & 0x7f);
switch (this.#length) {
case 127n:
this.#dataOffset = 2 + 8 + maskLength;
this.#mask = this.#frame.subarray(2 + 8, 10 + MASK_LENGTH);
this.#length = this.#frame.readBigUInt64BE(2);
break;
case 126n:
this.#dataOffset = 2 + 2 + maskLength;
this.#mask = this.#frame.subarray(2 + 2, 4 + MASK_LENGTH);
this.#length = BigInt(this.#frame.readUInt16BE(2));
break;
default:
this.#dataOffset = 2 + maskLength;
this.#mask = this.#frame.subarray(2, 2 + MASK_LENGTH);
}

if (!this.#server) return;
for (let i = 0n; i < this.#length; ++i) {
this.#frame[BigInt(this.#dataOffset) + i] ^=
this.#mask[i & 0x0000000000000003n];
}
}

#encode(text) {
const data = Buffer.from(text);
this.#frame = Buffer.alloc(2);
this.#frame[0] = 0x81; // FIN = 1, RSV = 0b000, opcode = 0b0001 (text frame)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this.#frame = Buffer.alloc(2);
this.#frame[0] = 0x81; // FIN = 1, RSV = 0b000, opcode = 0b0001 (text frame)
let meta = Buffer.alloc(2);
meta = 0x81; // FIN = 1, RSV = 0b000, opcode = 0b0001 (text frame)

Copy link
Contributor

Choose a reason for hiding this comment

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

Better to name variables in a way that reflects their purpose, preserving code readability.

const length = data.length;
if (length < LEN_16_BIT) {
this.#frame[1] = length;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this.#frame[1] = length;
meta[1] = length;

} else if (length < MAX_16_BIT) {
const len = Buffer.alloc(2);
len.writeUint16BE(length, 0);
this.#frame[1] = LEN_16_BIT;
this.#frame = Buffer.concat([this.#frame, len]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this.#frame[1] = LEN_16_BIT;
this.#frame = Buffer.concat([this.#frame, len]);
meta[1] = LEN_16_BIT;
meta = Buffer.concat([this.#frame, len]);

} else if (length < MAX_64_BIT) {
const len = Buffer.alloc(8);
len.writeBigUInt64BE(BigInt(length), 0);
this.#frame[1] = LEN_64_BIT;
this.#frame = Buffer.concat([this.#frame, len]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this.#frame[1] = LEN_64_BIT;
this.#frame = Buffer.concat([this.#frame, len]);
meta[1] = LEN_64_BIT;
meta = Buffer.concat([this.#frame, len]);

} else {
throw new Error('text value is too long to encode in one frame!');
}
if (!this.#server) throw new Error('Unsupported');
this.#frame = Buffer.concat([this.#frame, data]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this.#frame = Buffer.concat([this.#frame, data]);
this.#frame = Buffer.concat([meta, data]);

}
return data;
};

toString() {
return this.#frame.toString(
'utf8',
this.#dataOffset,
Number(BigInt(this.#dataOffset) + this.#length),
);
}

get data() {
return this.#frame.subarray(this.#dataOffset);
}

get frame() {
return this.#frame;
}

get mask() {
return this.#mask;
}
}

class Connection {
constructor(socket) {
Expand All @@ -56,34 +147,17 @@ class Connection {
}

send(text) {
const data = Buffer.from(text);
let meta = Buffer.alloc(2);
const length = data.length;
meta[0] = OPCODE_SHORT;
if (length < LEN_16_BIT) {
meta[1] = length;
} else if (length < MAX_16_BIT) {
const len = Buffer.from([(length & 0xff00) >> 8, length & 0x00ff]);
meta = Buffer.concat([meta, len]);
meta[1] = LEN_16_BIT;
} else {
const len = Buffer.alloc(8);
len.writeBigInt64BE(BigInt(length), 0);
meta = Buffer.concat([meta, len]);
meta[1] = LEN_64_BIT;
}
const frame = Buffer.concat([meta, data]);
this.socket.write(frame);
const frame = new Frame(text);
this.socket.write(frame.frame);
}

receive(data) {
console.log('data: ', data[0], data.length);
if (data[0] !== OPCODE_SHORT) return;
const frame = parseFrame(data);
const msg = unmask(frame.data, frame.mask);
const text = msg.toString();
this.send(`Echo "${text}"`);
const frame = new Frame(data);
const text = frame.toString();
console.log('Message:', text);
this.send(`Echo "${text}"`);
}

accept(key) {
Expand Down