[Pkg-javascript-devel] Bug#1106428: Additional insights on the failing QUERY method test

[Jérémy Lal]

Le jeu. 5 juin 2025 à 02:28, Santiago Vila <sanvila at 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 ?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/pkg-javascript-devel/attachments/20250605/4c32e8ee/attachment.htm>