fastify / send

Fork of the send module to deal with CVE-2017-20165
MIT License
13 stars 11 forks source link

"files" field removed from package.json. Test files etc. included in published package #42

Closed RubenStoesser closed 1 year ago

RubenStoesser commented 1 year ago

Prerequisites

Last working version

1.0.0

Stopped working in version

2.0.0

Node.js version

all

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

13.2.1

💥 Regression Report

In a PR released with 2.0.0 (https://github.com/fastify/send/pull/6) the files field in package.json was removed.

Due to this all files of the repo are now included in the published npm package (tests, benchmarks, examples etc.).

Steps to Reproduce

I only noticed this because in our CI environment Mend security scans started failing with this error message while resolving the dependency tree:

java.nio.file.InvalidPathException: Malformed input or input contains unmappable characters: node_modules/@fastify/send/test/fixtures/snow ???

This seems to be caused by the snowman emoji in this folder name: https://github.com/fastify/send/tree/master/test/fixtures/snow%20%E2%98%83

The issue wasn't present before because the entire tests folder was excluded from the published npm package.

Expected Behavior

Usually only relevant production files need to be included in the published package. If the change was on purpose another quick-fix for this specific issue would be to remove the non-standard character from the "snow" folder name

climba03003 commented 1 year ago

If the change was on purpose another quick-fix for this specific issue would be to remove the non-standard character from the "snow" folder name

The changes is on purpose and those characters exists also on purpose for testing the characters handling.

Fdawgs commented 1 year ago

See https://github.com/fastify/skeleton/issues/42

marccremer commented 1 year ago

The issue https://github.com/fastify/skeleton/issues/42 points out that we should NOT include test folders, or have I misunderstood something?

image

Eomm commented 1 year ago

we should NOT include test folders, or have I misunderstood something?

You have misunderstood:

image

Anyway, that image shows this pinojs issue: https://github.com/pinojs/pino/pull/1588 That org is not the fastify one, so org has its rules.