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: temp dir to be the same as SerchDir to avoid invalid cross-device link (#1203) #1241

Merged
merged 1 commit into from Jun 27, 2022

Conversation

tanopanta
Copy link
Contributor

@tanopanta tanopanta commented Jun 24, 2022

Describe the PR

Fix temp file logic t avoid invalid cross-device link

Relation issue
#1203

Additional context

Cause

This error occurs when os.Rename between different partitions

https://groups.google.com/g/golang-dev/c/5w7Jmg_iCJQ

In other words, TMPDIR and PWD are different partitions.

When ioutil.TempFile(dir, pattern string) 's first parameter is empty string, TempFile are created on TMPDIR.

If dir is the empty string, TempFile uses the default directory for temporary files (see os.TempDir).
https://pkg.go.dev/io/ioutil#TempFile

Solution
Fix filepath.Split() to filepath.Dir() and Base().
filepath.Split may return an empty string.

package main

import (
	"fmt"
	"path/filepath"
)

func main() {
	paths := []string{
		"/home/swaggo/main.go",
		"main.go",
	}

	fmt.Println("filepath.Split")
	for _, p := range paths {
		dir, file := filepath.Split(p)
		fmt.Printf("input: %q\n\tdir: %q\n\tfile: %q\n", p, dir, file)
	}
	fmt.Println("filepath.Dir, Base")
	for _, p := range paths {
		dir := filepath.Dir(p)
		file := filepath.Base(p)
		fmt.Printf("input: %q\n\tdir: %q\n\tfile: %q\n", p, dir, file)
	}
}
filepath.Split
input: "/home/swaggo/main.go"
	dir: "/home/swaggo/"
	file: "main.go"
input: "main.go"
	dir: ""
	file: "main.go"
filepath.Dir, Base
input: "/home/swaggo/main.go"
	dir: "/home/swaggo"
	file: "main.go"
input: "main.go"
	dir: "."
	file: "main.go"

Check

> pwd
/add_disk/swag
> go install github.com/swaggo/swag/cmd/swag@latest
> swag fmt main.go
2022/06/25 01:12:20 fmt: rename /tmp/main.go3296877051 main.go: invalid cross-device link
> make install
....
> swag fmt main.go
> 

@codecov
Copy link

codecov bot commented Jun 24, 2022

Codecov Report

Merging #1241 (f9f81af) into master (0e2ec6c) will not change coverage.
The diff coverage is 100.00%.

❗ Current head f9f81af differs from pull request most recent head 41ab438. Consider uploading reports for the commit 41ab438 to get more accurate results

@@           Coverage Diff           @@
##           master    #1241   +/-   ##
=======================================
  Coverage   94.83%   94.83%           
=======================================
  Files          14       14           
  Lines        2614     2614           
=======================================
  Hits         2479     2479           
  Misses         74       74           
  Partials       61       61           
Impacted Files Coverage Δ
format/format.go 92.30% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e2ec6c...41ab438. Read the comment docs.

@@ -103,7 +103,7 @@ func (f *Format) format(path string) error {
}

func write(path string, contents []byte) error {
f, err := ioutil.TempFile(filepath.Split(path))
f, err := ioutil.TempFile(filepath.Dir(path), filepath.Base(path))
Copy link
Contributor Author

@tanopanta tanopanta Jun 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This line is the same as formatter.go (L372) before modifying #1192
https://github.com/swaggo/swag/pull/1192/files

Copy link
Contributor

@ubogdan ubogdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ubogdan ubogdan merged commit 796a346 into swaggo:master Jun 27, 2022
@ubogdan
Copy link
Contributor

ubogdan commented Jun 27, 2022

@tanopanta Thanks for your contribution.

@tanopanta tanopanta deleted the fix-invalid-cross-device-link branch June 28, 2022 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants