honojs/node-server

`c.req.url` is always http

hilja opened this issue · 7 comments

This might be how it should work, but let me confirm.

c.req.url is always http, even on https requests:

app.get('/foo', c => {
  // Always http, in prod too
  console.log(c.req.url)
  console.log(c.req.raw.url)
  return c.text('OK')
})

I'm on fly.io, basically a proxy that handles my ssl (plus other things). But in Hono I always get the url as http, and my JWT verification fails. It's easy to fix but I somehow expected the proto to be the same as the original request.

Here's my serve:

serve({
  fetch: app.fetch,
  port: Number(PORT),
})

More details on my use-case

This is specifically a request from QStash. It's a queue thing, works something like this:

curl -XPOST \
    -H 'Authorization: Bearer <QSTASH_TOKEN>' \
    -H "Content-type: application/json" \
    -d '{ "hello": "world" }' \
    'https://qstash.upstash.io/v2/publish/https://example.com/foo'

Notice the url after the QStash endpoint, that's the callback URL that Hono is listening to, and which I've set it to https. Here's the headers QStash sends:

{
  'accept-encoding': 'gzip',
  authorization: 'Basic xxx',
  'content-length': '203',
  'content-type': 'application/json',
  host: 'example.com/foo',
  'upstash-caller-ip': 'xxx.xx.xx.xx',
  'upstash-message-id': 'msg_xxx',
  'upstash-retried': '0',
  'upstash-signature': 'xxx',
  'user-agent': 'Upstash-QStash',
  'x-forwarded-for': 'x.xxx.xxx.xxx',
  'x-forwarded-host': 'example.com/foo',
  'x-forwarded-proto': 'https'
}

I'm guessing it would be https if I used the https server like shown in the docs:

import { createServer } from 'node:https'
serve({
  fetch: app.fetch,
  createServer: createServer,
  serverOptions: { /* keys */},
})

But I don't need to because of fly.

Correct me if I'm mistaken but it feels like Hono should use x-forwarded-proto to set the correct proto?

Any luck on that one ? I have the same issue

seep commented

We're seeing this as well with https.createServer.

seep commented

This line seems to be the issue: https://github.com/honojs/node-server/blob/main/src/request.ts#L120 The request shim has the HTTP protocol hard coded.

@hilja Thanks for raising the issue.

Hi @usualoma , as @seep mentioned, I think we can fix the line. Can you take a look?

seep commented

If it helps, I was able to work around this by pulling the IncomingMessage object out of the request and checking incoming.socket.encrypted. I think you can do the same here for the fix. When using https.createServer the socket will be an instance of tls.TLSSocket instead of net.Socket. For reference: https://nodejs.org/api/tls.html#tlssocketencrypted

Hi @hilja Thanks for creating the issue.

Hi @seep Thanks for the reference info.

Indeed, when using 'node:http2' or 'node:https', it certainly looks better to be https:. I will create a PR for the change.

As for the x-forwarded-proto, I think it should be handled on the application side. Or if there are a lot of requests, we could provide a hook point that can be rewritten. Anyway, I think it is better not to handle x-forwarded-proto implicitly.

Created #155