Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix for files with % in filename #452

Merged
merged 4 commits into from
May 7, 2024
Merged

Fix for files with % in filename #452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bdd4a1f
Add a test file with a percent in the filename
chetbox May 2, 2024
03c18cc
Fix `href` encoding in directory listing
chetbox May 2, 2024
529a2ac
Fix for filename with `%` in their filename
chetbox May 2, 2024
e7a845d
Reinstate `path.join` workaround for Windows
chetbox May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ async function fastifyStatic (fastify, opts) {
}
}

const stream = send(request.raw, pathnameForSend, options)
// `send(..., path, ...)` will URI-decode path so we pass an encoded path here
const stream = send(request.raw, encodeURI(pathnameForSend), options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your comment: we uri decode path in send? How about apply a change in @fastify/send , like pass decodeUri option in options, by default true for backwards compat and we set it here to false and then we dont need to encodeURI?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the docs of @fastify/send is seems the path passed is intended to be URL-encoded:

path is a urlencoded path to send (urlencoded, not the actual file-system path)

https://github.com/fastify/send?tab=readme-ov-file#api

While I agree that this seems like a little extra overhead it looks like the @fastify/send API was not being used correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe send could be refactored to do what Uzlopak suggests

let resolvedFilename
stream.on('file', function (file) {
resolvedFilename = file
Expand Down
2 changes: 1 addition & 1 deletion lib/dirList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ const dirList = {
route = path.normalize(path.join(route, '..'))
Copy link
Contributor

@Uzlopak Uzlopak May 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we change it to?

Maybe on top of the file we do:
const pathPosixNormalize = path.posix.normalize
and then do:

Suggested change
route = path.normalize(path.join(route, '..'))
route = pathPosixNormalize(path.join(route, '..'))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might work but it's not part of the fix for this PR. See #452 (comment)

}
return {
href: path.join(prefix, route, entry.name).replace(/\\/gu, '/'),
href: encodeURI(path.join(prefix, route, entry.name).replace(/\\/gu, '/')),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, can we avoid this regex?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regex was already there. I tried switching to path.posix.join but that failed the tests on Windows.

name: entry.name,
stats: entry.stats,
extendedInfo: entry.extendedInfo
Expand Down
14 changes: 8 additions & 6 deletions test/dir-list.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ t.test('dir list, custom options', t => {
}

const route = '/public/'
const content = { dirs: ['deep', 'shallow'], files: ['.example', 'a .md', 'foo.html', 'foobar.html', 'index.css', 'index.html'] }
const content = { dirs: ['deep', 'shallow'], files: ['.example', '100%.txt', 'a .md', 'foo.html', 'foobar.html', 'index.css', 'index.html'] }

helper.arrange(t, options, (url) => {
t.test(route, t => {
Expand Down Expand Up @@ -205,7 +205,8 @@ t.test('dir list html format', t => {
</ul>
<ul>
<li><a href="/public/.example" target="_blank">.example</a></li>
<li><a href="/public/a .md" target="_blank">a .md</a></li>
<li><a href="/public/100%25.txt" target="_blank">100%.txt</a></li>
<li><a href="/public/a%20.md" target="_blank">a .md</a></li>
<li><a href="/public/foo.html" target="_blank">foo.html</a></li>
<li><a href="/public/foobar.html" target="_blank">foobar.html</a></li>
<li><a href="/public/index.css" target="_blank">index.css</a></li>
Expand Down Expand Up @@ -236,7 +237,8 @@ t.test('dir list html format', t => {
</ul>
<ul>
<li><a href="/public/.example" target="_blank">.example</a></li>
<li><a href="/public/a .md" target="_blank">a .md</a></li>
<li><a href="/public/100%25.txt" target="_blank">100%.txt</a></li>
<li><a href="/public/a%20.md" target="_blank">a .md</a></li>
<li><a href="/public/foo.html" target="_blank">foo.html</a></li>
<li><a href="/public/foobar.html" target="_blank">foobar.html</a></li>
<li><a href="/public/index.css" target="_blank">index.css</a></li>
Expand Down Expand Up @@ -492,7 +494,7 @@ t.test('json format with url parameter format', t => {
}
}
const route = '/public/'
const jsonContent = { dirs: ['deep', 'shallow'], files: ['.example', 'a .md', 'foo.html', 'foobar.html', 'index.css', 'index.html'] }
const jsonContent = { dirs: ['deep', 'shallow'], files: ['.example', '100%.txt', 'a .md', 'foo.html', 'foobar.html', 'index.css', 'index.html'] }

helper.arrange(t, options, (url) => {
simple.concat({
Expand Down Expand Up @@ -539,7 +541,7 @@ t.test('json format with url parameter format and without render option', t => {
}
}
const route = '/public/'
const jsonContent = { dirs: ['deep', 'shallow'], files: ['.example', 'a .md', 'foo.html', 'foobar.html', 'index.css', 'index.html'] }
const jsonContent = { dirs: ['deep', 'shallow'], files: ['.example', '100%.txt', 'a .md', 'foo.html', 'foobar.html', 'index.css', 'index.html'] }

helper.arrange(t, options, (url) => {
simple.concat({
Expand Down Expand Up @@ -588,7 +590,7 @@ t.test('html format with url parameter format', t => {
}
}
const route = '/public/'
const jsonContent = { dirs: ['deep', 'shallow'], files: ['.example', 'a .md', 'foo.html', 'foobar.html', 'index.css', 'index.html'] }
const jsonContent = { dirs: ['deep', 'shallow'], files: ['.example', '100%.txt', 'a .md', 'foo.html', 'foobar.html', 'index.css', 'index.html'] }

helper.arrange(t, options, (url) => {
simple.concat({
Expand Down
27 changes: 27 additions & 0 deletions test/static.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3963,6 +3963,33 @@ t.test(
}
)

t.test(
'serves files with % in the filename',
async (t) => {
t.plan(2)

const txtContent = fs.readFileSync(path.join(__dirname, 'static', '100%.txt'), 'utf-8')

const pluginOptions = {
root: url.pathToFileURL(path.join(__dirname, '/static')),
wildcard: false
}

const fastify = Fastify()

fastify.register(fastifyStatic, pluginOptions)
const response = await fastify.inject({
method: 'GET',
url: '100%25.txt',
headers: {
'accept-encoding': '*, *'
}
})
t.equal(response.statusCode, 200)
t.same(response.body, txtContent)
}
)

t.test('content-length in head route should not return zero when using wildcard', t => {
t.plan(6)

Expand Down
1 change: 1 addition & 0 deletions test/static/100%.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
100%