search

sinclairzx81 / smoke

Run Web Servers in Web Browsers over WebRTC
Other
535 stars 40 forks source link

Firefox: reading body from POST returns "Object { body: "[object ReadableStream]" }" instead of actual body data #28

Closed minwidth0px closed 1 month ago

minwidth0px commented 1 month ago

Here's a minimal example (tested on @sinclair/smoke@0.8.5, as per npm list)


import { Network } from '@sinclair/smoke'
import { RemoteNetworkHub } from './hub.mjs'

const server = new Network({ hub: new RemoteNetworkHub('ws://localhost:5001/hub') })

const client = new Network({ hub: new RemoteNetworkHub('ws://localhost:5001/hub') })

server.Http.listen({ port: 5000 }, async request => {
    if (request.method === 'POST') {
        const body = await request.text()
        console.log({ body })
    }
    return new Response('Test successful')
})

const remoteAddr = await server.Hub.address()

const text = await client.Http.fetch(`http://${remoteAddr}:5000`, {
    method: "POST",
    headers: {
      "Content-Type": "application/json",
    },
    body: JSON.stringify({ hello: "world" }),
  }).then(r => r.text())

console.log({ text })

This is a simple variation of the sample project in #21.

results:

Firefox:


Object { body: "[object ReadableStream]" }
Object { text: "Test successful" }

Chrome:


{body: '{"hello":"world"}'}
{text: 'Test successful'}

This isn't just a formatting thing and actually shows up in errors (eg. 'unexpected value "[o"' )

I also tested the server and client on Chrome and Firefox separately, and found that the issue is on the server side of Firefox. Using chrome as the server and Firefox as the client returns {body: '{"hello":"world"}'}. I tested this behavior on separate browsers using two separate files like so:

a.mts:


import { Network } from '@sinclair/smoke'
import { RemoteNetworkHub } from './hub.mjs'

const client = new Network({ hub: new RemoteNetworkHub('ws://localhost:5001/hub') })

//paste this value into the b.html's textbox
console.log( await client.Hub.address())

client.Http.listen({ port: 5000 }, async request => {
    if (request.method === 'POST') {
        const body = await request.text()
        console.log({ body })
    }
    return new Response('Test successful')
})

a.html:


<!DOCTYPE html>
<html>
  <head>
    <script type="module" src="a.js"></script>
  </head>
  <body>View console logs for output</body>
</html>

b.mts:


import { Network } from '@sinclair/smoke'
import { RemoteNetworkHub } from './hub.mjs'

const client = new Network({ hub: new RemoteNetworkHub('ws://localhost:5001/hub') })

const btn = document.getElementById('btn')
const input = document.getElementById('input')

if (!btn) throw new Error('Button not found')
btn.addEventListener('click', async () => {
  const remoteAddr = (input as HTMLInputElement)?.value
  console.log({ remoteAddr })
  const text = await client.Http.fetch(`http://${remoteAddr}:5000`, {
    method: "POST",
    headers: {
      "Content-Type": "application/json",
    },
    body: JSON.stringify({ hello: "world" }),
  }).then(r => r.text())
  console.log({ text })
})

b.html:


<!DOCTYPE html>
<html>
  <head>
    <script type="module" src="b.js"></script>
  </head>
  <input type="text" id="input" />
  <button id="btn">Click me</button>
  <body>View console logs for output</body>
</html>

I'm going to look into the cause of this issue, but I thought I'd report it now and respond to this post when I find the cause. (Not sure if I should keep all Firefox issues in #24, but I didn't want to clutter it with random bugs. Let me know what is better.)

minwidth0px commented 1 month ago

One thing I've noticed is that ---REQUEST_END--- is not being received in Firefox. I determined this by logging the following in HttpListener:


  #createReadableFromRequestInit(listenerRequestInit, stream) {
    if (["HEAD", "GET"].includes(listenerRequestInit.method))
      return null;
    return new ReadableStream({
      pull: async (controller) => {
        const next = await stream.read();
        console.log("next", next);
        console.log(new TextDecoder().decode(next));
        if (next === null || equals(next, REQUEST_END)) {
          return controller.close();
        } else {
          console.log("enquing...")
          controller.enqueue(next);
        }
      }
    });
  }

Firefox:


90796fb9-32ed-4283-86a5-ced092338fe4
next Uint8Array(17) [ 123, 34, 104, 101, 108, 108, 111, 34, 58, 34, … ]
{"hello":"world"}
enquing...
Object { body: "[object ReadableStream]" }

Chrome:


456908b4-1f55-4f7b-92ba-ae65969530dc
next Uint8Array(17) [123, 34, 104, 101, 108, 108, 111, 34, 58, 34, 119, 111, 114, 108, 100, 34, 125, buffer: ArrayBuffer(17), byteLength: 17, byteOffset: 0, length: 17, Symbol(Symbol.toStringTag): 'Uint8Array']
{"hello":"world"}
 enquing...
 next Uint8Array(17) [45, 45, 45, 82, 69, 81, 85, 69, 83, 84, 95, 69, 78, 68, 45, 45, 45, buffer: ArrayBuffer(17), byteLength: 17, byteOffset: 0, length: 17, Symbol(Symbol.toStringTag): 'Uint8Array']
 ---REQUEST_END---
 {body: '{"hello":"world"}'}

Note how in Firefox ---REQUEST_END--- is never printed. This means controller.close is never called, although I don't know what the implications of that are, and maybe there is some other affect ---REQUEST_END--- is meant to have and that is causing the issue, or maybe this is a completely unrelated bug. But it's another difference between Firefox and Chrome in any event.

sinclairzx81 commented 1 month ago

@minwidth0px Hi,

Object { body: "[object ReadableStream]" }

So this issue appears to related to how Firefox implements the Request API. The offending code is below.

https://github.com/sinclairzx81/smoke/blob/master/src/http/listener.mts#L136-L142

// Read the body as a ReadableStream
const body = this.#createReadableFromRequestInit(listenerRequestInit, stream)

// Pass the ReadableStream to Request 
const request = new Request(url, {
  method: listenerRequestInit.method,
  headers: headers,
  body: body, // Body is a ReadableStream
  duplex: 'half',
} as RequestInit)

// Expect .text() to asynchronously read from the ReadableStream.

const text = await request.text() // this function should asynchronously read from the ReadableStream
                                  // assigned to the `body`, not return `[object ReadableStream]`

So, there isn't much that can be done here (afaik) other than polyfill / patching the Request prototype in Firefox (something I am very reluctant to do) or outright reimplementing Request from scratch (also extremely reluctant to do)

Maybe there is a specification write up on Request that would give an indication on what .text() "should do" if the assigned body is a ReadableStream? If there is a specification on what the correct implementation is, I'd be willing to align Smoke to that, but yeah, the Firefox implementation seems at odds with expectations.

sinclairzx81 commented 1 month ago

Relevant Firefox Issue (With the issue dating back several years)

https://bugzilla.mozilla.org/show_bug.cgi?id=1387483

minwidth0px commented 1 month ago

@sinclairzx81 so the ---REQUEST_END--- thing is unrelated?

sinclairzx81 commented 1 month ago

@sinclairzx81 so the ---REQUEST_END--- thing is unrelated?

Not sure, just looking into it

minwidth0px commented 1 month ago

So, there isn't much that can be done here (afaik) other than polyfill / patching the Request prototype in Firefox (something I am very reluctant to do) or outright reimplementing Request from scratch (also extremely reluctant to do)

Agreed that it may not be worth it if Firefox is not following the spec here.

minwidth0px commented 1 month ago

Since this is a Firefox bug I'm going to close this issue. This does mean that Firefox doesn't work for post requests now (I think), but it seems like this bug may be fixed soon actually, according to the linked bug report. I may hack some bad solution in the meantime on my local project (obviously will not make a PR for that). We can open a new bug for the ---REQUEST_END--- thing if that is a separate issue.

sinclairzx81 commented 1 month ago

@minwidth0px I've implemented a fix that should resolve both REQUEST_END and Firefox .text() issues.

The current solution appears to be to assign Firefox Requests a Blob instead of ReadableStream. Have setup a code path for that that runs in Firefox only.

Fix is on 0.8.7

minwidth0px commented 1 month ago

confirmed that it is working on 0.8.7 on Firefox. You can't stream responses from a Firefox server, although I assume that is just a limitation of using blob instead of stream (as implied by the name). Hopefully that bug is fixed soon and we can remove this workaround.