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

[BUG] Websocket upgrades shouldn't be possible with POST requests. #25

Closed
ItsWendell opened this issue Aug 8, 2021 · 2 comments
Closed

Comments

@ItsWendell
Copy link

Upgrading a request to WebSockets should only be possible with GET requests. I verified this in production and took me a while to figure out that this was the thing that was going wrong. The following code sample shouldn't work.

        const res = await durableObject.fetch(
          `http://fake-host/events/messageSend?clientId=${ctx.message.id}`,
          {
            method: 'POST',
            headers: {
              Upgrade: 'websocket'
            },
          },
        )
        console.log('has WebSocket', typeof res?.webSocket !== "undefined");

Took me a while to figure out why my code wasn't working in production but was working with Miniflare. this was the reason. Will be great to have this fixed in miniflare.

I might be able to work on a PR this week. What are the preferences in terms of testing?

E.g. would a test called test("fetch: only performs web socket upgrade on GET requests") be ok?

In case I don't get round to it, I think it might be as simple as changing the following file: src/modules/standards.ts

before:

    // Handle web socket upgrades
    if (request.headers.get("upgrade") === "websocket") {

after:

    // Handle web socket upgrades, this should only for GET requests
    if (request.method === "GET" && request.headers.get("upgrade") === "websocket") {
@lukeed
Copy link
Contributor

lukeed commented Aug 13, 2021

Yeah, as per the spec only GET requests can be upgraded

@mrbbot mrbbot closed this as completed in 1c0ed30 Aug 22, 2021
@mrbbot
Copy link
Contributor

mrbbot commented Aug 27, 2021

Just released version 1.4.0 with this fix. Please let me know if you have any other issues or feedback. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants