Skip to content

fix bad directory relocation fallback code#12782

Merged
cscheid merged 4 commits intomainfrom
bugfix/directory-relocator-bug
May 20, 2025
Merged

fix bad directory relocation fallback code#12782
cscheid merged 4 commits intomainfrom
bugfix/directory-relocator-bug

Conversation

@cscheid
Copy link
Copy Markdown
Member

@cscheid cscheid commented May 20, 2025

Fix typo on fallback code for directory relocation. This is hard to test because it gets executed only in an exception handler in strange circumstances.

@cscheid cscheid requested review from cderv and gordonwoodhull May 20, 2025 14:43
@posit-snyk-bot
Copy link
Copy Markdown
Collaborator

posit-snyk-bot commented May 20, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

Copy link
Copy Markdown
Member

@cderv cderv left a comment

Choose a reason for hiding this comment

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

Looks like a bad bug.

Thanks for finding that. It looks ok to me.

I agree it is hard to test.

@cscheid cscheid mentioned this pull request May 20, 2025
@cscheid
Copy link
Copy Markdown
Member Author

cscheid commented May 20, 2025

Thanks for reviewing. Yeah, we had two real bugs because of the bad documentation in Deno's isSubdir. I've added a unit test to the other one since it's much easier to.

Copy link
Copy Markdown
Member

@gordonwoodhull gordonwoodhull left a comment

Choose a reason for hiding this comment

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

Sure looks like a typo in rare fallback case, argh.

@cscheid cscheid merged commit 1251871 into main May 20, 2025
6 of 8 checks passed
@cscheid cscheid deleted the bugfix/directory-relocator-bug branch May 20, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants