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

Index: Ignore nested storage folder in the originals path #1642

Closed
oscadev opened this issue Oct 20, 2021 · 12 comments
Closed

Index: Ignore nested storage folder in the originals path #1642

oscadev opened this issue Oct 20, 2021 · 12 comments
Assignees
Labels
easy Easy issue for beginners enhancement Refactoring, improvement or maintenance task released Available in the stable release tested Changes have been tested successfully

Comments

@oscadev
Copy link

oscadev commented Oct 20, 2021

RIght now, if a user places their "Storage" folder (the folder where albums, sidecar, config, cache etc are) inside their "pictures" folder *the folder that it scans to index from), it creates a loop where it creates thumbnails in Storage, indexes the thumbnails, then makes thumbnails of the thumbnail images, index those etc in a recursive loop.

A potential solution is to write a .dont-scan-me file in the storage folder which tells Photoprism to skip it when indexing.

@lastzero
Copy link
Member

That's in fact a supported config IF you use a hidden folder name like .photoprism or add the name to a .ppignore file. Should be able to detect this specific edge case though and then also ignore it. Can't change your container config from inside the container. Only option would be to error on startup otherwise.

@svengreb
Copy link

I'd vote for the fail-fast solution here to error on startup. Even though it can be avoid by setting up ignore pattern, this is a case where I see responsibility on the side of the application. Without looking into the code I guess the configurations should be available to the application on startup and therefore allows to check whether the storage path is a sub-directory of the import/originals directory. If so deny to start with the reason of invalid configuration(s).

@graciousgrey graciousgrey added the enhancement Refactoring, improvement or maintenance task label Oct 21, 2021
@lastzero
Copy link
Member

It's really not THAT easy because Docker hides the actual host paths. The storage folder could be ANYWHERE in originals. Can't check all folders in originals on startup.

@graciousgrey
Copy link
Member

An onboarding wizard for setting config values like paths and the admin password using the web ui is planned as well. It will include at least a hint to not put the storage insides of originals.

@srett
Copy link
Contributor

srett commented Oct 21, 2021

It's really not THAT easy because Docker hides the actual host paths. The storage folder could be ANYWHERE in originals. Can't check all folders in originals on startup.

I think if it's supposed to be a supported configuration, creating a special marker-file a la .dont-scan-me in the storage folder is the most pragmatic solution. Create it on startup, if this fails error out immediately. Then when scanning, don't descend into any directory that contains said flag file.
The only way to break things then would be if the user deliberately deletes the flag after startup, but you can only do so much to protect a user from themselves. ;-)

@lastzero lastzero self-assigned this Oct 21, 2021
@svengreb
Copy link

It's really not THAT easy because Docker hides the actual host paths. The storage folder could be ANYWHERE in originals. Can't check all folders in originals on startup.

Right, I haven't take the container <-> host mapping into account, but @srett already posted the next idea I had 😄
Always creating a .ppignore file in the storage directory on startup should cover 99,9% of all use cases. When someone deletes the file during runtime this is definitely an action that was forced by the user. I guess the file should also include a comment that informs about the purpose of the file and the consequences when it gets deleted.

@lastzero
Copy link
Member

Sounds perfect 👍

@lastzero lastzero changed the title Prevent infinite indexing loop when "storage" is inside "pictures" directory Config: Prevent nesting of "storage" folder inside "originals" if not hidden Jun 20, 2022
@lastzero lastzero changed the title Config: Prevent nesting of "storage" folder inside "originals" if not hidden Config: Prevent nesting of "storage" folder inside "originals" unless hidden Jun 20, 2022
@lastzero lastzero added the help wanted Well suited for external contributors! label Jun 20, 2022
@lastzero
Copy link
Member

lastzero commented Jun 20, 2022

Updated the Known Issues page in our docs to include this issue:

Published: https://docs.photoprism.app/known-issues/#nested-storage-folder

lastzero added a commit that referenced this issue Jan 20, 2024
This creates a .ppstorage file in the storage folder so that it can be
automatically ignored if found in the originals path while indexing.

Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero lastzero changed the title Config: Prevent nesting of "storage" folder inside "originals" unless hidden Index: Ignore nested storage folder in the originals path Jan 20, 2024
@lastzero lastzero added please-test Ready for acceptance test and removed help wanted Well suited for external contributors! labels Jan 20, 2024
@lastzero
Copy link
Member

The commit referenced above creates a .ppstorage file in the storage folder when PhotoPrism starts, so that nested storage folders can be automatically ignored if found in the originals path while indexing.

An updated photoprism/photoprism:test image will soon be available on Docker Hub. Any help with testing these changes is much appreciated as I can only spend very limited time on this issue. Thank you very much!

@lastzero
Copy link
Member

In particular, I have not yet tested what happens if the storage folder is identical to the originals folder. Ideally, this should generate a clearly visible error message when PhotoPrism starts so that users know about it and can add/change the related config values.

@lastzero lastzero added help wanted Well suited for external contributors! easy Easy issue for beginners labels Jan 20, 2024
@lastzero lastzero removed their assignment Jan 20, 2024
@oscadev
Copy link
Author

oscadev commented Jan 20, 2024

Great!

lastzero added a commit that referenced this issue Jan 21, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

With these changes, PhotoPrism will fail with an error if:

  • the originals and storage folders are configured to be identical
  • there is a .ppstorage file in the originals and/or import folder
  • the storage, cache and/or config folders are not writable

In addition, .ppignore files containing * will automatically be created in the storage, cache, and config folders so that files and folders in these directories should be ignored while indexing (also by older versions of PhotoPrism).

An updated photoprism/photoprism:test image that includes those improvements is now available on Docker Hub. As written earlier, any help with testing the changes is much appreciated as I can only spend a limited amount of time on this issue and we want to make sure it works for everyone. Thank you very much!

lastzero added a commit that referenced this issue Jan 29, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
@graciousgrey graciousgrey added tested Changes have been tested successfully and removed please-test Ready for acceptance test labels Feb 12, 2024
@graciousgrey graciousgrey removed the help wanted Well suited for external contributors! label Apr 12, 2024
@graciousgrey graciousgrey added the released Available in the stable release label May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy Easy issue for beginners enhancement Refactoring, improvement or maintenance task released Available in the stable release tested Changes have been tested successfully
Projects
Status: Release 🌈
Development

No branches or pull requests

5 participants