-
-
Notifications
You must be signed in to change notification settings - Fork 16.8k
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
Support HTTP QUERY
method
#5615
Comments
Edit: this will light up once it fully lights up in Node, see the Node repro at the bottom Here's my test harness rn, I don't think we support this automatically ✨✨✨ const express = require('express');
const app = express();
const router = express.Router();
app.use(express.json());
// Test using app.query (if this is valid)
try {
app.query('/test-app-query', (req, res) => {
console.log({ body: req.body });
res.status(200).json({ data: req.body });
});
console.log("app.query() tested and set up.");
} catch (error) {
console.error("app.query() failed:", error);
}
// Test using router.query (if this is valid)
try {
router.query('/test-router-query', (req, res) => {
console.log({ body: req.body });
res.status(200).json({ data: req.body });
});
app.use(router);
console.log("router.query() tested and set up.");
} catch (error) {
console.error("router.query() failed:", error);
}
// Fallback general middleware to check if QUERY method is getting through
app.use('/test', (req, res, next) => {
if (req.method === 'QUERY') {
console.log('Received QUERY method via general handler');
res.status(200).json({ received: req.body });
} else {
next();
}
});
// General error handler
app.use((err, req, res, next) => {
console.error('Error Handler:', { err });
res.status(500).send('Server Error');
});
app.listen(3000, () => {
console.log("Server listening on port 3000");
}); These are the curl requests Im using: curl -i -X QUERY -H "Content-Type: application/json" -d '{"foo": "bar"}' http://localhost:3000/test-app-query
curl -i -X QUERY -H "Content-Type: application/json" -d '{"foo": "bar"}' http://localhost:3000/test-router-query
curl -i -X QUERY -H "Content-Type: application/json" -d '{"foo": "bar"}' http://localhost:3000/test They each give me a response of:
CURL shouldn't matter here, but just in case:
Maybe node really doesn't support this yet? Check this out:const http = require('http');
// Do we know about query?
console.log(http.METHODS.includes('QUERY')) // true
// Create an HTTP server
const server = http.createServer((req, res) => {
// Log the method to the console for verification
console.log('Received method:', req.method);
// Check if the method is QUERY
if (req.method === 'QUERY') {
res.writeHead(200, { 'Content-Type': 'text/plain' });
res.end('Received a QUERY method request');
} else {
// Respond with Method Not Allowed if the method is not QUERY
res.writeHead(405, { 'Content-Type': 'text/plain' });
res.end('Method Not Allowed');
}
});
// Listen on port 3000
server.listen(3000, () => {
console.log('Server listening on port 3000');
}); node -v
# v21.7.3
curl -i -X QUERY -H "Content-Type: application/json" -d '{"foo": "bar"}' http://localhost:3000
HTTP/1.1 405 Method Not Allowed
Content-Type: text/plain
Date: Sat, 20 Apr 2024 23:22:16 GMT
Connection: keep-alive
Keep-Alive: timeout=5
Transfer-Encoding: chunked
Method Not Allowed |
I may be wrong but I do think this is supported by express? I do remember that it worked with a manually build version of Node.js but not with the latest release @wesleytodd |
Yeah that is quite possible. I thought the version with this fixed was supposed to go out with the security patch but maybe I misunderstood and it was set to land later? Based on this yes I am thinking that is the case: nodejs/node#51562 (comment) |
related: expressjs#5615
related: expressjs#5615
related: expressjs#5615
related: expressjs#5615
This serves two purposes: - Enforcing constraints on its actual usage in `walkRouting()`, - Should ease adoption of the new `QUERY` method in the future, see: - expressjs/express#5615 - nodejs/node#51562
This serves two purposes: - Enforcing constraints on its actual usage in `walkRouting()`, - Should ease adoption of the new `QUERY` method in the future, see: - expressjs/express#5615 - nodejs/node#51562
This serves two purposes: - Enforcing constraints on its actual usage in `walkRouting()`, - Should ease adoption of the new `QUERY` method in the future, see: - expressjs/express#5615 - nodejs/node#51562
Looks like this is now passing in Node v22.2.0, which includes this commit nodejs/node@65c8380 I don't think support will come to node 21, as the change was considered semver major by Node. |
Honestly we might already, but opening this issue to track landing support and writing tests.
Node 21 added support for a new method!
QUERY
which is awesome, looks to have landed here nodejs/node#51719 in21.7.2
(changelog)@wesleytodd any idea what needs to be done to router to support QUERY? If this Issue belongs at
router
plz do move it!We may actually already support this ¯_(ツ)_/¯ and it's just that a test is failing.
Im unable to get it to work in Node
21.7.3
currently, and left a comment in the node tracking issue for support nodejs/node#51562TODO:
The text was updated successfully, but these errors were encountered: