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

Response built by express is broken if res and req don't extend node's http.IncomingMessage and http.ServerResponse #6039

Open
Tofandel opened this issue Oct 11, 2024 · 2 comments · May be fixed by #6047

Comments

@Tofandel
Copy link

Tofandel commented Oct 11, 2024

To reproduce

import createApp from 'express';

const app = createApp();
console.log(app.response);

console.log(app.response.getHeader('vary'));
import createApp from 'express';
import {IncomingMessage} from 'unenv/runtime/node/http/_request'
import {ServerResponse} from 'unenv/runtime/node/http/_response'

const req = new IncomingMessage();
const res = new ServerResponse(req);

// Taken from express init middleware implementation
const app = createApp();
Object.setPrototypeOf(res, app.response);

res.setHeader('vary', 'test'); // TypeError: Cannot set properties of undefined (setting 'vary')
node:_http_outgoing:747
  const entry = headers[name.toLowerCase()];
                       ^

TypeError: Cannot read properties of undefined (reading 'vary')
    at ServerResponse.getHeader (node:_http_outgoing:747:24)

Because response is initialized from prototype without constructor, headers is empty and fails

Solution:

var request = require('./request');

/**
 * Response prototype.
 * @public
 */

var res = new http.ServerResponse(request)
@Tofandel Tofandel added the bug label Oct 11, 2024
Tofandel added a commit to Tofandel/express that referenced this issue Oct 11, 2024
@Tofandel
Copy link
Author

Tofandel commented Oct 11, 2024

See also #2548 and unjs/unenv#328

@Tofandel
Copy link
Author

The issue is actually coming from the use of sePrototypeOf and not really the reproduction in the end

We should rewrite the initExpress middleware like this

  app._router.stack[1].handle = (req, res, next) => {
    req.res = res;
    res.req = req;
    req.next = next;

    app.request.res = res;

    const setProtoKey = (obj, proto, prop) => {
      Object.defineProperty(obj, prop, {
        get: function () {
          return proto[prop];
        },
        set: function (val) {
          delete obj[prop];
          obj[prop] = val;
        },
        enumerable: proto.propertyIsEnumerable(prop),
        configurable: true,
      });
    };

    const reqKeys = [
      'app',
      'header',          'get',
      'accepts',         'acceptsEncodings',
      'acceptsEncoding', 'acceptsCharsets',
      'acceptsCharset',  'acceptsLanguages',
      'acceptsLanguage', 'range',
      'param',           'is',
      'protocol',        'secure',
      'ip',              'ips',
      'subdomains',      'path',
      'hostname',        'host',
      'fresh',           'stale',
      'xhr'
    ];

    const resKeys = [
      'app',
      'status',
      'links',
      'send',
      'json',
      'jsonp',
      'sendStatus',
      'sendFile',
      'sendfile',
      'download',
      'type',
      'contentType',
      'format',
      'attachment',
      'append',
      'header',
      'set',
      'get',
      'clearCookie',
      'cookie',
      'location',
      'redirect',
      'vary',
      'render',
    ];

    reqKeys.forEach((key) => {
      setProtoKey(req, app.request, key);
    });
    resKeys.forEach((key) => {
      setProtoKey(res, app.response, key);
    });

    res.locals = res.locals || Object.create(null);

    next();
  };

@Tofandel Tofandel changed the title Response built by express is broken Response built by express is broken if res and req don't extend node's http.IncomingMessage and http.ServerResponse Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants