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

Socket IO 4.3 Breaks HTTPS Support #4124

Closed
Striar-Yunis opened this issue Oct 15, 2021 · 1 comment · Fixed by #4127
Closed

Socket IO 4.3 Breaks HTTPS Support #4124

Striar-Yunis opened this issue Oct 15, 2021 · 1 comment · Fixed by #4127
Labels
bug Something isn't working
Milestone

Comments

@Striar-Yunis
Copy link

Striar-Yunis commented Oct 15, 2021

Describe the bug
4.2 -> 4.3 added this if statement check:

if (srv instanceof http.Server || typeof srv === "number") {

That check means that servers made with the https module won't be bound:

To Reproduce
Socket.IO server version: 4.3.0

Server

const { readFileSync } = require("fs");
const { createServer } = require("https");
const { Server } = require("socket.io");

const httpServer = createServer({
  key: readFileSync("/path/to/my/key.pem"),
  cert: readFileSync("/path/to/my/cert.pem")
});

const io = new Server(httpServer, { /* options */ });

io.on("connection", (socket) => {
  // ...
});

httpServer.listen(3000);

^ From https://socket.io/docs/v4/server-initialization/#with-an-https-server

Expected behavior
The https server should be attached. It isn't. You can see this by modifying the following in the /dist/index.js

console.log('look for this 1');
if (srv instanceof http.Server || typeof srv === "number") {
    console.log('look for this 2');
    this.attach(srv);
}
@Striar-Yunis Striar-Yunis added the bug Something isn't working label Oct 15, 2021
@Striar-Yunis Striar-Yunis changed the title Socket IO 4.3 Breaks External HTTPS Support Socket IO 4.3 Breaks HTTPS Support Oct 15, 2021
@HexaField
Copy link
Contributor

This also breaks feathers/socketio. I cannot see why this check is necessary, I have created a PR that reverts it.

darrachequesne pushed a commit that referenced this issue Oct 16, 2021
The check excluded an HTTPS server from being properly attached.

Related: #4124
@darrachequesne darrachequesne added this to the 4.3.1 milestone Oct 18, 2021
dzad pushed a commit to dzad/socket.io that referenced this issue May 29, 2023
The check excluded an HTTPS server from being properly attached.

Related: socketio#4124
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants