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

Logical Paths: Allowed Characters and Path Separators #411

Closed
ahankinson opened this issue Dec 3, 2019 · 7 comments · Fixed by #423
Closed

Logical Paths: Allowed Characters and Path Separators #411

ahankinson opened this issue Dec 3, 2019 · 7 comments · Fixed by #423

Comments

@ahankinson
Copy link
Contributor

To address #407 the editors have decided to restrict characters in the content paths, but have not reached agreement on what can be in the logical paths. The options are:

  • Maintain the same restrictions as the content paths (aka no Windows-sensitive names)
  • Have no restrictions on the names INCLUDING that both forward and back slashes are allowed as logical names, depending on the system that was used to ingest the content.
  • Have no restrictions on the names EXCEPT that path separators must be normalized to forward (/) slashes.

The editors were in agreement that option 1 is not necessary. There was disagreement on points two and three, so this issue is to provide a space for additional feedback on the issue.

@pwinckles
Copy link

pwinckles commented Dec 3, 2019

I don't think that separators should be normalized in logical paths. The reason being that if a client has an API like object.addFile(logicalPathString, file) it's impossible for the client code to know what to do when backslashes occur in the logical path string. If they're part of a file name, they should be left unchanged, but if they're path separators they'd need to be transformed. How can you tell the difference? Should some\file\path be some/file/path, some\file/path, some/file\path, or some\file\path? The most you could do is recommend that people only use forward slashes as separators in logical paths.

The only logical path restrictions that seem valuable to me are in regards to ., .., and empty filenames (//).

@zimeon zimeon added this to the 1.0 milestone Dec 3, 2019
@julianmorley
Copy link
Contributor

julianmorley commented Dec 3, 2019

I also don't see any need to restrict content of logical paths; given how objects can be created, it's entirely possible to create a valid OCFL that has logical paths that never have any relation to any real file system, past, present or future. And that's OK as long as the logical paths make sense to some upstream system that consumes OCFL objects.

Edited to add: I guess what I'm saying is that logical paths don't need to make sense to OCFL or be resolvable to the current filesystem that the OCFL object is stored on; they just need to make sense to whatever system is consuming OCFL. In fact, I'm almost tempted to lobby for renaming logical paths to logical identifier.

@julianmorley
Copy link
Contributor

For example, I believe this should be perfectly valid OCFL:

  "manifest": {
    "cffe55838a878a29da82a0e10b2909b7e46b6f7167ed7f815782465573e98f27": [
      "v0001/data/my_content/dracula.txt"
    ]
  },
  "versions": {
    "v0001": { ... },
      "state": {
        "cffe55838a878a29da82a0e10b2909b7e46b6f7167ed7f815782465573e98f27": [
          "I found this behind the sofa, we think it's a book scan of some kind."
        ]
      }
    }

@rosy1280
Copy link
Contributor

Suggestion is to not provide restrictions on logical paths except for .,.., empty filenames (//) and not starting a filename with a /

@rosy1280
Copy link
Contributor

@julianmorley and @neilsjefferies can you thumbs up on the above comment if you agree?

@rosy1280
Copy link
Contributor

@ahankinson everyone has thumbed this up except for @julianmorley. Perhaps we have enough to move forward?

@zimeon
Copy link
Contributor

zimeon commented Feb 4, 2020

2020-02-04 - Editors agree to move forward based on #411 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants