Skip to content

Commit

Permalink
Fixed #189 - Update utils authority package to handle proper encoding…
Browse files Browse the repository at this point in the history
…/decoding of uri with reserved characters.
  • Loading branch information
funkyshu committed Jun 10, 2024
1 parent 7017a37 commit 9fdf005
Show file tree
Hide file tree
Showing 13 changed files with 210 additions and 154 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## [Unreleased]

## [6.14.3] - 2024-05-30
### Fixed
- Fixed #189 - Update utils authority package to handle proper encoding/decoding of uri with reserved characters.

## [6.14.2] - 2024-05-30
### Fixed
- Fixed #187 - Update to latest jlaffaye/ftp library to fix issue where FTPS connections were failing due to a bug in the library. Also updated dataconn to continue even if it fails to MakeDir.
Expand Down
12 changes: 9 additions & 3 deletions backend/ftp/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (f *File) MoveToFile(t vfs.File) error {
// ftp rename if vfs is ftp and for the same user/host
if f.fileSystem.Scheme() == t.Location().FileSystem().Scheme() &&
f.authority.UserInfo().Username() == t.(*File).authority.UserInfo().Username() &&
f.authority.Host() == t.(*File).authority.Host() {
f.authority.HostPortStr() == t.(*File).authority.HostPortStr() {

// ensure destination exists before moving
exists, err := t.Location().Exists()
Expand Down Expand Up @@ -246,7 +246,7 @@ func (f *File) CopyToFile(file vfs.File) (err error) { //nolint:gocyclo

if f.fileSystem.Scheme() == file.Location().FileSystem().Scheme() &&
f.authority.UserInfo().Username() == file.(*File).authority.UserInfo().Username() &&
f.authority.Host() == file.(*File).authority.Host() {
f.authority.HostPortStr() == file.(*File).authority.HostPortStr() {
// in the case that both files have the same authority we'll copy by writing a temporary
// file to mem and then writing it back to the ftp server
tempFile, err := f.createLocalTempFile()
Expand Down Expand Up @@ -436,7 +436,13 @@ func (f *File) Write(data []byte) (res int, err error) {

// URI returns the File's URI as a string.
func (f *File) URI() string {
return utils.GetFileURI(f)
loc := f.Location().(*Location)
return utils.EncodeURI(
f.fileSystem.Scheme(),
loc.Authority.UserInfo().Username(),
loc.Authority.HostPortStr(),
f.Path(),
)
}

// String implement fmt.Stringer, returning the file's URI as the default string.
Expand Down
6 changes: 6 additions & 0 deletions backend/ftp/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,12 @@ func (ts *fileTestSuite) TestPath() {
func (ts *fileTestSuite) TestURI() {
expected := "ftp://[email protected]:22/some/path/to/file.txt"
ts.Equal(expected, ts.testFile.URI(), "URI test")

expected = "ftp://domain.com%[email protected]:22/some/path/to/file.txt"
fs := NewFileSystem()

Check failure on line 1130 in backend/ftp/file_test.go

View workflow job for this annotation

GitHub Actions / lint

importShadow: shadow of imported from 'github.com/dsoprea/go-utility/v2/filesystem' package 'fs' (gocritic)
f, err := fs.NewFile("domain.com%[email protected]:22", "/some/path/to/file.txt")
ts.NoError(err)
ts.Equal(expected, f.URI(), "URI test")
}

func (ts *fileTestSuite) TestStringer() {
Expand Down
2 changes: 1 addition & 1 deletion backend/ftp/location.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func (l *Location) FileSystem() vfs.FileSystem {

// URI returns the Location's URI as a string.
func (l *Location) URI() string {
return utils.GetLocationURI(l)
return utils.EncodeURI(l.FileSystem().Scheme(), l.Authority.UserInfo().Username(), l.Authority.HostPortStr(), l.Path())
}

// String implement fmt.Stringer, returning the location's URI as the default string.
Expand Down
10 changes: 10 additions & 0 deletions backend/ftp/location_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,16 @@ func (lt *locationTestSuite) TestURI() {
file, err := lt.ftpfs.NewFile(authority, "/blah/file.txt")
lt.NoError(err)
lt.Equal("ftp://[email protected]/blah/file.txt", file.URI(), "file uri with user, pass, host")

authority = `domain.com\[email protected]`
file, err = lt.ftpfs.NewFile(authority, "/blah/file.txt")

Check failure on line 327 in backend/ftp/location_test.go

View workflow job for this annotation

GitHub Actions / lint

SA4006: this value of `file` is never used (staticcheck)
lt.Error(err)
lt.ErrorContains(err, "net/url: invalid userinfo", "file uri with bad user")

authority = `domain.com%[email protected]`
file, err = lt.ftpfs.NewFile(authority, "/blah/file.txt")
lt.NoError(err)
lt.Equal(`ftp://domain.com%[email protected]/blah/file.txt`, file.URI(), "file uri with percent-encoded character in user")
}

func (lt *locationTestSuite) TestString() {
Expand Down
10 changes: 8 additions & 2 deletions backend/sftp/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (f *File) MoveToFile(t vfs.File) error {
// sftp rename if vfs is sftp and for the same user/host
if f.fileSystem.Scheme() == t.Location().FileSystem().Scheme() &&
f.Authority.UserInfo().Username() == t.(*File).Authority.UserInfo().Username() &&
f.Authority.Host() == t.(*File).Authority.Host() {
f.Authority.HostPortStr() == t.(*File).Authority.HostPortStr() {
// ensure destination exists before moving
exists, err := t.Location().Exists()
if err != nil {
Expand Down Expand Up @@ -364,7 +364,13 @@ func (f *File) Write(data []byte) (res int, err error) {

// URI returns the File's URI as a string.
func (f *File) URI() string {
return utils.GetFileURI(f)
loc := f.Location().(*Location)
return utils.EncodeURI(
f.fileSystem.Scheme(),
loc.Authority.UserInfo().Username(),
loc.Authority.HostPortStr(),
f.Path(),
)
}

// String implement fmt.Stringer, returning the file's URI as the default string.
Expand Down
6 changes: 6 additions & 0 deletions backend/sftp/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,12 @@ func (ts *fileTestSuite) TestPath() {
func (ts *fileTestSuite) TestURI() {
expected := "sftp://[email protected]:22/some/path/to/file.txt"
ts.Equal(expected, ts.testFile.URI(), "URI test")

expected = "sftp://domain.com%[email protected]:22/some/path/to/file.txt"
fs := NewFileSystem()
f, err := fs.NewFile("domain.com%[email protected]:22", "/some/path/to/file.txt")
ts.NoError(err)
ts.Equal(expected, f.URI(), "URI test")
}

func (ts *fileTestSuite) TestStringer() {
Expand Down
2 changes: 1 addition & 1 deletion backend/sftp/location.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func (l *Location) FileSystem() vfs.FileSystem {

// URI returns the Location's URI as a string.
func (l *Location) URI() string {
return utils.GetLocationURI(l)
return utils.EncodeURI(l.FileSystem().Scheme(), l.Authority.UserInfo().Username(), l.Authority.HostPortStr(), l.Path())
}

// String implement fmt.Stringer, returning the location's URI as the default string.
Expand Down
10 changes: 10 additions & 0 deletions backend/sftp/location_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,16 @@ func (lt *locationTestSuite) TestURI() {
file, err := lt.sftpfs.NewFile(authority, "/blah/file.txt")
lt.NoError(err)
lt.Equal("sftp://[email protected]/blah/file.txt", file.URI(), "file uri with user, pass, host")

authority = `domain.com\[email protected]`
file, err = lt.sftpfs.NewFile(authority, "/blah/file.txt")

Check failure on line 160 in backend/sftp/location_test.go

View workflow job for this annotation

GitHub Actions / lint

SA4006: this value of `file` is never used (staticcheck)
lt.Error(err)
lt.ErrorContains(err, "net/url: invalid userinfo", "file uri with bad user")

authority = `domain.com%[email protected]`
file, err = lt.sftpfs.NewFile(authority, "/blah/file.txt")
lt.NoError(err)
lt.Equal(`sftp://domain.com%[email protected]/blah/file.txt`, file.URI(), "file uri with percent-encoded character in user")
}

func (lt *locationTestSuite) TestString() {
Expand Down
160 changes: 45 additions & 115 deletions utils/authority.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package utils
import (
"errors"
"fmt"
"net/url"
"regexp"
"strconv"
"strings"
Expand All @@ -18,8 +19,13 @@ import (
Where:
authority = [ userinfo "@" ] host [ ":" port ]
userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
host = IP-literal / IPv4address / reg-name
port = *DIGIT
reg-name = *( unreserved / pct-encoded / sub-delims )
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
sub-delims = "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "="
pct-encoded = "%" HEXDIG HEXDIG
*/

// ------------------------------------ host :port
Expand All @@ -30,21 +36,23 @@ type Authority struct {
userinfo UserInfo
host string
port uint16
url *url.URL
}

// UserInfo represents user/pass portion of a URI
type UserInfo struct {
username, password string
url *url.URL
}

// Username returns the username of a URI UserInfo. May be an empty string.
func (u UserInfo) Username() string {
return u.username
return u.url.User.Username()
}

// Password returns the password of a URI UserInfo. May be an empty string.
func (u UserInfo) Password() string {
return u.password
p, _ := u.url.User.Password()
return p
}

// String() returns a string representation of authority. It does not include password per
Expand All @@ -54,20 +62,22 @@ func (u UserInfo) Password() string {
// subcomponent unless the data after the colon is the empty string (indicating no password).
func (a Authority) String() string {
authority := a.HostPortStr()
if a.userinfo.Username() != "" {
authority = fmt.Sprintf("%s@%s", a.userinfo.Username(), authority)
if a.UserInfo().Username() != "" {
authority = fmt.Sprintf("%s@%s", a.UserInfo().Username(), authority)
}
return authority
}

// UserInfo returns the userinfo section of authority. userinfo is username and password(deprecated).
func (a Authority) UserInfo() UserInfo {
return a.userinfo
return UserInfo{
url: a.url,
}
}

// Host returns the host portion of an authority
func (a Authority) Host() string {
return a.host
return a.url.Hostname()
}

// Port returns the port portion of an authority
Expand All @@ -83,142 +93,62 @@ func (a Authority) HostPortStr() string {
return a.Host()
}

var schemeRE = regexp.MustCompile("^[A-Za-z][A-Za-z0-9+.-]*://")

// NewAuthority initializes Authority struct by parsing authority string.
func NewAuthority(authority string) (Authority, error) {
if authority == "" {
return Authority{}, errors.New("authority string may not be empty")
}
u, p, h, err := parseAuthority(authority)

var err error
matched := schemeRE.MatchString(authority)
if !matched {
authority = "scheme://" + authority
}

u, err := url.Parse(authority)
if err != nil {
return Authority{}, err
}

matches := portRegex.FindAllStringSubmatch(h, -1)
if len(matches) == 0 {
return Authority{}, errors.New("could not determine port")
}
host, portStr := splitHostPort(u.Host)
var port uint16
if matches[0][2] != "" {
val, err := strconv.ParseUint(matches[0][2], 10, 16)
if portStr != "" {
val, err := strconv.ParseUint(portStr, 10, 16)
if err != nil {
return Authority{}, err
}
port = uint16(val)
}

return Authority{
userinfo: UserInfo{
username: u,
password: p,
},
host: matches[0][1],
host: host,
port: port,
url: u,
}, nil
}

/*
NOTE: Below was mostly taken line-for-line from the "url" package (https://github.com/golang/go/blob/master/src/net/url/url.go),
minus unencoding and some unused split logic. Unfortunately none of it was exposed in a way that could be used for parsing authority.
Copyright (c) 2009 The Go Authors. All rights reserved.
Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
met:
* Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.
* Redistributions in binary form must reproduce the above
copyright notice, this list of conditions and the following disclaimer
in the documentation and/or other materials provided with the
distribution.
* Neither the name of Google Inc. nor the names of its
contributors may be used to endorse or promote products derived from
this software without specific prior written permission.
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

func parseAuthority(authority string) (username, password, host string, err error) {
i := strings.LastIndex(authority, "@")
if i < 0 {
host, err = parseHost(authority)
} else {
host, err = parseHost(authority[i+1:])
}
if err != nil {
return "", "", "", err
}
if i < 0 {
return "", "", host, nil
}
userinfo := authority[:i]
if !validUserinfo(userinfo) {
return "", "", host, errors.New("invalid userinfo")
}
if !strings.Contains(userinfo, ":") {
username = userinfo
} else {
username, password = split(userinfo, ":")
}

return
}

func split(s, c string) (string, string) {
i := strings.Index(s, c)
return s[:i], s[i+len(c):]
}
// splitHostPort separates host and port. If the port is not valid, it returns
// the entire input as host, and it doesn't check the validity of the host.
// Unlike net.SplitHostPort, but per RFC 3986, it requires ports to be numeric.
func splitHostPort(hostPort string) (host, port string) {
host = hostPort

func validUserinfo(s string) bool {
for _, r := range s {
if r >= 'A' && r <= 'Z' {
continue
}
if r >= 'a' && r <= 'z' {
continue
}
if r >= '0' && r <= '9' {
continue
}
switch r {
case '-', '.', '_', ':', '~', '!', '$', '&', '\'',
'(', ')', '*', '+', ',', ';', '=', '%', '@':
continue
default:
return false
}
colon := strings.LastIndexByte(host, ':')
if colon != -1 && validOptionalPort(host[colon:]) {
host, port = host[:colon], host[colon+1:]
}
return true
}

func parseHost(host string) (string, error) {
if strings.HasPrefix(host, "[") {
// Parse an IP-Literal in RFC 3986 and RFC 6874.
// E.g., "[fe80::1]", "[fe80::1%25en0]", "[fe80::1]:80".
i := strings.LastIndex(host, "]")
if i < 0 {
return "", errors.New("missing ']' in host")
}
colonPort := host[i+1:]
if !validOptionalPort(colonPort) {
return "", fmt.Errorf("invalid port %q after host", colonPort)
}
if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") {
host = host[1 : len(host)-1]
}

return host, nil
return
}

// validOptionalPort reports whether port is either an empty string
// or matches /^:\d*$/
func validOptionalPort(port string) bool {
if port == "" {
return true
Expand Down
Loading

0 comments on commit 9fdf005

Please sign in to comment.