From 841c167618133056a04d9287fdb05e5b8ba35f54 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 29 Mar 2023 03:55:57 +0000 Subject: [PATCH 01/13] refactor: replace terminal package Signed-off-by: Billy Zha --- cmd/oras/root/login.go | 68 +++++++++++++++++++----------------------- go.mod | 10 +++---- go.sum | 53 +++++++++++++++++--------------- 3 files changed, 65 insertions(+), 66 deletions(-) diff --git a/cmd/oras/root/login.go b/cmd/oras/root/login.go index ae2ebe89f..7d4b1d96e 100644 --- a/cmd/oras/root/login.go +++ b/cmd/oras/root/login.go @@ -18,13 +18,12 @@ package root import ( "bufio" "context" - "errors" "fmt" "os" "strings" - "github.com/moby/term" "github.com/spf13/cobra" + "golang.org/x/term" "oras.land/oras/cmd/oras/internal/option" "oras.land/oras/internal/credential" ) @@ -76,30 +75,36 @@ Example - Log in with username and password in an interactive terminal and no TL func runLogin(ctx context.Context, opts loginOptions) (err error) { ctx, _ = opts.WithContext(ctx) - // prompt for credential if opts.Password == "" { + // prompt for credential + reader := bufio.NewReader(os.Stdin) if opts.Username == "" { - // prompt for username - username, err := readLine("Username: ", false) + fmt.Print("username: ") + username, err := readLine(reader) if err != nil { return err } - opts.Username = strings.TrimSpace(username) + opts.Username = strings.TrimSpace(string(username)) } + + fd := int(os.Stdin.Fd()) + prompt := "password" if opts.Username == "" { - // prompt for token - if opts.Password, err = readLine("Token: ", true); err != nil { - return err - } else if opts.Password == "" { - return errors.New("token required") - } + prompt = "token" + } + fmt.Printf("%s: ", prompt) + var bytes []byte + if term.IsTerminal(fd) { + bytes, err = term.ReadPassword(fd) } else { - // prompt for password - if opts.Password, err = readLine("Password: ", true); err != nil { - return err - } else if opts.Password == "" { - return errors.New("password required") - } + bytes, err = readLine(reader) + } + if err != nil { + return + } + fmt.Println() + if opts.Password = string(bytes); opts.Password == "" { + return fmt.Errorf("%s required", prompt) } } @@ -131,26 +136,15 @@ func runLogin(ctx context.Context, opts loginOptions) (err error) { return nil } -func readLine(prompt string, silent bool) (string, error) { - fmt.Print(prompt) - if silent { - fd := os.Stdin.Fd() - state, err := term.SaveState(fd) +func readLine(reader *bufio.Reader) (content []byte, err error) { + var line []byte + for more := true; more; { + // read until no more + line, more, err = reader.ReadLine() if err != nil { - return "", err + return nil, err } - term.DisableEcho(fd, state) - defer term.RestoreTerminal(fd, state) - } - - reader := bufio.NewReader(os.Stdin) - line, _, err := reader.ReadLine() - if err != nil { - return "", err - } - if silent { - fmt.Println() + content = append(content, line...) } - - return string(line), nil + return } diff --git a/go.mod b/go.mod index 865a8f21c..f9c59bf47 100644 --- a/go.mod +++ b/go.mod @@ -4,23 +4,23 @@ go 1.20 require ( github.com/docker/cli v23.0.2+incompatible - github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6 github.com/need-being/go-tree v0.1.0 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.0-rc2 github.com/sirupsen/logrus v1.9.0 github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 + golang.org/x/term v0.7.0 gopkg.in/yaml.v3 v3.0.1 oras.land/oras-go/v2 v2.0.2 ) require ( - github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect - github.com/docker/docker v20.10.17+incompatible // indirect - github.com/docker/docker-credential-helpers v0.6.4 // indirect + github.com/docker/docker v23.0.3+incompatible // indirect + github.com/docker/docker-credential-helpers v0.7.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/pkg/errors v0.9.1 // indirect golang.org/x/sync v0.1.0 // indirect - golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 // indirect + golang.org/x/sys v0.7.0 // indirect + gotest.tools/v3 v3.4.0 // indirect ) diff --git a/go.sum b/go.sum index 64b301ce1..4a2a6cac9 100644 --- a/go.sum +++ b/go.sum @@ -1,32 +1,23 @@ -github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8= -github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= -github.com/creack/pty v1.1.11 h1:07n33Z8lZxZ2qwegKbObQohDhXDQxiMMz1NOUGYlesw= -github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/danieljoos/wincred v1.1.0/go.mod h1:XYlo+eRTsVA9aHGp7NGjFkPla4m+DCL7hqDjlFjiygg= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/docker/cli v23.0.2+incompatible h1:Yj4wkrNtyCNLCMobKDYzEUIsbtMbfAulkHMH75/ecik= github.com/docker/cli v23.0.2+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= -github.com/docker/docker v20.10.17+incompatible h1:JYCuMrWaVNophQTOrMMoSwudOVEfcegoZZrleKc1xwE= -github.com/docker/docker v20.10.17+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= -github.com/docker/docker-credential-helpers v0.6.4 h1:axCks+yV+2MR3/kZhAmy07yC56WZ2Pwu/fKWtKuZB0o= -github.com/docker/docker-credential-helpers v0.6.4/go.mod h1:ofX3UI0Gz1TteYBjtgs07O36Pyasyp66D2uKT7H8W1c= -github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= -github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4= -github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/docker/docker v23.0.3+incompatible h1:9GhVsShNWz1hO//9BNg/dpMnZW25KydO4wtVxWAIbho= +github.com/docker/docker v23.0.3+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= +github.com/docker/docker-credential-helpers v0.7.0 h1:xtCHsjxogADNZcdv1pKUHXryefjlVRqWqIhk/uXJp0A= +github.com/docker/docker-credential-helpers v0.7.0/go.mod h1:rETQfLdHNT3foU5kuNkFR1R1V12OJRRO5lzt2D1b5X0= +github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= +github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6 h1:dcztxKSvZ4Id8iPpHERQBbIJfabdt4wUm5qy3wOL2Zc= -github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6/go.mod h1:E2VnQOmVuvZB6UYnnDB0qG5Nq/1tD9acaOpo6xmt0Kw= github.com/need-being/go-tree v0.1.0 h1:blQrtD006cFm97UDeMUfixwPc9o06A6c+uLaUskdNNw= github.com/need-being/go-tree v0.1.0/go.mod h1:UOHUchuOm+lxM+EtvQ9h/IO88hK/ke7FHai4oGhhEoI= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.0-rc2 h1:2zx/Stx4Wc5pIPDvIxHXvXtQFW/7XWJGmnM7r3wg034= github.com/opencontainers/image-spec v1.1.0-rc2/go.mod h1:3OVijpioIKYWTqjiG0zfF6wvoJ4fAXGbjdZuI2NgsRQ= -github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -36,33 +27,47 @@ github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0 github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I= github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= -github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 h1:0A+M6Uqn+Eje4kHMK80dtF3JCXC4ykBgQG4Fe06QRhQ= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU= +golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.7.0 h1:BEvjmm5fURWqcfbSKTdpkDXYBrUS1c0m8agp14W48vQ= +golang.org/x/term v0.7.0/go.mod h1:P32HKFT3hSsZrRxla30E9HqToFYAQPCMs/zFMBUFqPY= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gotest.tools/v3 v3.0.2 h1:kG1BFyqVHuQoVQiR1bWGnfz/fmHvvuiSPIV7rvl360E= -gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk= +gotest.tools/v3 v3.4.0 h1:ZazjZUfuVeZGLAmlKKuyv3IKP5orXcwtOwDQH6YVr6o= +gotest.tools/v3 v3.4.0/go.mod h1:CtbdzLSsqVhDgMtKsx03ird5YTGB3ar27v0u/yKBW5g= oras.land/oras-go/v2 v2.0.2 h1:3aSQdJ7EUC0ft2e9PjJB9Jzastz5ojPA4LzZ3Q4YbUc= oras.land/oras-go/v2 v2.0.2/go.mod h1:PWnWc/Kyyg7wUTUsDHshrsJkzuxXzreeMd6NrfdnFSo= From 31b3dc7e4e1d84be87b0864c0f73b47ee606820d Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 29 Mar 2023 04:02:14 +0000 Subject: [PATCH 02/13] add e2e tests Signed-off-by: Billy Zha --- test/e2e/suite/auth/auth.go | 50 +++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/test/e2e/suite/auth/auth.go b/test/e2e/suite/auth/auth.go index bfe9139a5..af2b91e9c 100644 --- a/test/e2e/suite/auth/auth.go +++ b/test/e2e/suite/auth/auth.go @@ -16,6 +16,8 @@ limitations under the License. package scenario import ( + "fmt" + "strings" "time" . "github.com/onsi/ginkgo/v2" @@ -52,8 +54,52 @@ var _ = Describe("Common registry user", Ordered, func() { ORAS("login", Host, "-u", Username, "-p", Password, "--registry-config", AuthConfigPath). WithTimeOut(20*time.Second). MatchContent("Login Succeeded\n"). - MatchErrKeyWords("WARNING", "Using --password via the CLI is insecure", "Use --password-stdin"). - WithDescription("login with username&password flags").Exec() + MatchErrKeyWords("WARNING", "Using --password via the CLI is insecure", "Use --password-stdin").Exec() + }) + + It("should fail if no username input", func() { + ORAS("login", Host, "--registry-config", AuthConfigPath). + WithTimeOut(20 * time.Second). + WithInput(strings.NewReader("")). + MatchKeyWords("username:"). + ExpectFailure(). + Exec() + }) + + It("should fail if no password input", func() { + ORAS("login", Host, "--registry-config", AuthConfigPath). + WithTimeOut(20*time.Second). + MatchKeyWords("username: ", "password: "). + WithInput(strings.NewReader(fmt.Sprintf("%s\n", Username))).ExpectFailure().Exec() + }) + + It("should fail if password is empty", func() { + ORAS("login", Host, "--registry-config", AuthConfigPath). + WithTimeOut(20*time.Second). + MatchKeyWords("username: ", "password: "). + MatchErrKeyWords("Error: password required"). + WithInput(strings.NewReader(fmt.Sprintf("%s\n\n", Username))).ExpectFailure().Exec() + }) + + It("should fail if no token input", func() { + ORAS("login", Host, "--registry-config", AuthConfigPath). + WithTimeOut(20*time.Second). + MatchKeyWords("username: ", "token: "). + WithInput(strings.NewReader("\n")).ExpectFailure().Exec() + }) + + It("should fail if token is empty", func() { + ORAS("login", Host, "--registry-config", AuthConfigPath). + WithTimeOut(20*time.Second). + MatchKeyWords("username: ", "token: "). + MatchErrKeyWords("Error: token required"). + WithInput(strings.NewReader("\n\n")).ExpectFailure().Exec() + }) + It("should use prompted input", func() { + ORAS("login", Host, "--registry-config", AuthConfigPath). + WithTimeOut(20*time.Second). + WithInput(strings.NewReader(fmt.Sprintf("%s\n%s\n", Username, Password))). + MatchKeyWords("Username: ", "Password: ", "Login Succeeded\n").Exec() }) }) }) From 9535df2832b24518c089c91b71429ca89df013ba Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 29 Mar 2023 04:08:32 +0000 Subject: [PATCH 03/13] code clean Signed-off-by: Billy Zha --- cmd/oras/root/login.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/oras/root/login.go b/cmd/oras/root/login.go index 7d4b1d96e..8b29ccc0b 100644 --- a/cmd/oras/root/login.go +++ b/cmd/oras/root/login.go @@ -87,13 +87,14 @@ func runLogin(ctx context.Context, opts loginOptions) (err error) { opts.Username = strings.TrimSpace(string(username)) } - fd := int(os.Stdin.Fd()) prompt := "password" if opts.Username == "" { prompt = "token" } fmt.Printf("%s: ", prompt) + var bytes []byte + fd := int(os.Stdin.Fd()) if term.IsTerminal(fd) { bytes, err = term.ReadPassword(fd) } else { From ce5314ae1757abf621a5166e8e710b5121d185a6 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 29 Mar 2023 04:47:38 +0000 Subject: [PATCH 04/13] modify readLine only Signed-off-by: Billy Zha --- cmd/oras/root/login.go | 66 ++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/cmd/oras/root/login.go b/cmd/oras/root/login.go index 8b29ccc0b..625bca984 100644 --- a/cmd/oras/root/login.go +++ b/cmd/oras/root/login.go @@ -18,6 +18,7 @@ package root import ( "bufio" "context" + "errors" "fmt" "os" "strings" @@ -75,37 +76,30 @@ Example - Log in with username and password in an interactive terminal and no TL func runLogin(ctx context.Context, opts loginOptions) (err error) { ctx, _ = opts.WithContext(ctx) + // prompt for credential if opts.Password == "" { - // prompt for credential - reader := bufio.NewReader(os.Stdin) if opts.Username == "" { - fmt.Print("username: ") - username, err := readLine(reader) + // prompt for username + username, err := readLine("Username: ", false) if err != nil { return err } - opts.Username = strings.TrimSpace(string(username)) + opts.Username = strings.TrimSpace(username) } - - prompt := "password" if opts.Username == "" { - prompt = "token" - } - fmt.Printf("%s: ", prompt) - - var bytes []byte - fd := int(os.Stdin.Fd()) - if term.IsTerminal(fd) { - bytes, err = term.ReadPassword(fd) + // prompt for token + if opts.Password, err = readLine("Token: ", true); err != nil { + return err + } else if opts.Password == "" { + return errors.New("token required") + } } else { - bytes, err = readLine(reader) - } - if err != nil { - return - } - fmt.Println() - if opts.Password = string(bytes); opts.Password == "" { - return fmt.Errorf("%s required", prompt) + // prompt for password + if opts.Password, err = readLine("Password: ", true); err != nil { + return err + } else if opts.Password == "" { + return errors.New("password required") + } } } @@ -137,15 +131,23 @@ func runLogin(ctx context.Context, opts loginOptions) (err error) { return nil } -func readLine(reader *bufio.Reader) (content []byte, err error) { +func readLine(prompt string, silent bool) (string, error) { + fmt.Print(prompt) + fd := int(os.Stdin.Fd()) var line []byte - for more := true; more; { - // read until no more - line, more, err = reader.ReadLine() - if err != nil { - return nil, err - } - content = append(content, line...) + var err error + if silent && term.IsTerminal(fd) { + line, err = term.ReadPassword(fd) + } else { + reader := bufio.NewReader(os.Stdin) + line, _, err = reader.ReadLine() } - return + if err != nil { + return "", err + } + if silent { + fmt.Println() + } + + return string(line), nil } From 000e251072e4827761d03e277e914c3e24b0262f Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 29 Mar 2023 04:53:31 +0000 Subject: [PATCH 05/13] read until no more Signed-off-by: Billy Zha --- cmd/oras/root/login.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/cmd/oras/root/login.go b/cmd/oras/root/login.go index 625bca984..64ac8d9a3 100644 --- a/cmd/oras/root/login.go +++ b/cmd/oras/root/login.go @@ -138,12 +138,19 @@ func readLine(prompt string, silent bool) (string, error) { var err error if silent && term.IsTerminal(fd) { line, err = term.ReadPassword(fd) + if err != nil { + return "", err + } } else { reader := bufio.NewReader(os.Stdin) - line, _, err = reader.ReadLine() - } - if err != nil { - return "", err + for more := true; more; { + // read until no more + line, more, err = reader.ReadLine() + if err != nil { + return "", err + } + line = append(line, line...) + } } if silent { fmt.Println() From 2fc7c4156d0a6e2f23a01634890c32a60f9853a1 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 29 Mar 2023 05:02:34 +0000 Subject: [PATCH 06/13] fix bug when reading line Signed-off-by: Billy Zha --- cmd/oras/root/login.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/oras/root/login.go b/cmd/oras/root/login.go index 64ac8d9a3..cf08c80a0 100644 --- a/cmd/oras/root/login.go +++ b/cmd/oras/root/login.go @@ -143,18 +143,18 @@ func readLine(prompt string, silent bool) (string, error) { } } else { reader := bufio.NewReader(os.Stdin) + var part []byte for more := true; more; { // read until no more - line, more, err = reader.ReadLine() + part, more, err = reader.ReadLine() if err != nil { return "", err } - line = append(line, line...) + line = append(line, part...) } } if silent { fmt.Println() } - return string(line), nil } From d5b541bd356fc5d06f6f794e24bb70155d106389 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 29 Mar 2023 07:36:18 +0000 Subject: [PATCH 07/13] implement with self-implemented scanf Signed-off-by: Billy Zha --- cmd/oras/root/login.go | 29 ++++++++++++++++++----------- test/e2e/suite/auth/auth.go | 9 +++++---- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/cmd/oras/root/login.go b/cmd/oras/root/login.go index cf08c80a0..1e413b278 100644 --- a/cmd/oras/root/login.go +++ b/cmd/oras/root/login.go @@ -16,10 +16,10 @@ limitations under the License. package root import ( - "bufio" "context" "errors" "fmt" + "io" "os" "strings" @@ -134,27 +134,34 @@ func runLogin(ctx context.Context, opts loginOptions) (err error) { func readLine(prompt string, silent bool) (string, error) { fmt.Print(prompt) fd := int(os.Stdin.Fd()) - var line []byte - var err error + line := "" if silent && term.IsTerminal(fd) { - line, err = term.ReadPassword(fd) + bytes, err := term.ReadPassword(fd) if err != nil { return "", err } + line = string(bytes) } else { - reader := bufio.NewReader(os.Stdin) - var part []byte - for more := true; more; { - // read until no more - part, more, err = reader.ReadLine() + // implement per-byte scanf here since fmt.Fscanln skips all the + // newline before scanning + for b := [1]byte{}; ; { + fmt.Print(len(b)) + _, err := os.Stdin.Read(b[:]) if err != nil { + if err == io.EOF { + break + } return "", err } - line = append(line, part...) + s := string(b[:]) + if s == "\n" { + break + } + line += s } } if silent { fmt.Println() } - return string(line), nil + return line, nil } diff --git a/test/e2e/suite/auth/auth.go b/test/e2e/suite/auth/auth.go index af2b91e9c..c15f9c4f1 100644 --- a/test/e2e/suite/auth/auth.go +++ b/test/e2e/suite/auth/auth.go @@ -69,14 +69,14 @@ var _ = Describe("Common registry user", Ordered, func() { It("should fail if no password input", func() { ORAS("login", Host, "--registry-config", AuthConfigPath). WithTimeOut(20*time.Second). - MatchKeyWords("username: ", "password: "). + MatchKeyWords("Username: ", "Password: "). WithInput(strings.NewReader(fmt.Sprintf("%s\n", Username))).ExpectFailure().Exec() }) It("should fail if password is empty", func() { ORAS("login", Host, "--registry-config", AuthConfigPath). WithTimeOut(20*time.Second). - MatchKeyWords("username: ", "password: "). + MatchKeyWords("Username: ", "Password: "). MatchErrKeyWords("Error: password required"). WithInput(strings.NewReader(fmt.Sprintf("%s\n\n", Username))).ExpectFailure().Exec() }) @@ -84,17 +84,18 @@ var _ = Describe("Common registry user", Ordered, func() { It("should fail if no token input", func() { ORAS("login", Host, "--registry-config", AuthConfigPath). WithTimeOut(20*time.Second). - MatchKeyWords("username: ", "token: "). + MatchKeyWords("Username: ", "Token: "). WithInput(strings.NewReader("\n")).ExpectFailure().Exec() }) It("should fail if token is empty", func() { ORAS("login", Host, "--registry-config", AuthConfigPath). WithTimeOut(20*time.Second). - MatchKeyWords("username: ", "token: "). + MatchKeyWords("Username: ", "Token: "). MatchErrKeyWords("Error: token required"). WithInput(strings.NewReader("\n\n")).ExpectFailure().Exec() }) + It("should use prompted input", func() { ORAS("login", Host, "--registry-config", AuthConfigPath). WithTimeOut(20*time.Second). From f51fb649b27ed6d5d201ca5a3fdd36cf1fe2c62f Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 30 Mar 2023 03:37:06 +0000 Subject: [PATCH 08/13] bug fix Signed-off-by: Billy Zha --- cmd/oras/root/login.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/oras/root/login.go b/cmd/oras/root/login.go index 1e413b278..0dfac6036 100644 --- a/cmd/oras/root/login.go +++ b/cmd/oras/root/login.go @@ -145,7 +145,6 @@ func readLine(prompt string, silent bool) (string, error) { // implement per-byte scanf here since fmt.Fscanln skips all the // newline before scanning for b := [1]byte{}; ; { - fmt.Print(len(b)) _, err := os.Stdin.Read(b[:]) if err != nil { if err == io.EOF { @@ -154,7 +153,7 @@ func readLine(prompt string, silent bool) (string, error) { return "", err } s := string(b[:]) - if s == "\n" { + if s == "\r" || s == "\n" { break } line += s From d51d1f7f265c8a48b993eb0254283821926793a2 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 30 Mar 2023 08:35:59 +0000 Subject: [PATCH 09/13] refactor and add tests Signed-off-by: Billy Zha --- cmd/oras/root/blob/push.go | 3 +- cmd/oras/root/login.go | 35 +++----- cmd/oras/root/manifest/push.go | 6 +- internal/{file/file.go => io/io.go} | 28 ++++++- internal/{file/file_test.go => io/io_test.go} | 82 +++++++++++++++---- 5 files changed, 106 insertions(+), 48 deletions(-) rename internal/{file/file.go => io/io.go} (88%) rename internal/{file/file_test.go => io/io_test.go} (78%) diff --git a/cmd/oras/root/blob/push.go b/cmd/oras/root/blob/push.go index ed5d409bc..c09e3271b 100644 --- a/cmd/oras/root/blob/push.go +++ b/cmd/oras/root/blob/push.go @@ -25,7 +25,6 @@ import ( "github.com/spf13/cobra" "oras.land/oras/cmd/oras/internal/display" "oras.land/oras/cmd/oras/internal/option" - "oras.land/oras/internal/file" ) type pushBlobOptions struct { @@ -104,7 +103,7 @@ func pushBlob(ctx context.Context, opts pushBlobOptions) (err error) { } // prepare blob content - desc, rc, err := file.PrepareBlobContent(opts.fileRef, opts.mediaType, opts.Reference, opts.size) + desc, rc, err := io.PrepareBlobContent(opts.fileRef, opts.mediaType, opts.Reference, opts.size) if err != nil { return err } diff --git a/cmd/oras/root/login.go b/cmd/oras/root/login.go index 0dfac6036..519aac768 100644 --- a/cmd/oras/root/login.go +++ b/cmd/oras/root/login.go @@ -19,7 +19,6 @@ import ( "context" "errors" "fmt" - "io" "os" "strings" @@ -27,6 +26,7 @@ import ( "golang.org/x/term" "oras.land/oras/cmd/oras/internal/option" "oras.land/oras/internal/credential" + "oras.land/oras/internal/io" ) type loginOptions struct { @@ -134,33 +134,20 @@ func runLogin(ctx context.Context, opts loginOptions) (err error) { func readLine(prompt string, silent bool) (string, error) { fmt.Print(prompt) fd := int(os.Stdin.Fd()) - line := "" + var bytes []byte + var err error if silent && term.IsTerminal(fd) { - bytes, err := term.ReadPassword(fd) - if err != nil { - return "", err - } - line = string(bytes) + bytes, err = term.ReadPassword(fd) + fmt.Println(string(bytes)) } else { - // implement per-byte scanf here since fmt.Fscanln skips all the - // newline before scanning - for b := [1]byte{}; ; { - _, err := os.Stdin.Read(b[:]) - if err != nil { - if err == io.EOF { - break - } - return "", err - } - s := string(b[:]) - if s == "\r" || s == "\n" { - break - } - line += s - } + bytes, err = io.ReadLine(os.Stdin) + fmt.Println(string(bytes)) + } + if err != nil { + return "", err } if silent { fmt.Println() } - return line, nil + return string(bytes), nil } diff --git a/cmd/oras/root/manifest/push.go b/cmd/oras/root/manifest/push.go index e7a86ef02..ce40b7ae6 100644 --- a/cmd/oras/root/manifest/push.go +++ b/cmd/oras/root/manifest/push.go @@ -30,7 +30,7 @@ import ( "oras.land/oras-go/v2/registry/remote" "oras.land/oras/cmd/oras/internal/display" "oras.land/oras/cmd/oras/internal/option" - "oras.land/oras/internal/file" + "oras.land/oras/internal/io" ) type pushOptions struct { @@ -116,7 +116,7 @@ func pushManifest(ctx context.Context, opts pushOptions) error { } // prepare manifest content - contentBytes, err := file.PrepareManifestContent(opts.fileRef) + contentBytes, err := io.PrepareManifestContent(opts.fileRef) if err != nil { return err } @@ -124,7 +124,7 @@ func pushManifest(ctx context.Context, opts pushOptions) error { // get manifest media type mediaType := opts.mediaType if opts.mediaType == "" { - mediaType, err = file.ParseMediaType(contentBytes) + mediaType, err = io.ParseMediaType(contentBytes) if err != nil { return err } diff --git a/internal/file/file.go b/internal/io/io.go similarity index 88% rename from internal/file/file.go rename to internal/io/io.go index e1e9cb37e..bcdcd4b01 100644 --- a/internal/file/file.go +++ b/internal/io/io.go @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package file +package io import ( "encoding/json" @@ -131,3 +131,29 @@ func ParseMediaType(content []byte) (string, error) { } return manifest.MediaType, nil } + +// ReadLine reads a line from the reader with trailing \r dropped. +func ReadLine(reader io.Reader) ([]byte, error) { + line := make([]byte, 0) + drop := 0 + for b := [1]byte{}; ; { + _, err := reader.Read(b[:]) + if err != nil { + if err == io.EOF { + drop = 0 + break + } + return nil, err + } + s := string(b[:]) + if s == "\r" { + drop = 1 + } else if s == "\n" { + break + } else { + drop = 0 + } + line = append(line, b[0]) + } + return line[:len(line)-drop], nil +} diff --git a/internal/file/file_test.go b/internal/io/io_test.go similarity index 78% rename from internal/file/file_test.go rename to internal/io/io_test.go index 42697fa3e..3e0c15a12 100644 --- a/internal/file/file_test.go +++ b/internal/io/io_test.go @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package file_test +package io_test import ( "errors" @@ -27,7 +27,7 @@ import ( digest "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras/internal/file" + iotest "oras.land/oras/internal/io" ) const manifestMediaType = "application/vnd.oci.image.manifest.v1+json" @@ -47,7 +47,7 @@ func TestFile_PrepareManifestContent(t *testing.T) { want := []byte(manifest) // test PrepareManifestContent - got, err := file.PrepareManifestContent(path) + got, err := iotest.PrepareManifestContent(path) if err != nil { t.Fatal("PrepareManifestContent() error=", err) } @@ -81,7 +81,7 @@ func TestFile_PrepareManifestContent_fromStdin(t *testing.T) { want := []byte(manifest) // test PrepareManifestContent read from stdin - got, err := file.PrepareManifestContent("-") + got, err := iotest.PrepareManifestContent("-") if err != nil { t.Fatal("PrepareManifestContent() error=", err) } @@ -92,7 +92,7 @@ func TestFile_PrepareManifestContent_fromStdin(t *testing.T) { func TestFile_PrepareManifestContent_errMissingFileName(t *testing.T) { // test PrepareManifestContent with missing file name - _, err := file.PrepareManifestContent("") + _, err := iotest.PrepareManifestContent("") expected := "missing file name" if err.Error() != expected { t.Fatalf("PrepareManifestContent() error = %v, wantErr %v", err, expected) @@ -101,7 +101,7 @@ func TestFile_PrepareManifestContent_errMissingFileName(t *testing.T) { func TestFile_PrepareManifestContent_errReadFile(t *testing.T) { // test PrepareManifestContent with nonexistent file - _, err := file.PrepareManifestContent("nonexistent.txt") + _, err := iotest.PrepareManifestContent("nonexistent.txt") expected := "failed to read nonexistent.txt" if !strings.Contains(err.Error(), expected) { t.Fatalf("PrepareManifestContent() error = %v, wantErr %v", err, expected) @@ -125,7 +125,7 @@ func TestFile_PrepareBlobContent(t *testing.T) { } // test PrepareBlobContent - got, rc, err := file.PrepareBlobContent(path, blobMediaType, "", -1) + got, rc, err := iotest.PrepareBlobContent(path, blobMediaType, "", -1) if err != nil { t.Fatal("PrepareBlobContent() error=", err) } @@ -147,7 +147,7 @@ func TestFile_PrepareBlobContent(t *testing.T) { // test PrepareBlobContent with provided digest and size dgstStr := "sha256:9a201d228ebd966211f7d1131be19f152be428bd373a92071c71d8deaf83b3e5" size := int64(12) - got, rc, err = file.PrepareBlobContent(path, blobMediaType, dgstStr, size) + got, rc, err = iotest.PrepareBlobContent(path, blobMediaType, dgstStr, size) if err != nil { t.Fatal("PrepareBlobContent() error=", err) } @@ -173,7 +173,7 @@ func TestFile_PrepareBlobContent(t *testing.T) { // test PrepareBlobContent with provided size, but the size does not match the // actual content size - _, _, err = file.PrepareBlobContent(path, blobMediaType, "", 15) + _, _, err = iotest.PrepareBlobContent(path, blobMediaType, "", 15) expected := fmt.Sprintf("input size %d does not match the actual content size %d", 15, size) if err.Error() != expected { t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) @@ -211,7 +211,7 @@ func TestFile_PrepareBlobContent_fromStdin(t *testing.T) { } // test PrepareBlobContent with provided digest and size - gotDesc, gotRc, err := file.PrepareBlobContent("-", blobMediaType, string(dgst), size) + gotDesc, gotRc, err := iotest.PrepareBlobContent("-", blobMediaType, string(dgst), size) defer gotRc.Close() if err != nil { t.Fatal("PrepareBlobContent() error=", err) @@ -227,14 +227,14 @@ func TestFile_PrepareBlobContent_fromStdin(t *testing.T) { } // test PrepareBlobContent from stdin with missing size - _, _, err = file.PrepareBlobContent("-", blobMediaType, "", -1) + _, _, err = iotest.PrepareBlobContent("-", blobMediaType, "", -1) expected := "content size must be provided if it is read from stdin" if err.Error() != expected { t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) } // test PrepareBlobContent from stdin with missing digest - _, _, err = file.PrepareBlobContent("-", blobMediaType, "", 5) + _, _, err = iotest.PrepareBlobContent("-", blobMediaType, "", 5) expected = "content digest must be provided if it is read from stdin" if err.Error() != expected { t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) @@ -244,7 +244,7 @@ func TestFile_PrepareBlobContent_fromStdin(t *testing.T) { func TestFile_PrepareBlobContent_errDigestInvalidFormat(t *testing.T) { // test PrepareBlobContent from stdin with invalid digest invalidDgst := "xyz" - _, _, err := file.PrepareBlobContent("-", blobMediaType, invalidDgst, 12) + _, _, err := iotest.PrepareBlobContent("-", blobMediaType, invalidDgst, 12) if !errors.Is(err, digest.ErrDigestInvalidFormat) { t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, digest.ErrDigestInvalidFormat) } @@ -252,7 +252,7 @@ func TestFile_PrepareBlobContent_errDigestInvalidFormat(t *testing.T) { func TestFile_PrepareBlobContent_errMissingFileName(t *testing.T) { // test PrepareBlobContent with missing file name - _, _, err := file.PrepareBlobContent("", blobMediaType, "", -1) + _, _, err := iotest.PrepareBlobContent("", blobMediaType, "", -1) expected := "missing file name" if err.Error() != expected { t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) @@ -261,7 +261,7 @@ func TestFile_PrepareBlobContent_errMissingFileName(t *testing.T) { func TestFile_PrepareBlobContent_errOpenFile(t *testing.T) { // test PrepareBlobContent with nonexistent file - _, _, err := file.PrepareBlobContent("nonexistent.txt", blobMediaType, "", -1) + _, _, err := iotest.PrepareBlobContent("nonexistent.txt", blobMediaType, "", -1) expected := "failed to open nonexistent.txt" if !strings.Contains(err.Error(), expected) { t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) @@ -274,7 +274,7 @@ func TestFile_ParseMediaType(t *testing.T) { // test ParseMediaType want := manifestMediaType - got, err := file.ParseMediaType(content) + got, err := iotest.ParseMediaType(content) if err != nil { t.Fatal("ParseMediaType() error=", err) } @@ -288,7 +288,7 @@ func TestFile_ParseMediaType_invalidContent_notAJson(t *testing.T) { content := []byte("manifest") // test ParseMediaType - _, err := file.ParseMediaType(content) + _, err := iotest.ParseMediaType(content) expected := "not a valid json file" if err.Error() != expected { t.Fatalf("ParseMediaType() error = %v, wantErr %v", err, expected) @@ -300,9 +300,55 @@ func TestFile_ParseMediaType_invalidContent_missingMediaType(t *testing.T) { content := []byte(`{"schemaVersion":2}`) // test ParseMediaType - _, err := file.ParseMediaType(content) + _, err := iotest.ParseMediaType(content) expected := "media type is not recognized" if err.Error() != expected { t.Fatalf("ParseMediaType() error = %v, wantErr %v", err, expected) } } + +func TestReadLine(t *testing.T) { + type args struct { + reader io.Reader + } + tests := []struct { + name string + args args + want []byte + wantErr bool + }{ + {"empty line", args{strings.NewReader("")}, []byte(""), false}, + {"LF", args{strings.NewReader("\n")}, []byte(""), false}, + {"CR", args{strings.NewReader("\r")}, []byte("\r"), false}, + {"CRLF", args{strings.NewReader("\r\n")}, []byte(""), false}, + {"input", args{strings.NewReader("foo")}, []byte("foo"), false}, + {"input ended with LF", args{strings.NewReader("foo\n")}, []byte("foo"), false}, + {"input ended with CR", args{strings.NewReader("foo\r")}, []byte("foo\r"), false}, + {"input ended with CRLF", args{strings.NewReader("foo\r\n")}, []byte("foo"), false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := iotest.ReadLine(tt.args.reader) + if (err != nil) != tt.wantErr { + t.Errorf("ReadLine() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ReadLine() = %v, want %v", got, tt.want) + } + }) + } +} + +type mockReader struct{} + +func (m *mockReader) Read(p []byte) (n int, err error) { + return 0, errors.New("mock error") +} + +func TestReadLine_err(t *testing.T) { + got, err := iotest.ReadLine(&mockReader{}) + if err == nil { + t.Errorf("ReadLine() = %v, want error", got) + } +} From 88f88e33c63d8256ac66c5de9dbe17c328399026 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 30 Mar 2023 15:19:22 +0000 Subject: [PATCH 10/13] resolve comments and add test Signed-off-by: Billy Zha --- cmd/oras/root/login.go | 9 +++------ internal/io/io.go | 29 +++++++++++++++++------------ internal/io/io_test.go | 38 ++++++++++++++++++++++++-------------- 3 files changed, 44 insertions(+), 32 deletions(-) diff --git a/cmd/oras/root/login.go b/cmd/oras/root/login.go index 519aac768..e015ca19a 100644 --- a/cmd/oras/root/login.go +++ b/cmd/oras/root/login.go @@ -137,17 +137,14 @@ func readLine(prompt string, silent bool) (string, error) { var bytes []byte var err error if silent && term.IsTerminal(fd) { - bytes, err = term.ReadPassword(fd) - fmt.Println(string(bytes)) + if bytes, err = term.ReadPassword(fd); err == nil { + _, err = fmt.Println() + } } else { bytes, err = io.ReadLine(os.Stdin) - fmt.Println(string(bytes)) } if err != nil { return "", err } - if silent { - fmt.Println() - } return string(bytes), nil } diff --git a/internal/io/io.go b/internal/io/io.go index bcdcd4b01..de36a67e1 100644 --- a/internal/io/io.go +++ b/internal/io/io.go @@ -134,26 +134,31 @@ func ParseMediaType(content []byte) (string, error) { // ReadLine reads a line from the reader with trailing \r dropped. func ReadLine(reader io.Reader) ([]byte, error) { - line := make([]byte, 0) + var line []byte + var buffer [1]byte drop := 0 - for b := [1]byte{}; ; { - _, err := reader.Read(b[:]) + for { + n, err := reader.Read(buffer[:]) if err != nil { if err == io.EOF { - drop = 0 - break + // a line ends with EOF or \r + return line[:len(line)-drop], nil } return nil, err } - s := string(b[:]) - if s == "\r" { + if n != 1 { + return nil, errors.New("failed to read with 1-byte buffer") + } + switch c := buffer[0]; c { + case '\r': drop = 1 - } else if s == "\n" { - break - } else { + line = append(line, c) + case '\n': + // a line ends with \n + return line[:len(line)-drop], nil + default: drop = 0 + line = append(line, c) } - line = append(line, b[0]) } - return line[:len(line)-drop], nil } diff --git a/internal/io/io_test.go b/internal/io/io_test.go index 3e0c15a12..bbe29dcd2 100644 --- a/internal/io/io_test.go +++ b/internal/io/io_test.go @@ -312,27 +312,37 @@ func TestReadLine(t *testing.T) { reader io.Reader } tests := []struct { - name string - args args - want []byte - wantErr bool + name string + args args + want []byte }{ - {"empty line", args{strings.NewReader("")}, []byte(""), false}, - {"LF", args{strings.NewReader("\n")}, []byte(""), false}, - {"CR", args{strings.NewReader("\r")}, []byte("\r"), false}, - {"CRLF", args{strings.NewReader("\r\n")}, []byte(""), false}, - {"input", args{strings.NewReader("foo")}, []byte("foo"), false}, - {"input ended with LF", args{strings.NewReader("foo\n")}, []byte("foo"), false}, - {"input ended with CR", args{strings.NewReader("foo\r")}, []byte("foo\r"), false}, - {"input ended with CRLF", args{strings.NewReader("foo\r\n")}, []byte("foo"), false}, + {"empty line", args{strings.NewReader("")}, nil}, + {"LF", args{strings.NewReader("\n")}, nil}, + {"CR", args{strings.NewReader("\r")}, []byte("")}, + {"CRLF", args{strings.NewReader("\r\n")}, []byte("")}, + {"input", args{strings.NewReader("foo")}, []byte("foo")}, + {"input ended with LF", args{strings.NewReader("foo\n")}, []byte("foo")}, + {"input ended with CR", args{strings.NewReader("foo\r")}, []byte("foo")}, + {"input ended with CRLF", args{strings.NewReader("foo\r\n")}, []byte("foo")}, + {"input contains CR", args{strings.NewReader("foo\rbar")}, []byte("foo\rbar")}, + {"input contains LF", args{strings.NewReader("foo\nbar")}, []byte("foo")}, + {"input contains CRLF", args{strings.NewReader("foo\r\nbar")}, []byte("foo")}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := iotest.ReadLine(tt.args.reader) - if (err != nil) != tt.wantErr { - t.Errorf("ReadLine() error = %v, wantErr %v", err, tt.wantErr) + if err != nil { + t.Errorf("ReadLine() error = %v", err) return } + if left, err := io.ReadAll(tt.args.reader); err != nil { + if err != io.EOF { + t.Errorf("Unexpected error in reading left: %v", err) + } + if len(left) != 0 || strings.ContainsAny(string(left), "\r\n") { + t.Errorf("Unexpected character left in the reader: %q", left) + } + } if !reflect.DeepEqual(got, tt.want) { t.Errorf("ReadLine() = %v, want %v", got, tt.want) } From 0612150a279880b58c377c8cb7cc04eac18ab7ca Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 30 Mar 2023 15:22:24 +0000 Subject: [PATCH 11/13] add comments Signed-off-by: Billy Zha --- internal/io/io.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/io/io.go b/internal/io/io.go index de36a67e1..6c056b413 100644 --- a/internal/io/io.go +++ b/internal/io/io.go @@ -141,7 +141,8 @@ func ReadLine(reader io.Reader) ([]byte, error) { n, err := reader.Read(buffer[:]) if err != nil { if err == io.EOF { - // a line ends with EOF or \r + // a line ends if reader is closed + // drop \r if it is the last character return line[:len(line)-drop], nil } return nil, err From c7426ade28921be3c579aa3a1ff06fb83bc93637 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 10 Apr 2023 07:03:24 +0000 Subject: [PATCH 12/13] change package name Signed-off-by: Billy Zha --- cmd/oras/root/blob/push.go | 3 +- cmd/oras/root/manifest/push.go | 6 +- internal/file/file.go | 133 ++++++++++++++ internal/file/file_test.go | 308 +++++++++++++++++++++++++++++++++ internal/io/io.go | 112 ------------ internal/io/io_test.go | 282 ------------------------------ 6 files changed, 446 insertions(+), 398 deletions(-) create mode 100644 internal/file/file.go create mode 100644 internal/file/file_test.go diff --git a/cmd/oras/root/blob/push.go b/cmd/oras/root/blob/push.go index c09e3271b..ed5d409bc 100644 --- a/cmd/oras/root/blob/push.go +++ b/cmd/oras/root/blob/push.go @@ -25,6 +25,7 @@ import ( "github.com/spf13/cobra" "oras.land/oras/cmd/oras/internal/display" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/internal/file" ) type pushBlobOptions struct { @@ -103,7 +104,7 @@ func pushBlob(ctx context.Context, opts pushBlobOptions) (err error) { } // prepare blob content - desc, rc, err := io.PrepareBlobContent(opts.fileRef, opts.mediaType, opts.Reference, opts.size) + desc, rc, err := file.PrepareBlobContent(opts.fileRef, opts.mediaType, opts.Reference, opts.size) if err != nil { return err } diff --git a/cmd/oras/root/manifest/push.go b/cmd/oras/root/manifest/push.go index ce40b7ae6..e7a86ef02 100644 --- a/cmd/oras/root/manifest/push.go +++ b/cmd/oras/root/manifest/push.go @@ -30,7 +30,7 @@ import ( "oras.land/oras-go/v2/registry/remote" "oras.land/oras/cmd/oras/internal/display" "oras.land/oras/cmd/oras/internal/option" - "oras.land/oras/internal/io" + "oras.land/oras/internal/file" ) type pushOptions struct { @@ -116,7 +116,7 @@ func pushManifest(ctx context.Context, opts pushOptions) error { } // prepare manifest content - contentBytes, err := io.PrepareManifestContent(opts.fileRef) + contentBytes, err := file.PrepareManifestContent(opts.fileRef) if err != nil { return err } @@ -124,7 +124,7 @@ func pushManifest(ctx context.Context, opts pushOptions) error { // get manifest media type mediaType := opts.mediaType if opts.mediaType == "" { - mediaType, err = io.ParseMediaType(contentBytes) + mediaType, err = file.ParseMediaType(contentBytes) if err != nil { return err } diff --git a/internal/file/file.go b/internal/file/file.go new file mode 100644 index 000000000..e1e9cb37e --- /dev/null +++ b/internal/file/file.go @@ -0,0 +1,133 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package file + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "os" + + digest "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" +) + +// PrepareManifestContent prepares the content for manifest from the file path +// or stdin. +func PrepareManifestContent(path string) ([]byte, error) { + if path == "" { + return nil, fmt.Errorf("missing file name") + } + + var content []byte + var err error + if path == "-" { + content, err = io.ReadAll(os.Stdin) + } else { + content, err = os.ReadFile(path) + } + if err != nil { + return nil, fmt.Errorf("failed to read %s: %w", path, err) + } + + return content, nil +} + +// PrepareBlobContent prepares the content descriptor for blob from the file +// path or stdin. Use the input digest and size if they are provided. Will +// return error if the content is from stdin but the content digest and size +// are missing. +func PrepareBlobContent(path string, mediaType string, dgstStr string, size int64) (desc ocispec.Descriptor, rc io.ReadCloser, prepareErr error) { + if path == "" { + return ocispec.Descriptor{}, nil, errors.New("missing file name") + } + + // validate digest + var dgst digest.Digest + if dgstStr != "" { + var err error + dgst, err = digest.Parse(dgstStr) + if err != nil { + return ocispec.Descriptor{}, nil, fmt.Errorf("invalid digest %s: %w", dgstStr, err) + } + } + + // prepares the content descriptor from stdin + if path == "-" { + // throw err if size or digest is not provided. + if size < 0 { + return ocispec.Descriptor{}, nil, errors.New("content size must be provided if it is read from stdin") + } + if dgst == "" { + return ocispec.Descriptor{}, nil, errors.New("content digest must be provided if it is read from stdin") + } + return ocispec.Descriptor{ + MediaType: mediaType, + Digest: dgst, + Size: size, + }, os.Stdin, nil + } + + file, err := os.Open(path) + if err != nil { + return ocispec.Descriptor{}, nil, fmt.Errorf("failed to open %s: %w", path, err) + } + defer func() { + if prepareErr != nil { + file.Close() + } + }() + + fi, err := file.Stat() + if err != nil { + return ocispec.Descriptor{}, nil, fmt.Errorf("failed to stat %s: %w", path, err) + } + actualSize := fi.Size() + if size >= 0 && size != actualSize { + return ocispec.Descriptor{}, nil, fmt.Errorf("input size %d does not match the actual content size %d", size, actualSize) + } + + if dgst == "" { + dgst, err = digest.FromReader(file) + if err != nil { + return ocispec.Descriptor{}, nil, err + } + if _, err = file.Seek(0, io.SeekStart); err != nil { + return ocispec.Descriptor{}, nil, err + } + } + + return ocispec.Descriptor{ + MediaType: mediaType, + Digest: dgst, + Size: actualSize, + }, file, nil +} + +// ParseMediaType parses the media type field of bytes content in json format. +func ParseMediaType(content []byte) (string, error) { + var manifest struct { + MediaType string `json:"mediaType"` + } + if err := json.Unmarshal(content, &manifest); err != nil { + return "", errors.New("not a valid json file") + } + if manifest.MediaType == "" { + return "", errors.New("media type is not recognized") + } + return manifest.MediaType, nil +} diff --git a/internal/file/file_test.go b/internal/file/file_test.go new file mode 100644 index 000000000..42697fa3e --- /dev/null +++ b/internal/file/file_test.go @@ -0,0 +1,308 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package file_test + +import ( + "errors" + "fmt" + "io" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + digest "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras/internal/file" +) + +const manifestMediaType = "application/vnd.oci.image.manifest.v1+json" +const blobMediaType = "application/mock-octet-stream" +const manifest = `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json","config":{"mediaType":"application/vnd.unknown.config.v1+json","digest":"sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a","size":2},"layers":[{"mediaType":"application/vnd.oci.image.layer.v1.tar","digest":"sha256:5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03","size":6,"annotations":{"org.opencontainers.image.title":"hello.txt"}}]}` + +func TestFile_PrepareManifestContent(t *testing.T) { + // generate test content + tempDir := t.TempDir() + content := []byte(manifest) + fileName := "manifest.json" + path := filepath.Join(tempDir, fileName) + if err := os.WriteFile(path, content, 0444); err != nil { + t.Fatal("error calling WriteFile(), error =", err) + } + + want := []byte(manifest) + + // test PrepareManifestContent + got, err := file.PrepareManifestContent(path) + if err != nil { + t.Fatal("PrepareManifestContent() error=", err) + } + if !reflect.DeepEqual(got, want) { + t.Errorf("PrepareManifestContent() = %v, want %v", got, want) + } +} + +func TestFile_PrepareManifestContent_fromStdin(t *testing.T) { + // generate test content + content := []byte(manifest) + tempDir := t.TempDir() + fileName := "manifest.json" + path := filepath.Join(tempDir, fileName) + tmpfile, err := os.Create(path) + if err != nil { + t.Fatal("error calling os.Create(), error =", err) + } + defer tmpfile.Close() + + if _, err := tmpfile.Write(content); err != nil { + t.Fatal("error calling Write(), error =", err) + } + if _, err := tmpfile.Seek(0, 0); err != nil { + t.Fatal("error calling Seek(), error =", err) + } + + os.Stdin = tmpfile + defer func(stdin *os.File) { os.Stdin = stdin }(os.Stdin) + + want := []byte(manifest) + + // test PrepareManifestContent read from stdin + got, err := file.PrepareManifestContent("-") + if err != nil { + t.Fatal("PrepareManifestContent() error=", err) + } + if !reflect.DeepEqual(got, want) { + t.Errorf("PrepareManifestContent() = %v, want %v", got, want) + } +} + +func TestFile_PrepareManifestContent_errMissingFileName(t *testing.T) { + // test PrepareManifestContent with missing file name + _, err := file.PrepareManifestContent("") + expected := "missing file name" + if err.Error() != expected { + t.Fatalf("PrepareManifestContent() error = %v, wantErr %v", err, expected) + } +} + +func TestFile_PrepareManifestContent_errReadFile(t *testing.T) { + // test PrepareManifestContent with nonexistent file + _, err := file.PrepareManifestContent("nonexistent.txt") + expected := "failed to read nonexistent.txt" + if !strings.Contains(err.Error(), expected) { + t.Fatalf("PrepareManifestContent() error = %v, wantErr %v", err, expected) + } +} + +func TestFile_PrepareBlobContent(t *testing.T) { + // generate test content + tempDir := t.TempDir() + content := []byte("hello world!") + fileName := "test.txt" + path := filepath.Join(tempDir, fileName) + if err := os.WriteFile(path, content, 0444); err != nil { + t.Fatal("error calling WriteFile(), error =", err) + } + + want := ocispec.Descriptor{ + MediaType: blobMediaType, + Digest: digest.FromBytes(content), + Size: int64(len(content)), + } + + // test PrepareBlobContent + got, rc, err := file.PrepareBlobContent(path, blobMediaType, "", -1) + if err != nil { + t.Fatal("PrepareBlobContent() error=", err) + } + if !reflect.DeepEqual(got, want) { + t.Errorf("PrepareBlobContent() = %v, want %v", got, want) + } + actualContent, err := io.ReadAll(rc) + if err != nil { + t.Fatal("PrepareBlobContent(): not able to read content from rc, error=", err) + } + err = rc.Close() + if err != nil { + t.Fatal("error calling rc.Close(), error =", err) + } + if !reflect.DeepEqual(actualContent, content) { + t.Errorf("PrepareBlobContent() = %v, want %v", actualContent, content) + } + + // test PrepareBlobContent with provided digest and size + dgstStr := "sha256:9a201d228ebd966211f7d1131be19f152be428bd373a92071c71d8deaf83b3e5" + size := int64(12) + got, rc, err = file.PrepareBlobContent(path, blobMediaType, dgstStr, size) + if err != nil { + t.Fatal("PrepareBlobContent() error=", err) + } + want = ocispec.Descriptor{ + MediaType: blobMediaType, + Digest: digest.Digest(dgstStr), + Size: size, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("PrepareBlobContent() = %v, want %v", got, want) + } + actualContent, err = io.ReadAll(rc) + if err != nil { + t.Fatal("PrepareBlobContent(): not able to read content from rc, error=", err) + } + err = rc.Close() + if err != nil { + t.Fatal("error calling rc.Close(), error =", err) + } + if !reflect.DeepEqual(actualContent, content) { + t.Errorf("PrepareBlobContent() = %v, want %v", actualContent, content) + } + + // test PrepareBlobContent with provided size, but the size does not match the + // actual content size + _, _, err = file.PrepareBlobContent(path, blobMediaType, "", 15) + expected := fmt.Sprintf("input size %d does not match the actual content size %d", 15, size) + if err.Error() != expected { + t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) + } +} + +func TestFile_PrepareBlobContent_fromStdin(t *testing.T) { + // generate test content + content := []byte("hello world!") + tempDir := t.TempDir() + fileName := "test.txt" + path := filepath.Join(tempDir, fileName) + tmpfile, err := os.Create(path) + if err != nil { + t.Fatal("error calling os.Create(), error =", err) + } + defer tmpfile.Close() + + if _, err := tmpfile.Write(content); err != nil { + t.Fatal("error calling Write(), error =", err) + } + if _, err := tmpfile.Seek(0, 0); err != nil { + t.Fatal("error calling Seek(), error =", err) + } + + defer func(stdin *os.File) { os.Stdin = stdin }(os.Stdin) + + os.Stdin = tmpfile + dgst := digest.FromBytes(content) + size := int64(len(content)) + wantDesc := ocispec.Descriptor{ + MediaType: blobMediaType, + Digest: digest.FromBytes(content), + Size: int64(len(content)), + } + + // test PrepareBlobContent with provided digest and size + gotDesc, gotRc, err := file.PrepareBlobContent("-", blobMediaType, string(dgst), size) + defer gotRc.Close() + if err != nil { + t.Fatal("PrepareBlobContent() error=", err) + } + if !reflect.DeepEqual(gotDesc, wantDesc) { + t.Errorf("PrepareBlobContent() = %v, want %v", gotDesc, wantDesc) + } + if _, err = tmpfile.Seek(0, io.SeekStart); err != nil { + t.Fatal("error calling Seek(), error =", err) + } + if !reflect.DeepEqual(gotRc, tmpfile) { + t.Errorf("PrepareBlobContent() = %v, want %v", gotRc, tmpfile) + } + + // test PrepareBlobContent from stdin with missing size + _, _, err = file.PrepareBlobContent("-", blobMediaType, "", -1) + expected := "content size must be provided if it is read from stdin" + if err.Error() != expected { + t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) + } + + // test PrepareBlobContent from stdin with missing digest + _, _, err = file.PrepareBlobContent("-", blobMediaType, "", 5) + expected = "content digest must be provided if it is read from stdin" + if err.Error() != expected { + t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) + } +} + +func TestFile_PrepareBlobContent_errDigestInvalidFormat(t *testing.T) { + // test PrepareBlobContent from stdin with invalid digest + invalidDgst := "xyz" + _, _, err := file.PrepareBlobContent("-", blobMediaType, invalidDgst, 12) + if !errors.Is(err, digest.ErrDigestInvalidFormat) { + t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, digest.ErrDigestInvalidFormat) + } +} + +func TestFile_PrepareBlobContent_errMissingFileName(t *testing.T) { + // test PrepareBlobContent with missing file name + _, _, err := file.PrepareBlobContent("", blobMediaType, "", -1) + expected := "missing file name" + if err.Error() != expected { + t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) + } +} + +func TestFile_PrepareBlobContent_errOpenFile(t *testing.T) { + // test PrepareBlobContent with nonexistent file + _, _, err := file.PrepareBlobContent("nonexistent.txt", blobMediaType, "", -1) + expected := "failed to open nonexistent.txt" + if !strings.Contains(err.Error(), expected) { + t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) + } +} + +func TestFile_ParseMediaType(t *testing.T) { + // generate test content + content := []byte(manifest) + + // test ParseMediaType + want := manifestMediaType + got, err := file.ParseMediaType(content) + if err != nil { + t.Fatal("ParseMediaType() error=", err) + } + if !reflect.DeepEqual(got, want) { + t.Errorf("ParseMediaType() = %v, want %v", got, want) + } +} + +func TestFile_ParseMediaType_invalidContent_notAJson(t *testing.T) { + // generate test content + content := []byte("manifest") + + // test ParseMediaType + _, err := file.ParseMediaType(content) + expected := "not a valid json file" + if err.Error() != expected { + t.Fatalf("ParseMediaType() error = %v, wantErr %v", err, expected) + } +} + +func TestFile_ParseMediaType_invalidContent_missingMediaType(t *testing.T) { + // generate test content + content := []byte(`{"schemaVersion":2}`) + + // test ParseMediaType + _, err := file.ParseMediaType(content) + expected := "media type is not recognized" + if err.Error() != expected { + t.Fatalf("ParseMediaType() error = %v, wantErr %v", err, expected) + } +} diff --git a/internal/io/io.go b/internal/io/io.go index 6c056b413..038e8a505 100644 --- a/internal/io/io.go +++ b/internal/io/io.go @@ -16,122 +16,10 @@ limitations under the License. package io import ( - "encoding/json" "errors" - "fmt" "io" - "os" - - digest "github.com/opencontainers/go-digest" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) -// PrepareManifestContent prepares the content for manifest from the file path -// or stdin. -func PrepareManifestContent(path string) ([]byte, error) { - if path == "" { - return nil, fmt.Errorf("missing file name") - } - - var content []byte - var err error - if path == "-" { - content, err = io.ReadAll(os.Stdin) - } else { - content, err = os.ReadFile(path) - } - if err != nil { - return nil, fmt.Errorf("failed to read %s: %w", path, err) - } - - return content, nil -} - -// PrepareBlobContent prepares the content descriptor for blob from the file -// path or stdin. Use the input digest and size if they are provided. Will -// return error if the content is from stdin but the content digest and size -// are missing. -func PrepareBlobContent(path string, mediaType string, dgstStr string, size int64) (desc ocispec.Descriptor, rc io.ReadCloser, prepareErr error) { - if path == "" { - return ocispec.Descriptor{}, nil, errors.New("missing file name") - } - - // validate digest - var dgst digest.Digest - if dgstStr != "" { - var err error - dgst, err = digest.Parse(dgstStr) - if err != nil { - return ocispec.Descriptor{}, nil, fmt.Errorf("invalid digest %s: %w", dgstStr, err) - } - } - - // prepares the content descriptor from stdin - if path == "-" { - // throw err if size or digest is not provided. - if size < 0 { - return ocispec.Descriptor{}, nil, errors.New("content size must be provided if it is read from stdin") - } - if dgst == "" { - return ocispec.Descriptor{}, nil, errors.New("content digest must be provided if it is read from stdin") - } - return ocispec.Descriptor{ - MediaType: mediaType, - Digest: dgst, - Size: size, - }, os.Stdin, nil - } - - file, err := os.Open(path) - if err != nil { - return ocispec.Descriptor{}, nil, fmt.Errorf("failed to open %s: %w", path, err) - } - defer func() { - if prepareErr != nil { - file.Close() - } - }() - - fi, err := file.Stat() - if err != nil { - return ocispec.Descriptor{}, nil, fmt.Errorf("failed to stat %s: %w", path, err) - } - actualSize := fi.Size() - if size >= 0 && size != actualSize { - return ocispec.Descriptor{}, nil, fmt.Errorf("input size %d does not match the actual content size %d", size, actualSize) - } - - if dgst == "" { - dgst, err = digest.FromReader(file) - if err != nil { - return ocispec.Descriptor{}, nil, err - } - if _, err = file.Seek(0, io.SeekStart); err != nil { - return ocispec.Descriptor{}, nil, err - } - } - - return ocispec.Descriptor{ - MediaType: mediaType, - Digest: dgst, - Size: actualSize, - }, file, nil -} - -// ParseMediaType parses the media type field of bytes content in json format. -func ParseMediaType(content []byte) (string, error) { - var manifest struct { - MediaType string `json:"mediaType"` - } - if err := json.Unmarshal(content, &manifest); err != nil { - return "", errors.New("not a valid json file") - } - if manifest.MediaType == "" { - return "", errors.New("media type is not recognized") - } - return manifest.MediaType, nil -} - // ReadLine reads a line from the reader with trailing \r dropped. func ReadLine(reader io.Reader) ([]byte, error) { var line []byte diff --git a/internal/io/io_test.go b/internal/io/io_test.go index bbe29dcd2..a2cdff448 100644 --- a/internal/io/io_test.go +++ b/internal/io/io_test.go @@ -17,296 +17,14 @@ package io_test import ( "errors" - "fmt" "io" - "os" - "path/filepath" "reflect" "strings" "testing" - digest "github.com/opencontainers/go-digest" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" iotest "oras.land/oras/internal/io" ) -const manifestMediaType = "application/vnd.oci.image.manifest.v1+json" -const blobMediaType = "application/mock-octet-stream" -const manifest = `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json","config":{"mediaType":"application/vnd.unknown.config.v1+json","digest":"sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a","size":2},"layers":[{"mediaType":"application/vnd.oci.image.layer.v1.tar","digest":"sha256:5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03","size":6,"annotations":{"org.opencontainers.image.title":"hello.txt"}}]}` - -func TestFile_PrepareManifestContent(t *testing.T) { - // generate test content - tempDir := t.TempDir() - content := []byte(manifest) - fileName := "manifest.json" - path := filepath.Join(tempDir, fileName) - if err := os.WriteFile(path, content, 0444); err != nil { - t.Fatal("error calling WriteFile(), error =", err) - } - - want := []byte(manifest) - - // test PrepareManifestContent - got, err := iotest.PrepareManifestContent(path) - if err != nil { - t.Fatal("PrepareManifestContent() error=", err) - } - if !reflect.DeepEqual(got, want) { - t.Errorf("PrepareManifestContent() = %v, want %v", got, want) - } -} - -func TestFile_PrepareManifestContent_fromStdin(t *testing.T) { - // generate test content - content := []byte(manifest) - tempDir := t.TempDir() - fileName := "manifest.json" - path := filepath.Join(tempDir, fileName) - tmpfile, err := os.Create(path) - if err != nil { - t.Fatal("error calling os.Create(), error =", err) - } - defer tmpfile.Close() - - if _, err := tmpfile.Write(content); err != nil { - t.Fatal("error calling Write(), error =", err) - } - if _, err := tmpfile.Seek(0, 0); err != nil { - t.Fatal("error calling Seek(), error =", err) - } - - os.Stdin = tmpfile - defer func(stdin *os.File) { os.Stdin = stdin }(os.Stdin) - - want := []byte(manifest) - - // test PrepareManifestContent read from stdin - got, err := iotest.PrepareManifestContent("-") - if err != nil { - t.Fatal("PrepareManifestContent() error=", err) - } - if !reflect.DeepEqual(got, want) { - t.Errorf("PrepareManifestContent() = %v, want %v", got, want) - } -} - -func TestFile_PrepareManifestContent_errMissingFileName(t *testing.T) { - // test PrepareManifestContent with missing file name - _, err := iotest.PrepareManifestContent("") - expected := "missing file name" - if err.Error() != expected { - t.Fatalf("PrepareManifestContent() error = %v, wantErr %v", err, expected) - } -} - -func TestFile_PrepareManifestContent_errReadFile(t *testing.T) { - // test PrepareManifestContent with nonexistent file - _, err := iotest.PrepareManifestContent("nonexistent.txt") - expected := "failed to read nonexistent.txt" - if !strings.Contains(err.Error(), expected) { - t.Fatalf("PrepareManifestContent() error = %v, wantErr %v", err, expected) - } -} - -func TestFile_PrepareBlobContent(t *testing.T) { - // generate test content - tempDir := t.TempDir() - content := []byte("hello world!") - fileName := "test.txt" - path := filepath.Join(tempDir, fileName) - if err := os.WriteFile(path, content, 0444); err != nil { - t.Fatal("error calling WriteFile(), error =", err) - } - - want := ocispec.Descriptor{ - MediaType: blobMediaType, - Digest: digest.FromBytes(content), - Size: int64(len(content)), - } - - // test PrepareBlobContent - got, rc, err := iotest.PrepareBlobContent(path, blobMediaType, "", -1) - if err != nil { - t.Fatal("PrepareBlobContent() error=", err) - } - if !reflect.DeepEqual(got, want) { - t.Errorf("PrepareBlobContent() = %v, want %v", got, want) - } - actualContent, err := io.ReadAll(rc) - if err != nil { - t.Fatal("PrepareBlobContent(): not able to read content from rc, error=", err) - } - err = rc.Close() - if err != nil { - t.Fatal("error calling rc.Close(), error =", err) - } - if !reflect.DeepEqual(actualContent, content) { - t.Errorf("PrepareBlobContent() = %v, want %v", actualContent, content) - } - - // test PrepareBlobContent with provided digest and size - dgstStr := "sha256:9a201d228ebd966211f7d1131be19f152be428bd373a92071c71d8deaf83b3e5" - size := int64(12) - got, rc, err = iotest.PrepareBlobContent(path, blobMediaType, dgstStr, size) - if err != nil { - t.Fatal("PrepareBlobContent() error=", err) - } - want = ocispec.Descriptor{ - MediaType: blobMediaType, - Digest: digest.Digest(dgstStr), - Size: size, - } - if !reflect.DeepEqual(got, want) { - t.Errorf("PrepareBlobContent() = %v, want %v", got, want) - } - actualContent, err = io.ReadAll(rc) - if err != nil { - t.Fatal("PrepareBlobContent(): not able to read content from rc, error=", err) - } - err = rc.Close() - if err != nil { - t.Fatal("error calling rc.Close(), error =", err) - } - if !reflect.DeepEqual(actualContent, content) { - t.Errorf("PrepareBlobContent() = %v, want %v", actualContent, content) - } - - // test PrepareBlobContent with provided size, but the size does not match the - // actual content size - _, _, err = iotest.PrepareBlobContent(path, blobMediaType, "", 15) - expected := fmt.Sprintf("input size %d does not match the actual content size %d", 15, size) - if err.Error() != expected { - t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) - } -} - -func TestFile_PrepareBlobContent_fromStdin(t *testing.T) { - // generate test content - content := []byte("hello world!") - tempDir := t.TempDir() - fileName := "test.txt" - path := filepath.Join(tempDir, fileName) - tmpfile, err := os.Create(path) - if err != nil { - t.Fatal("error calling os.Create(), error =", err) - } - defer tmpfile.Close() - - if _, err := tmpfile.Write(content); err != nil { - t.Fatal("error calling Write(), error =", err) - } - if _, err := tmpfile.Seek(0, 0); err != nil { - t.Fatal("error calling Seek(), error =", err) - } - - defer func(stdin *os.File) { os.Stdin = stdin }(os.Stdin) - - os.Stdin = tmpfile - dgst := digest.FromBytes(content) - size := int64(len(content)) - wantDesc := ocispec.Descriptor{ - MediaType: blobMediaType, - Digest: digest.FromBytes(content), - Size: int64(len(content)), - } - - // test PrepareBlobContent with provided digest and size - gotDesc, gotRc, err := iotest.PrepareBlobContent("-", blobMediaType, string(dgst), size) - defer gotRc.Close() - if err != nil { - t.Fatal("PrepareBlobContent() error=", err) - } - if !reflect.DeepEqual(gotDesc, wantDesc) { - t.Errorf("PrepareBlobContent() = %v, want %v", gotDesc, wantDesc) - } - if _, err = tmpfile.Seek(0, io.SeekStart); err != nil { - t.Fatal("error calling Seek(), error =", err) - } - if !reflect.DeepEqual(gotRc, tmpfile) { - t.Errorf("PrepareBlobContent() = %v, want %v", gotRc, tmpfile) - } - - // test PrepareBlobContent from stdin with missing size - _, _, err = iotest.PrepareBlobContent("-", blobMediaType, "", -1) - expected := "content size must be provided if it is read from stdin" - if err.Error() != expected { - t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) - } - - // test PrepareBlobContent from stdin with missing digest - _, _, err = iotest.PrepareBlobContent("-", blobMediaType, "", 5) - expected = "content digest must be provided if it is read from stdin" - if err.Error() != expected { - t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) - } -} - -func TestFile_PrepareBlobContent_errDigestInvalidFormat(t *testing.T) { - // test PrepareBlobContent from stdin with invalid digest - invalidDgst := "xyz" - _, _, err := iotest.PrepareBlobContent("-", blobMediaType, invalidDgst, 12) - if !errors.Is(err, digest.ErrDigestInvalidFormat) { - t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, digest.ErrDigestInvalidFormat) - } -} - -func TestFile_PrepareBlobContent_errMissingFileName(t *testing.T) { - // test PrepareBlobContent with missing file name - _, _, err := iotest.PrepareBlobContent("", blobMediaType, "", -1) - expected := "missing file name" - if err.Error() != expected { - t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) - } -} - -func TestFile_PrepareBlobContent_errOpenFile(t *testing.T) { - // test PrepareBlobContent with nonexistent file - _, _, err := iotest.PrepareBlobContent("nonexistent.txt", blobMediaType, "", -1) - expected := "failed to open nonexistent.txt" - if !strings.Contains(err.Error(), expected) { - t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected) - } -} - -func TestFile_ParseMediaType(t *testing.T) { - // generate test content - content := []byte(manifest) - - // test ParseMediaType - want := manifestMediaType - got, err := iotest.ParseMediaType(content) - if err != nil { - t.Fatal("ParseMediaType() error=", err) - } - if !reflect.DeepEqual(got, want) { - t.Errorf("ParseMediaType() = %v, want %v", got, want) - } -} - -func TestFile_ParseMediaType_invalidContent_notAJson(t *testing.T) { - // generate test content - content := []byte("manifest") - - // test ParseMediaType - _, err := iotest.ParseMediaType(content) - expected := "not a valid json file" - if err.Error() != expected { - t.Fatalf("ParseMediaType() error = %v, wantErr %v", err, expected) - } -} - -func TestFile_ParseMediaType_invalidContent_missingMediaType(t *testing.T) { - // generate test content - content := []byte(`{"schemaVersion":2}`) - - // test ParseMediaType - _, err := iotest.ParseMediaType(content) - expected := "media type is not recognized" - if err.Error() != expected { - t.Fatalf("ParseMediaType() error = %v, wantErr %v", err, expected) - } -} - func TestReadLine(t *testing.T) { type args struct { reader io.Reader From 3f93344527083305d2c412a9b9c8cfb2905e2e9c Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 10 Apr 2023 08:18:14 +0000 Subject: [PATCH 13/13] resolve comment Signed-off-by: Billy Zha --- internal/io/io.go | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/internal/io/io.go b/internal/io/io.go index 038e8a505..c09033c76 100644 --- a/internal/io/io.go +++ b/internal/io/io.go @@ -16,7 +16,7 @@ limitations under the License. package io import ( - "errors" + "bytes" "io" ) @@ -24,30 +24,22 @@ import ( func ReadLine(reader io.Reader) ([]byte, error) { var line []byte var buffer [1]byte - drop := 0 for { n, err := reader.Read(buffer[:]) if err != nil { if err == io.EOF { - // a line ends if reader is closed - // drop \r if it is the last character - return line[:len(line)-drop], nil + break } return nil, err } - if n != 1 { - return nil, errors.New("failed to read with 1-byte buffer") + if n == 0 { + continue } - switch c := buffer[0]; c { - case '\r': - drop = 1 - line = append(line, c) - case '\n': - // a line ends with \n - return line[:len(line)-drop], nil - default: - drop = 0 - line = append(line, c) + c := buffer[0] + if c == '\n' { + break } + line = append(line, c) } + return bytes.TrimSuffix(line, []byte{'\r'}), nil }