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

all sandbox methods should return Abs Paths? #53

Open
srghma opened this issue Oct 9, 2024 · 3 comments · May be fixed by #54
Open

all sandbox methods should return Abs Paths? #53

srghma opened this issue Oct 9, 2024 · 3 comments · May be fixed by #54

Comments

@srghma
Copy link
Member

srghma commented Oct 9, 2024

Describe the bug

I write https://github.com/srghma/purescript-pathy-node/blob/56c0cbce481ac50b11f5b66690967701a5281d65/test/Test/Main.purs#L109-L110

    -- outerTmpDir :: SandboxedPath Rel Dir
    -- outerTmpDir = sandboxAny (currentDir </> dir (Proxy :: _ "tmp") </> dir (Proxy :: _ "dir-entries-test"))

and it gives "/tmp/dir-entries-test/"

and I'm like "WHAAT, should be rel"

To Reproduce

add this to tests

  test "sandbox - Rel path, but should be abosolute?"
    ( let (relDir :: Maybe (SandboxedPath Rel Dir)) = sandbox rootDir (currentDir </> dirBar)
      in printPath posixPrinter <$> relDir)
    (Just "/bar/")

  test "sandbox - Rel path, but should be abosolute?"
    ( let (relDir :: Maybe (SandboxedPath Rel Dir)) = sandbox (rootDir </> dirBar) (currentDir </> dirBar)
      in printPath posixPrinter <$> relDir)
    (Just "/bar/bar/")

Expected behavior

maybe

data SandboxedPath a b = SandboxedPath (Path Abs Dir) (Path a b)

should be

data SandboxedPath dirOrFile = SandboxedPath (Path Abs Dir) (Path Abs dirOrFile)

@srghma
Copy link
Member Author

srghma commented Oct 9, 2024

or maybe this is ok, since I can write

sandboxOrThrow
  :: forall a b m
   . IsRelOrAbs a
  => IsDirOrFile b
  => MonadThrow Error m
  => Path Abs Dir
  -> Path a b
  -> m (SandboxedPath a b)
sandboxOrThrow root path = sandbox root path # maybe (throwError $ error $ "cannot sandbox path: root = " <> show root <> ", path = " <> show path) pure

(outerTmpDir :: SandboxedPath Rel Dir) <- sandboxOrThrow cwd (currentDir </> dir (Proxy :: _ "tmp") </> dir (Proxy :: _ "dir-entries-test"))

but on other hand - what is the use of currentDir inside of a sandbox method, maybe currentDir makes everything harder to understand

@srghma
Copy link
Member Author

srghma commented Oct 9, 2024

well, someone could write getFirstFile whose output form depends on the input

-- ./mydir/ -> ./mydir/myfile.txt
-- /myabspath/mydir/ -> /myabspath/mydir/myfile.txt
getFirstFile :: FilePath -> Aff FilePath
getFirstFile = ...

getFirstFileAff
  :: forall relOrAbs
   . IsRelOrAbs relOrAbs
  => SandboxedPath relOrAbs Dir
  -> Aff (Maybe (Path relOrAbs File))
getFirstFileAff path = do
  let printedPath = printPath currentPrinter path
  (maybeFile :: Maybe FilePath) <- Node.FS.getFirstFile path
  map (parse currentParser parseRelFile # ...) maybeFile

and this would be wrong - printPath will always return Abs Path

it can only ever be

getFirstFileAff
  :: forall relOrAbs
   . IsRelOrAbs relOrAbs
  => SandboxedPath relOrAbs Dir
  -> Aff (Maybe (Path Abs File))

srghma added a commit to srghma/purescript-pathy that referenced this issue Oct 9, 2024
@srghma
Copy link
Member Author

srghma commented Oct 9, 2024

Implemented, works ok, error is thrown now if I return Rel
2024-10-09-01pm-00-30-screenshot

also implemented a SafeAppend for Sandboxed (allows only adding new paths, disallows going up)

srghma@5a747ea

@srghma srghma linked a pull request Oct 11, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant