Skip to content

Commit dd48b19

Browse files
committed
improve comments and tests
1 parent cb9c518 commit dd48b19

File tree

2 files changed

+21
-9
lines changed

2 files changed

+21
-9
lines changed

modules/util/path.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,17 @@ import (
1414
"strings"
1515
)
1616

17-
// SafePathRel joins the path elements into a single path, each element is cleaned separately
18-
// It only returns the following values (like path.Join):
17+
// SafePathRel joins the path elements into a single path, each element is cleaned by path.Clean separately.
18+
// It only returns the following values (like path.Join), any redundant part (empty, relative dots, slashes) is removed.
1919
//
2020
// empty => ``
2121
// `` => ``
2222
// `..` => `.`
2323
// `dir` => `dir`
2424
// `/dir/` => `dir`
2525
// `foo\..\bar` => `foo\..\bar`
26-
//
27-
// any redundant part(relative dots, slashes) is removed.
26+
// {`foo`, ``, `bar`} => `foo/bar`
27+
// {`foo`, `..`, `bar`} => `foo/bar`
2828
func SafePathRel(elem ...string) string {
2929
elems := make([]string, len(elem))
3030
for i, e := range elem {
@@ -44,12 +44,12 @@ func SafePathRel(elem ...string) string {
4444
}
4545

4646
// SafePathRelX joins the path elements into a single path like SafePathRel,
47-
// and covert all backslashes to slashes. (X means "extended", also means the combination of `\` and `/`)
47+
// and covert all backslashes to slashes. (X means "extended", also means the combination of `\` and `/`).
4848
// It returns similar results as SafePathRel except:
4949
//
5050
// `foo\..\bar` => `bar` (because it's processed as `foo/../bar`)
5151
//
52-
// any redundant part(relative dots, slashes) is removed.
52+
// All backslashes are handled as slashes, the result only contains slashes.
5353
func SafePathRelX(elem ...string) string {
5454
elems := make([]string, len(elem))
5555
for i, e := range elem {
@@ -63,9 +63,13 @@ func SafePathRelX(elem ...string) string {
6363

6464
const pathSeparator = string(os.PathSeparator)
6565

66-
// SafeFilePathAbs joins the path elements into a single file path, each element is cleaned separately
67-
// and convert all slashes/backslashes to path separators.
68-
// The first element must be an absolute path, caller should prepare the base path
66+
// SafeFilePathAbs joins the path elements into a single file path, each element is cleaned by filepath.Clean separately.
67+
// All slashes/backslashes are converted to path separators before cleaning, the result only contains path separators.
68+
// The first element must be an absolute path, caller should prepare the base path.
69+
// Like SafePathRel, any redundant part (empty, relative dots, slashes) is removed.
70+
//
71+
// {`/foo`, ``, `bar`} => `/foo/bar`
72+
// {`/foo`, `..`, `bar`} => `/foo/bar`
6973
func SafeFilePathAbs(elem ...string) string {
7074
elems := make([]string, len(elem))
7175

modules/util/path_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ func TestCleanPath(t *testing.T) {
149149
{[]string{`/a/`}, `a`},
150150
{[]string{`../a/`, `../b`, `c/..`, `d`}, `a/b/d`},
151151
{[]string{`a\..\b`}, `a\..\b`},
152+
{[]string{`a`, ``, `b`}, `a/b`},
153+
{[]string{`a`, `..`, `b`}, `a/b`},
152154
}
153155
for _, c := range cases {
154156
assert.Equal(t, c.expected, SafePathRel(c.elems...), "case: %v", c.elems)
@@ -165,6 +167,8 @@ func TestCleanPath(t *testing.T) {
165167
{[]string{`/a/`}, `a`},
166168
{[]string{`../a/`, `../b`, `c/..`, `d`}, `a/b/d`},
167169
{[]string{`a\..\b`}, `b`},
170+
{[]string{`a`, ``, `b`}, `a/b`},
171+
{[]string{`a`, `..`, `b`}, `a/b`},
168172
}
169173
for _, c := range cases {
170174
assert.Equal(t, c.expected, SafePathRelX(c.elems...), "case: %v", c.elems)
@@ -181,6 +185,8 @@ func TestCleanPath(t *testing.T) {
181185
{[]string{`C:\a/`}, `C:\a`},
182186
{[]string{`C:\..\a\`, `../b`, `c\..`, `d`}, `C:\a\b\d`},
183187
{[]string{`C:\a/..\b`}, `C:\b`},
188+
{[]string{`C:\a`, ``, `b`}, `C:\a\b`},
189+
{[]string{`C:\a`, `..`, `b`}, `C:\a\b`},
184190
}
185191
} else {
186192
cases = []struct {
@@ -192,6 +198,8 @@ func TestCleanPath(t *testing.T) {
192198
{[]string{`/a/`}, `/a`},
193199
{[]string{`/../a/`, `../b`, `c/..`, `d`}, `/a/b/d`},
194200
{[]string{`/a\..\b`}, `/b`},
201+
{[]string{`/a`, ``, `b`}, `/a/b`},
202+
{[]string{`/a`, `..`, `b`}, `/a/b`},
195203
}
196204
}
197205
for _, c := range cases {

0 commit comments

Comments
 (0)