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

Fix crash when calling absPath with empty input #620

Merged
merged 4 commits into from
Aug 9, 2024

Conversation

gzfuzz
Copy link
Contributor

@gzfuzz gzfuzz commented Aug 1, 2024

🦟 Bug fix

Fixes #614

Summary

The current version of common::absPath may cause crash when calling the /gazebo/resource_paths/resolve service with empty request. By changing filesystem::absolute to the non-throwing version, and check the error_code accordingly, the crash could be avoided.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

@gzfuzz gzfuzz requested a review from marcoag as a code owner August 1, 2024 04:51
@github-actions github-actions bot added 🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic labels Aug 1, 2024
@gzfuzz
Copy link
Contributor Author

gzfuzz commented Aug 1, 2024

It seems that fs::absolute("", ec) behaves differently on Ubuntu ("" with error code) and macOS (cwd() without error code?)... Should empty input be treated as a special case?

@iche033
Copy link
Contributor

iche033 commented Aug 7, 2024

from https://en.cppreference.com/w/cpp/filesystem/absolute:

For POSIX-based operating systems, std::filesystem::absolute(p) is equivalent to std::filesystem::current_path() / p except for when p is the empty path.

Doesn't really says what the expected result is if p is empty. Likely just OS dependent. I lean towards adding a special #ifdef __APPLE__ check in the test.

@iche033 iche033 merged commit e818abc into gazebosim:gz-common5 Aug 9, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Crash with stack trace gz::common::absPath
2 participants