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.

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

fix(server): normalize external path #4239

Merged
merged 4 commits into from Oct 6, 2023
Merged

Conversation

etnoy
Copy link
Contributor

@etnoy etnoy commented Sep 26, 2023

This normalizes the user's external path. This should fix many bugs we've found with external paths and no assets being found.

@vercel
Copy link

vercel bot commented Sep 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
immich ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 6, 2023 8:45pm

@jrasm91
Copy link
Contributor

jrasm91 commented Sep 26, 2023

Can you give an example of a situation that this would fix?

@etnoy
Copy link
Contributor Author

etnoy commented Sep 27, 2023

Can you give an example of a situation that this would fix?

After some consideration, this might fix fewer bugs than anticipated. However, whenever you have /../ in the external path, this makes things easier. Note that this will always be the case when we do e2e.

@jrasm91
Copy link
Contributor

jrasm91 commented Sep 27, 2023

In that situation can we normalize the path before it is saved instead?

@etnoy
Copy link
Contributor Author

etnoy commented Oct 4, 2023

In that situation can we normalize the path before it is saved instead?

done!

@alextran1502 alextran1502 enabled auto-merge (squash) October 6, 2023 20:47
@alextran1502 alextran1502 merged commit 4dffae3 into main Oct 6, 2023
21 checks passed
@alextran1502 alextran1502 deleted the fix/normalize-external-path branch October 6, 2023 20:47
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.

None yet

3 participants