Add optional parameter "add_trailing_slash"

[imcarlosguerrero]

As I stated at #359, It would be nice to have the option of whether add or not the trailing slash at the end of the Socket.IO's endpoint.

[miguelgrinberg]

The problem is that when you remove the trailing slash path matching using startswith() doesn't work anymore. Like for example, if you set your path to /foo then requests sent to /foobar, /foo1, /foobar/something or many other unrelated paths will be captured and sent to Socket.IO, even though they are completely different endpoints that the other application may want to handle.

Wouldn't it make more sense to append a trailing slash on any incoming requests that don't have it?

[imcarlosguerrero]

The problem is that when you remove the trailing slash path matching using startswith() doesn't work anymore. Like for example, if you set your path to /foo then requests sent to /foobar, /foo1, /foobar/something or many other unrelated paths will be captured and sent to Socket.IO, even though they are completely different endpoints that the other application may want to handle.

Wouldn't it make more sense to append a trailing slash on any incoming requests that don't have it?

Yes, I agree it'd make more sense to add the trailing slash at the end of the incoming requests, but I just couldn't get that to work with Next.js (#359), I know it's a problem there but I did could get what I wanted to work by modifying this library so I thought It'd be good for everyone else to have this option.

I checked the example you gave and you're right, when the Socket.IO's endpoint is set to /foo, the requests sent to /foobar are captured too.

Got this when connecting to the /api/foo endpoint using Postman, which is the expected behaviour.

[0] INFO: ('127.0.0.1', 6558) - "WebSocket /api/foo?EIO=4&transport=websocket" [accepted]
[0] INFO: [Socket.IO Server] - -A75PPM0hjgNxrMEAAAB: connected

But this happens when trying to reach /api/foobar which is my FastAPI endpoint

[0] INFO: 127.0.0.1:6561 - "GET /api/foobar HTTP/1.1" 400 Bad Request
[0] The client is using an unsupported version of the Socket.IO or Engine.IO protocols (further occurrences of this error will be logged with level INFO)

This is just as you described but this didn't happen to me when my endpoint was /api/socket.io, so I tried setting the Socket.IO's endpoint to /api/foo.foo and to my surprise everything seems to be working now.

[0] INFO: ('127.0.0.1', 7023) - "WebSocket /api/foo.foo?EIO=4&transport=websocket" [accepted]
[0] INFO: [Socket.IO Server] - gkj8UAoEdWL75jiYAAAB: connected

and my /api/foobar FastAPI's endpoint works as well.

[0] INFO: 127.0.0.1:7019 - "GET /api/foobar HTTP/1.1" 200 OK

Requests such as /api/foobar/something is also working well, as it's returning 404 but without the client error because that route doesn't exists.

[0] INFO: 127.0.0.1:7160 - "GET /api/foobar/something HTTP/1.1" 404 Not Found

Does the . (dot) do something in this case?

[miguelgrinberg]

Wouldn't it make more sense to append a trailing slash on any incoming requests that don't have it?

Yes, I agree it'd make more sense to add the trailing slash at the end of the incoming requests, but I just couldn't get that to work > with Next.js (#359),

What I mean is that you can append a / to all incoming requests in the WSGI middleware, before it is compared against the engineio_path.

[imcarlosguerrero]

Wouldn't it make more sense to append a trailing slash on any incoming requests that don't have it?

Yes, I agree it'd make more sense to add the trailing slash at the end of the incoming requests, but I just couldn't get that to work > with Next.js (#359),

What I mean is that you can append a / to all incoming requests in the WSGI middleware, before it is compared against the engineio_path.

Hmm, I hadn't thought in that, I should do it that way, thanks for the recommendation, I think I'll apply it; but wouldn't it be possible to add this as an option given that Socket.IO allows it?

[miguelgrinberg]

but wouldn't it be possible to add this as an option given that Socket.IO allows it?

I guess, but only if it worked correctly. As I said above, matching the URL is harder without that trailing slash as a delimiter.

[imcarlosguerrero]

but wouldn't it be possible to add this as an option given that Socket.IO allows it?

I guess, but only if it worked correctly. As I said above, matching the URL is harder without that trailing slash as a delimiter.

I'll try to get it working correctly and will submit the commit.

[miguelgrinberg]

A solution that requires no configuration like the one I propose would be preferable. There is really not much of a point in having the application explicitly add or remove the trailing slash, since the server can easily support both with the change I suggested. The configuration option should be to disable this behavior, in the odd case it causes a problem.

[imcarlosguerrero]

The configuration option should be to disable this behavior, in the odd case it causes a problem.

You mean the option should be remove_trailing_slash?

[miguelgrinberg]

The default behavior should be that you provide your endpoint with or without trailing slash, and the server accepts the endpoint with or without trailing slash. This should work in pretty much all cases without any need to add configuration. I think just doing this would be an improvement.

If there has to be configuration, rather than doing what the JS server did, I would implement the default as I just described, and then add an option to disable automatic handling, which means that if you provide an endpoint with trailing slash then that is the only accepted URL, and if you provide one without trailing slash again that is the only endpoint that is accepted. But I honestly don't see a need to have this as an option since the default should work just fine for everybody. In the future, if it turns out I'm wrong and there is some use case unknown to me that cannot be implemented due to the automatic slash handling, then we can look into adding an option to disable it.

[miguelgrinberg]

I think the approach that you are taking is more complex that it needs to be. I have committed a simpler solution now, let me know what you think!

[imcarlosguerrero]

It works perfectly! Thank you for listening my petition and fixing my code. Hope this get available soon in the stable release.