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

Content directory path separators #530

Closed
pwinckles opened this issue Mar 18, 2021 · 3 comments · Fixed by #566
Closed

Content directory path separators #530

pwinckles opened this issue Mar 18, 2021 · 3 comments · Fixed by #566
Assignees
Labels
OCFL Object Ready for Review Ready for review by editorial group Validation Validation errors, warnings and other issues
Milestone

Comments

@pwinckles
Copy link

pwinckles commented Mar 18, 2021

The contentDirectory value MUST NOT contain the forward slash (/) path separator and MUST NOT be either one or two periods (. or ..).

https://ocfl.io/1.0/spec/#content-directory

I believe the intent of that sentence is that the content directory must be a direct child of the version directory. The problem is that it only forbids the / path separator, so on Windows you could legally have nested\content\dir or even ..\..\..\..\..\..\.

This is complicated by:

The forward slash (/) path separator MUST be used in content paths in the manifest and fixity blocks within the inventory. Implementations that target systems using other separators will need to translate paths appropriately.

https://ocfl.io/1.0/spec/#inventory

However, that statement does not mandate a / separator in contentDirectory, presumably because it's not supposed to able to have any path separator.

@zimeon zimeon added OCFL Object Validation Validation errors, warnings and other issues labels Mar 26, 2021
@zimeon
Copy link
Contributor

zimeon commented Mar 26, 2021

I think when reading these things one has to be careful to distinguish between rules for the inventory and how that might be mapped onto a filesystem or store that perhaps does not use unix conventions. We've made it so that the mapping to unix conventions or to URIs is easy, but other things may be more tricky (like Windows)

Per https://ocfl.io/1.0/spec/#inventory:

The forward slash (/) path separator must be used in content paths in the manifest and fixity blocks within the inventory. Implementations that target systems using other separators will need to translate paths appropriately.

Reading the comment above I'm not quite sure what the question is.

@pwinckles
Copy link
Author

From my perspective this is the problem.

  1. I am using Windows.
  2. I set contentDirectory to equal nested\content\dir
  3. I add a file to my object and write it to the manifest as v1/nested/content/dir/file.txt

By my reading of the spec, this is unintended but valid.

@zimeon zimeon added this to the 1.1 milestone May 4, 2021
@awoods
Copy link
Member

awoods commented Sep 6, 2021

My understanding of the following text is that the "contentDirectory" must be direct child directory of the version directory in which it is found. I believe the intent of allowing configurable "contentDirectory" was to be able to use a directory name other than content, not to open the door to a nested directory structure as the parent of content files:

The contentDirectory value MUST NOT contain the forward slash (/) path separator and MUST NOT be either one or two periods (. or ..).

Maybe the following language to 3.3.1 Content Directory would help clarify.
Change:

The contentDirectory value MUST NOT contain the forward slash (/) path separator and MUST NOT be either one or two periods (. or ..).

To:

The contentDirectory value MUST represent a direct child directory of the version directory in which it is found. As such, the contentDirectory value MUST NOT contain the forward slash (/) path separator and MUST NOT be either one or two periods (. or ..).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCFL Object Ready for Review Ready for review by editorial group Validation Validation errors, warnings and other issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants