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

feature: BaseServer.corsMiddleware should be publicliy modifiable #651

Closed
9ssi7 opened this issue Jul 17, 2022 · 3 comments
Closed

feature: BaseServer.corsMiddleware should be publicliy modifiable #651

9ssi7 opened this issue Jul 17, 2022 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@9ssi7
Copy link

9ssi7 commented Jul 17, 2022

Is your feature request related to a problem? Please describe.

socket.io (here engine.io) uses express's cors library for cors (dependency). I am currently developing a framework and had to write my own cors package in this framework. My framework supports websocket and uses socket.io. So when I try to use my cors package for websocket I do some very unpleasant things.

Describe the solution you'd like

The corsMiddleware function of the BaseServer class must be explicitly changed over the io instance or a middleware must be entered as a cors option.

Describe alternatives you've considered

Alternatively, the following option should be entered:

the current code is right here

    if (this.opts.corsMiddleware) {
       this.corsMiddleware = this.opts.corsMiddleware;
    } else if (this.opts.cors) {
      this.corsMiddleware = require("cors")(this.opts.cors);
    }

and here

    if (this.corsMiddleware) {
      const next = () => {
         this.verify(req, false, callback);
      }
      this.corsMiddleware(this.opts.cors)(req, res, next)
    } else {
      this.verify(req, false, callback);
    }

Although this is not a very good example, I think it is enough to explain my problem. I can create a better PR if you give ideas and support.

Best,
Sami Salih İbrahimbaş

@9ssi7 9ssi7 added the enhancement New feature or request label Jul 17, 2022
@9ssi7
Copy link
Author

9ssi7 commented Jul 17, 2022

Wow, sorry for skipping this, I'm currently using it like this

import { Server } from "socket.io"

const io = new Server();
io.engine.corsMiddleware = (opts, req, res, next) => {
    return myMiddleware(req, res, next)
}

@darrachequesne
Copy link
Member

@ssibrahimbas Hi! Would adding the possibility to provide custom middlewares to the Engine.IO server suit your needs?

darrachequesne added a commit that referenced this issue Feb 6, 2023
This commit implements middlewares at the Engine.IO level, because
Socket.IO middlewares are meant for namespace authorization and are not
executed during a classic HTTP request/response cycle.

A workaround was possible by using the allowRequest option and the
"headers" event, but this feels way cleaner and works with upgrade
requests too.

Syntax:

```js
engine.use((req, res, next) => {
  // do something

  next();
});

// with express-session
import session from "express-session";

engine.use(session({
  secret: "keyboard cat",
  resave: false,
  saveUninitialized: true,
  cookie: { secure: true }
});

// with helmet
import helmet from "helmet";

engine.use(helmet());
```

Related:

- #668
- #651
- socketio/socket.io#4609
- socketio/socket.io#3933
- a lot of other issues asking for compatibility with express-session
@darrachequesne
Copy link
Member

Proper support for middlewares has been added in 24786e7, included in engine.io@6.4.0 and socket.io@4.6.0:

io.engine.use(yourMiddleware);

Reference: https://socket.io/docs/v4/middlewares/#compatibility-with-express-middleware

Please reopen if needed!

@darrachequesne darrachequesne added this to the 6.4.0 milestone Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants