Le jeu. 5 juin 2025 à 02:28, Santiago Vila <sanv...@debian.org> a écrit :
> Thanks a lot for the investigation!
>
> So, it seems the logical thing to do here is to disable those failing
> tests,
> as they were not really intended to be run with nodejs 20.
>
> Is there any team preference about how to do that?
>
> I can think of at least three methods.
>
> Method 1: Skip also when running nodejs 20
>
> --- a/test/body-parser.js
> +++ b/test/body-parser.js
> @@ -82,7 +82,7 @@ describe('bodyParser()', function () {
> // update this implementation to run on supported versions of 21
> once they exist
> // upstream tracking https://github.com/nodejs/node/issues/51562
> // express tracking issue:
> https://github.com/expressjs/express/issues/5615
> - return getMajorVersion(versionString) === '21'
> + return getMajorVersion(versionString) === '20' ||
> getMajorVersion(versionString) === '21'
> }
>
> methods.slice().sort().forEach(function (method) {
>
> Method 2: Make the function to return true unconditionally, since this is
> targeted
> for trixie which will have nodejs 20.
>
> --- a/test/body-parser.js
> +++ b/test/body-parser.js
> @@ -82,7 +82,7 @@ describe('bodyParser()', function () {
> // update this implementation to run on supported versions of 21
> once they exist
> // upstream tracking https://github.com/nodejs/node/issues/51562
> // express tracking issue:
> https://github.com/expressjs/express/issues/5615
> - return getMajorVersion(versionString) === '21'
> + return true
> }
>
> methods.slice().sort().forEach(function (method) {
>
> Method 3: Assume that the function would return true in the place where
> the return value is used:
>
> --- a/test/body-parser.js
> +++ b/test/body-parser.js
> @@ -89,7 +89,7 @@ describe('bodyParser()', function () {
> if (method === 'connect') return
>
> it('should support ' + method.toUpperCase() + ' requests',
> function (done) {
> - if (method === 'query' && shouldSkipQuery(process.versions.node))
> {
> + if (method === 'query') {
> this.skip()
> }
> request(this.server)[method]('/')
>
>
> I could make a team upload if some authorized voice tells me which
> solution is best/preferred.
> (My personal preference would be method 2).
>
> Thanks.
>
The best fix would be in the embedded copy of llhttp in nodejs 20.19.2 -
remove the HTTP QUERY support that was
added in
https://github.com/nodejs/node/commit/eb25047b1b08180ed68dcefe2299682fbc7966ab
This would avoid potentially hidden bugs in the nodejs-dependents packages.
Shall we reassign this bug to nodejs ?
