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

Crash with stack trace gz::common::absPath #614

Closed
gzfuzz opened this issue Jul 1, 2024 · 5 comments · Fixed by #620
Closed

Crash with stack trace gz::common::absPath #614

gzfuzz opened this issue Jul 1, 2024 · 5 comments · Fixed by #620
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@gzfuzz
Copy link
Contributor

gzfuzz commented Jul 1, 2024

Environment

  • OS Version: Ubuntu 22.04
  • Source or binary build?
    source build
    gz-sim version: 5641ef2cc7bed13e06892d29ffd3663b2f172e76
    gz-common version: 1ffd6ef
    built with
    gcc version: 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04)
    build options: -DCMAKE_BUILD_TYPE=Coverage

Description

  • Expected behavior: Gazebo doesn't crash
  • Actual behavior: Gazebo crashes

Steps to reproduce:

  1. run gz sim with any sdf file
  2. gz service --timeout 10000 -s /gazebo/resource_paths/resolve --reptype gz.msgs.StringMsg --reqtype gz.msgs.StringMsg --req ''

Output

a.log

@gzfuzz gzfuzz added the bug Something isn't working label Jul 1, 2024
@gzfuzz
Copy link
Contributor Author

gzfuzz commented Jul 2, 2024

https://github.com/gazebosim/gz-common/blob/gz-common5/src/Filesystem.cc#L136

std::string common::absPath(const std::string &_path)
{
  return fs::absolute(_path).string();
}

The root cause seems to be here, no empty check for _path, or shall it be fixed in the gz-sim repository?

https://github.com/gazebosim/gz-sim/blob/gz-sim8/src/ServerPrivate.cc#L489

bool ServerPrivate::ResourcePathsResolveService(
    const msgs::StringMsg &_req,
    msgs::StringMsg &_res)
{
  // Get the request
  std::string req = _req.data();
  // return false for empty req?

  // Handle the case where the request is already a valid path
  if (common::exists(common::absPath(req)))
  {
    _res.set_data(common::absPath(req));
    return true;
  }
  ...
}

@mjcarroll
Copy link
Contributor

Thanks for bringing this up.

It should probably be updated to use the std::error_code variant so that it doesn't crash but instead returns some sort of error, similar to create_directories

bool common::createDirectories(const std::string &_path)
{
std::error_code ec;
// Disregard return of create_directories, because it may return false if the
// directory is not actually created (already exists)
bool created = fs::create_directories(_path, ec);
(void) created;
return fsWarn("createDirectories", ec);
}

@azeey azeey moved this from Inbox to To do in Core development Jul 22, 2024
@azeey
Copy link
Contributor

azeey commented Jul 22, 2024

@zhileiren are you interested in working on this? Can I assign it to you?

@azeey azeey added the help wanted Extra attention is needed label Jul 22, 2024
@gzfuzz
Copy link
Contributor Author

gzfuzz commented Jul 29, 2024

Sure I'm interested. I've not contributed to the gazebo repository before, but if it's appropriate, I would like to try filing a pull request fixing this issue.

@azeey
Copy link
Contributor

azeey commented Jul 30, 2024

Thanks! I've assigned the issue to you. Let me know if you need help with creating the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants