expressjs / express

Fast, unopinionated, minimalist web framework for node.
https://expressjs.com
MIT License
64.99k stars 15.5k forks source link

Can not set response headers when send a json body in a options request #5626

Open yhojann-cl opened 4 months ago

yhojann-cl commented 4 months ago

By example:

router.options('/', (req, res, next) => {
    res
        .status(200)
        .setHeader('Access-Control-Allow-Methods', allowFor('/')) // Without this line works fine.
        .json({
            schema: schemafor('/')
        });
});

Says:

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client at ServerResponse.setHeader (node:_http_outgoing:652:11) at ServerResponse.header (/.../node_modules/express/lib/response.js:794:10) at sendOptionsResponse (/.../node_modules/express/lib/router/index.js:654:9) at Immediate. (/.../node_modules/express/lib/router/index.js:167:7) at Immediate.proxy (/.../node_modules/express/lib/router/index.js:671:8) at process.processImmediate (node:internal/timers:480:21)

The RFC allows the return of http content for options requests. I use it to return crud data schema settings for the frontend and allowed methods for show or hide pages in the dashboard.

wesleytodd commented 4 months ago

Yeah I believe you are correct, but it would be good if you could get the specific section of the spec for this so we can align on the expectations. I did a cursory search and found this which I think is the right place. It has been a long time since I have looked at these and this doc is new to me.

Additionally I would say I am surprised this is broken. I don't remember any specific options handling here and setHeader is not an express api but comes directly from node. It might be worth checking with a reduced test case what the behavior is without express.

danizavtz commented 4 months ago

hello @yhojann-cl , can you detail what functions allowFor and schemafor are? What is the source code of these functions?

I'm guessing that maybe not using the function json works, because when you use this function it finishes the response cicle and send a response. As explicit we can see in this code

So maybe, you can try to use the function write to populate a response as you want, and after finish you can send the response explicit calling function end().

As an example:

//not tested code
router.options('/', (req, res, next) => {
    res
        .status(200)
        .setHeader('Access-Control-Allow-Methods', allowFor('/')) // Without this line works fine.
        .setHeader('Content-Type', 'application/json')
        .write({
            schema: schemafor('/')
        })
       //write more stuff, case needed, then finish response with end()...
       res.end();

});
Codexnever commented 3 months ago
router.options('/', (req, res, next) => {
    // Set the status code and headers
    res
        .status(200) // Set status code to 200 OK
        .setHeader('Access-Control-Allow-Methods', allowFor('/')) // Set allowed methods header
        .setHeader('Content-Type', 'application/json'); // Set content type header to JSON

    // Manually construct the JSON response
    res.write(JSON.stringify({
        schema: schemafor('/') // Populate JSON response with schema data
    }));

    // End the response
    res.end(); // Complete response
});
AbdLaswi commented 3 months ago

@yhojann-cl I modified your code to be the following ( cuz I don't the values of schemafor and allowFor ): router.options('/', (req, res, next) => { res .status(200) .setHeader('Access-Control-Allow-Methods', '/') .json({ schema: "schemafor('/')" }); });

and it worked fine, without adding "res.end() or res.write()"

coltonehrman commented 1 month ago

@yhojann-cl I also created a test for this, and can't reproduce the error https://github.com/coltonehrman/bag-of-tools/blob/main/examples/express/headers-sent-options-request/headers.test.ts

lochansaroy02 commented 2 weeks ago
    res.setHeader('Access-Control-Allow-Methods', allowFor('/'));

    res.status(200).json({
        schema: schemafor('/')
    });
});

Setting Headers Before Sending the Response: The header is set before calling .status(200) and .json(), ensuring that the headers are all properly configured before the response is sent to the client.