bug: IsParent doesn't handle the case where paths are empty properly

[HarikrishnanBalagopal]

Bug

Overview

https://github.com/konveyor/move2kube/blob/e88ceceee7e8e76a587827d9555035566d4d0b38/common/utils.go#L1078

When path is relative it joins with the current working. In WASM the current working directory is / and we hit this if condition https://github.com/konveyor/move2kube/blob/e88ceceee7e8e76a587827d9555035566d4d0b38/common/utils.go#L1088-L1090 so it returns true.

And because IsParent returns true, we try to make a path relative to an empty string '' and hit this error https://github.com/konveyor/move2kube/blob/e88ceceee7e8e76a587827d9555035566d4d0b38/environment/environment.go#L272-L274

Fix

• Make sure all the paths are absolute before giving to these functions. We should be using absolute paths everywhere.
• Also add a if parent == "" || child == "" { return false; } in the IsParent function

[akagami-harsh]

hey @HarikrishnanBalagopal, i would like to work on this issue. can you please assign this to me

[akagami-harsh]

Make sure all the paths are absolute before giving to these functions. We should be using absolute paths everywhere.

Do I need to make changes throughout the entire repository where the isParent function is being called

[HarikrishnanBalagopal]

Make sure all the paths are absolute before giving to these functions. We should be using absolute paths everywhere.

Do I need to make changes throughout the entire repository where the isParent function is being called

Not only the places where IsParent is called, internally everywhere we should be using absolute paths. You can use a bunch of print statements to check where the relative paths are coming from and fix it at the source.

There are a few places where relative paths may be necessary but I can't recall right now where that is.

[akagami-harsh]

Make sure all the paths are absolute before giving to these functions. We should be using absolute paths everywhere.

Do I need to make changes throughout the entire repository where the isParent function is being called

Not only the places where IsParent is called, internally everywhere we should be using absolute paths. You can use a bunch of print statements to check where the relative paths are coming from and fix it at the source.

There are a few places where relative paths may be necessary but I can't recall right now where that is.

IsParent already uses the filepath.Abs() to convert path form relative to absolute, then why do we need to pass absoulte path.

func IsParent(child, parent string) bool {
    var err error
    child, err = filepath.Abs(child)
    if err != nil {
        logrus.Fatalf("Failed to make the path %s absolute. Error: %s", child, err)
    }
    parent, err = filepath.Abs(parent)
    if err != nil {
        logrus.Fatalf("Failed to make the path %s absolute. Error: %s", parent, err)
    }
    if parent == "/" {
        return true
    }

[HarikrishnanBalagopal]

Make sure all the paths are absolute before giving to these functions. We should be using absolute paths everywhere.

Do I need to make changes throughout the entire repository where the isParent function is being called

Not only the places where IsParent is called, internally everywhere we should be using absolute paths. You can use a bunch of print statements to check where the relative paths are coming from and fix it at the source. There are a few places where relative paths may be necessary but I can't recall right now where that is.

IsParent already uses the filepath.Abs() to convert path form relative to absolute, then why do we need to pass absoulte path.

func IsParent(child, parent string) bool {
  var err error
  child, err = filepath.Abs(child)
  if err != nil {
      logrus.Fatalf("Failed to make the path %s absolute. Error: %s", child, err)
  }
  parent, err = filepath.Abs(parent)
  if err != nil {
      logrus.Fatalf("Failed to make the path %s absolute. Error: %s", parent, err)
  }
  if parent == "/" {
      return true
  }

because that's the bug. The path in the calling context is a relative path (or empty string) and the path inside the isParent is an absolute path (empty string becomes slash in wasm after combining with the PWD /). See the first post for more details.

In general we try to use only absolute paths in all the internal functions to avoid these sort of conversions and associated bugs.

[satyampsoni]

@akagami-harsh are you working it? I have made the changes, if you are not working please let me know I'll make the pr

[akagami-harsh]

@akagami-harsh are you working it? I have made the changes, if you are not working please let me know I'll make the pr

@satyampsoni Go for it.