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: file not found errors when the root has a trailing slash #436

Merged
merged 8 commits into from
Feb 7, 2024
Merged

fix: file not found errors when the root has a trailing slash #436

merged 8 commits into from
Feb 7, 2024

Conversation

gurgunday
Copy link
Member

@gurgunday gurgunday commented Feb 7, 2024 •
edited
Loading

It currently does not handle a root with a trailing slash correctly

@gurgunday gurgunday requested a review from a team February 7, 2024 00:14
@gurgunday gurgunday changed the title fix: file not found fix: file not found on wildcard: false Feb 7, 2024
@gurgunday
Copy link
Member Author

gurgunday commented Feb 7, 2024 •
edited
Loading

Have no idea how to test this...

It's not working in production, which runs on Linux, and I don't have a Linux machine right now

@gurgunday gurgunday changed the title fix: file not found on wildcard: false fix: file not found errors with wildcard: false Feb 7, 2024
climba03003
climba03003 reviewed Feb 7, 2024
View reviewed changes
index.js Outdated Show resolved Hide resolved
@gurgunday gurgunday requested a review from climba03003 February 7, 2024 06:16
@gurgunday gurgunday added bug Confirmed bug bugfix Issue or PR that should land as semver patch labels Feb 7, 2024
@gurgunday gurgunday requested a review from a team February 7, 2024 07:06
climba03003
climba03003 reviewed Feb 7, 2024
View reviewed changes
Copy link
Member

@climba03003 climba03003 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I would like to see a test before merging.

If it is not available, you should try to provide comparison on the result between v8 and v10 of glob.
Nested with at least 4 level folder structure would be great.

@gurgunday
Copy link
Member Author

gurgunday commented Feb 7, 2024 •
edited
Loading

I really don’t know where the issue is since linux tests are passing too…

I’ll try to see if I can do something about it

@gurgunday
Copy link
Member Author

Alright, I got the issue, will send a test now

@gurgunday gurgunday requested review from climba03003 and a team February 7, 2024 09:33
@gurgunday gurgunday changed the title fix: file not found errors with wildcard: false fix: file not found errors when the root has a trailing slash Feb 7, 2024
Eomm
Eomm reviewed Feb 7, 2024
View reviewed changes
index.js
const files = await glob(`${posixRootPath}/${globPattern}`, { follow: true, nodir: true, dot: opts.serveDotFiles })
for (let rootPath of roots) {
rootPath = rootPath.split(path.win32.sep).join(path.posix.sep)
!rootPath.endsWith('/') && (rootPath += '/')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
!rootPath.endsWith('/') && (rootPath += '/')
if(!rootPath.endsWith(path.posix.sep)){
rootPath += path.posix.sep
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Istanbul complains

@gurgunday
Copy link
Member Author

gurgunday commented Feb 7, 2024 •
edited
Loading

I really need to make this release, so my apologies for merging without an approval, please do let me know of your feedback and I'll make a second PR addressing them

@gurgunday gurgunday merged commit 6c66465 into fastify:master Feb 7, 2024
19 checks passed
@gurgunday gurgunday deleted the fix branch February 7, 2024 18:56
Uzlopak
Uzlopak reviewed Feb 8, 2024
View reviewed changes
Copy link
Contributor

@Uzlopak Uzlopak left a comment •
edited
Loading

Choose a reason for hiding this comment

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

retrospective indemnity approval

ddadaal referenced this pull request in PKUHPC/OpenSCOW Feb 18, 2024
@renovate
fix(deps): update dependency @fastify/static to v7 (#1115)
7c9853c 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@fastify/static](https://togithub.com/fastify/fastify-static) |
[`6.12.0` ->
`7.0.1`](https://renovatebot.com/diffs/npm/@fastify%2fstatic/6.12.0/7.0.1)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@fastify%2fstatic/7.0.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@fastify%2fstatic/7.0.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@fastify%2fstatic/6.12.0/7.0.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@fastify%2fstatic/6.12.0/7.0.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>fastify/fastify-static (@&#8203;fastify/static)</summary>

###
[`v7.0.1`](https://togithub.com/fastify/fastify-static/releases/tag/v7.0.1)

[Compare
Source](https://togithub.com/fastify/fastify-static/compare/v7.0.0...v7.0.1)

#### What's Changed

- build(deps-dev): bump
[@&#8203;fastify/compress](https://togithub.com/fastify/compress) from
6.5.0 to 7.0.0 by [@&#8203;dependabot](https://togithub.com/dependabot)
in
[https://github.com/fastify/fastify-static/pull/433](https://togithub.com/fastify/fastify-static/pull/433)
- Optimize `dirList` by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/435](https://togithub.com/fastify/fastify-static/pull/435)
- fix: file not found errors when the root has a trailing slash by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/436](https://togithub.com/fastify/fastify-static/pull/436)

**Full Changelog**:
fastify/fastify-static@v7.0.0...v7.0.1

###
[`v7.0.0`](https://togithub.com/fastify/fastify-static/releases/tag/v7.0.0)

[Compare
Source](https://togithub.com/fastify/fastify-static/compare/v6.12.0...v7.0.0)

#### What's Changed

- build(deps-dev): bump tsd from 0.29.0 to 0.30.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/fastify/fastify-static/pull/424](https://togithub.com/fastify/fastify-static/pull/424)
- docs(readme): replace `fastify.io` links with `fastify.dev` by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/425](https://togithub.com/fastify/fastify-static/pull/425)
- docs(readme): replace `fastify.io` links with `fastify.dev` by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/426](https://togithub.com/fastify/fastify-static/pull/426)
- chore(package): fix repository url by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/429](https://togithub.com/fastify/fastify-static/pull/429)
- chore: improve types for `setHeaders` option by
[@&#8203;EvanHahn](https://togithub.com/EvanHahn) in
[https://github.com/fastify/fastify-static/pull/428](https://togithub.com/fastify/fastify-static/pull/428)
- Upgrade glob to v10 by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/406](https://togithub.com/fastify/fastify-static/pull/406)

#### New Contributors

- [@&#8203;EvanHahn](https://togithub.com/EvanHahn) made their first
contribution in
[https://github.com/fastify/fastify-static/pull/428](https://togithub.com/fastify/fastify-static/pull/428)

**Full Changelog**:
fastify/fastify-static@v6.12.0...v7.0.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "on sunday" in timezone Asia/Shanghai,
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log [here](https://developer.mend.io/github/PKUHPC/SCOW).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNTMuMiIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIn0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@climba03003 climba03003 mentioned this pull request Apr 3, 2024
Closed
2 tasks
@YuraGB YuraGB mentioned this pull request Jun 14, 2024
Open
@johanbook johanbook mentioned this pull request Jul 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Eomm Eomm Eomm left review comments

@Uzlopak Uzlopak Uzlopak left review comments

@climba03003 climba03003 Awaiting requested review from climba03003

Assignees
No one assigned
Labels
bug Confirmed bug bugfix Issue or PR that should land as semver patch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gurgunday @Uzlopak @Eomm @climba03003