-
Notifications
You must be signed in to change notification settings - Fork 16
error out when run_vminitd.sock path length is too long #89
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -246,10 +246,19 @@ func (v *vmInstance) Start(ctx context.Context, opts ...vm.StartOpt) (err error) | |||||
| return fmt.Errorf("failed to get cwd: %w", err) | ||||||
| } | ||||||
| socketPath := filepath.Join(v.state, "run_vminitd.sock") | ||||||
| // Compute the relative socket path to avoid exceeding the max length on macOS. | ||||||
| socketPath, err = filepath.Rel(cwd, socketPath) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to get relative socket path: %w", err) | ||||||
| } | ||||||
| // When the socket path exceeds macOS max length, it appears as if the VM | ||||||
| // didn't start properly. There's no easy way to figure this out as the | ||||||
| // only log is: "Timeout while waiting for VM to start". Thus, return an | ||||||
| // error preventively here. | ||||||
| if runtime.GOOS == "darwin" && len(socketPath) >= 104 { | ||||||
| return fmt.Errorf("socket path is too long: %s", socketPath) | ||||||
|
||||||
| return fmt.Errorf("socket path is too long: %s", socketPath) | |
| return fmt.Errorf("socket path exceeds macOS limit of 103 characters (%d): %s. Consider using a shorter state directory path or avoiding symlinked directories in /tmp", len(socketPath), socketPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition checks if length is >= 104, but since the 104-character limit includes the null terminator (\0), the check should be
> 103or>= 104with the actual path plus null terminator. The current check may reject valid 103-character paths. Consider changing tolen(socketPath) > 103to allow exactly 103 characters of actual path data.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think copilot got confused by your comment:
The limit is 104 bytes (so 103 chars + NUL byte), so the code is correct.