diff options
| -rw-r--r-- | config/config.go | 29 | ||||
| -rw-r--r-- | config/refspec.go | 22 | ||||
| -rw-r--r-- | config/refspec_test.go | 17 | ||||
| -rw-r--r-- | options.go | 11 | ||||
| -rw-r--r-- | remote_test.go | 8 |
5 files changed, 56 insertions, 31 deletions
diff --git a/config/config.go b/config/config.go index 15844be..866ae8e 100644 --- a/config/config.go +++ b/config/config.go @@ -1,4 +1,4 @@ -// Package config contains the abstraction of config files +// Package config contains the abstraction of multiple config files package config import ( @@ -89,8 +89,7 @@ func (c *Config) Unmarshal(b []byte) error { } c.unmarshalCore() - c.unmarshalRemotes() - return nil + return c.unmarshalRemotes() } func (c *Config) unmarshalCore() { @@ -100,14 +99,18 @@ func (c *Config) unmarshalCore() { } } -func (c *Config) unmarshalRemotes() { +func (c *Config) unmarshalRemotes() error { s := c.raw.Section(remoteSection) for _, sub := range s.Subsections { r := &RemoteConfig{} - r.unmarshal(sub) + if err := r.unmarshal(sub); err != nil { + return err + } c.Remotes[r.Name] = r } + + return nil } // Marshal returns Config encoded as a git-config file @@ -163,6 +166,12 @@ func (c *RemoteConfig) Validate() error { return ErrRemoteConfigEmptyURL } + for _, r := range c.Fetch { + if err := r.Validate(); err != nil { + return err + } + } + if len(c.Fetch) == 0 { c.Fetch = []RefSpec{RefSpec(fmt.Sprintf(DefaultFetchRefSpec, c.Name))} } @@ -170,20 +179,24 @@ func (c *RemoteConfig) Validate() error { return nil } -func (c *RemoteConfig) unmarshal(s *format.Subsection) { +func (c *RemoteConfig) unmarshal(s *format.Subsection) error { c.raw = s fetch := []RefSpec{} for _, f := range c.raw.Options.GetAll(fetchKey) { rs := RefSpec(f) - if rs.IsValid() { - fetch = append(fetch, rs) + if err := rs.Validate(); err != nil { + return err } + + fetch = append(fetch, rs) } c.Name = c.raw.Name c.URL = c.raw.Option(urlKey) c.Fetch = fetch + + return nil } func (c *RemoteConfig) marshal() *format.Subsection { diff --git a/config/refspec.go b/config/refspec.go index dca0d82..dd68edc 100644 --- a/config/refspec.go +++ b/config/refspec.go @@ -1,6 +1,7 @@ package config import ( + "errors" "strings" "srcd.works/go-git.v4/plumbing" @@ -12,6 +13,11 @@ const ( refSpecSeparator = ":" ) +var ( + ErrRefSpecMalformedSeparator = errors.New("malformed refspec, separators are wrong") + ErrRefSpecMalformedWildcard = errors.New("malformed refspec, missmatched number of wildcards") +) + // RefSpec is a mapping from local branches to remote references // The format of the refspec is an optional +, followed by <src>:<dst>, where // <src> is the pattern for references on the remote side and <dst> is where @@ -22,21 +28,25 @@ const ( // https://git-scm.com/book/es/v2/Git-Internals-The-Refspec type RefSpec string -// IsValid validates the RefSpec -func (s RefSpec) IsValid() bool { +// Validate validates the RefSpec +func (s RefSpec) Validate() error { spec := string(s) if strings.Count(spec, refSpecSeparator) != 1 { - return false + return ErrRefSpecMalformedSeparator } sep := strings.Index(spec, refSpecSeparator) - if sep == len(spec) { - return false + if sep == len(spec)-1 { + return ErrRefSpecMalformedSeparator } ws := strings.Count(spec[0:sep], refSpecWildcard) wd := strings.Count(spec[sep+1:], refSpecWildcard) - return ws == wd && ws < 2 && wd < 2 + if ws == wd && ws < 2 && wd < 2 { + return nil + } + + return ErrRefSpecMalformedWildcard } // IsForceUpdate returns if update is allowed in non fast-forward merges diff --git a/config/refspec_test.go b/config/refspec_test.go index 6f97422..6672cd2 100644 --- a/config/refspec_test.go +++ b/config/refspec_test.go @@ -15,25 +15,28 @@ func Test(t *testing.T) { TestingT(t) } func (s *RefSpecSuite) TestRefSpecIsValid(c *C) { spec := RefSpec("+refs/heads/*:refs/remotes/origin/*") - c.Assert(spec.IsValid(), Equals, true) + c.Assert(spec.Validate(), Equals, nil) spec = RefSpec("refs/heads/*:refs/remotes/origin/") - c.Assert(spec.IsValid(), Equals, false) + c.Assert(spec.Validate(), Equals, ErrRefSpecMalformedWildcard) spec = RefSpec("refs/heads/master:refs/remotes/origin/master") - c.Assert(spec.IsValid(), Equals, true) + c.Assert(spec.Validate(), Equals, nil) spec = RefSpec(":refs/heads/master") - c.Assert(spec.IsValid(), Equals, true) + c.Assert(spec.Validate(), Equals, nil) spec = RefSpec(":refs/heads/*") - c.Assert(spec.IsValid(), Equals, false) + c.Assert(spec.Validate(), Equals, ErrRefSpecMalformedWildcard) spec = RefSpec(":*") - c.Assert(spec.IsValid(), Equals, false) + c.Assert(spec.Validate(), Equals, ErrRefSpecMalformedWildcard) spec = RefSpec("refs/heads/*") - c.Assert(spec.IsValid(), Equals, false) + c.Assert(spec.Validate(), Equals, ErrRefSpecMalformedSeparator) + + spec = RefSpec("refs/heads:") + c.Assert(spec.Validate(), Equals, ErrRefSpecMalformedSeparator) } func (s *RefSpecSuite) TestRefSpecIsForceUpdate(c *C) { @@ -15,8 +15,7 @@ const ( ) var ( - ErrMissingURL = errors.New("URL field is required") - ErrInvalidRefSpec = errors.New("invalid refspec") + ErrMissingURL = errors.New("URL field is required") ) // CloneOptions describes how a clone should be performed @@ -110,8 +109,8 @@ func (o *FetchOptions) Validate() error { } for _, r := range o.RefSpecs { - if !r.IsValid() { - return ErrInvalidRefSpec + if err := r.Validate(); err != nil { + return err } } @@ -142,8 +141,8 @@ func (o *PushOptions) Validate() error { } for _, r := range o.RefSpecs { - if !r.IsValid() { - return ErrInvalidRefSpec + if err := r.Validate(); err != nil { + return err } } diff --git a/remote_test.go b/remote_test.go index 1e3a2d8..1e34905 100644 --- a/remote_test.go +++ b/remote_test.go @@ -7,8 +7,8 @@ import ( "io/ioutil" "os" - "srcd.works/go-git.v4/config" "github.com/src-d/go-git-fixtures" + "srcd.works/go-git.v4/config" "srcd.works/go-git.v4/plumbing" "srcd.works/go-git.v4/plumbing/storer" "srcd.works/go-git.v4/storage/filesystem" @@ -46,7 +46,7 @@ func (s *RemoteSuite) TestFetchInvalidFetchOptions(c *C) { r := newRemote(nil, &config.RemoteConfig{Name: "foo", URL: "qux://foo"}) invalid := config.RefSpec("^*$ñ") err := r.Fetch(&FetchOptions{RefSpecs: []config.RefSpec{invalid}}) - c.Assert(err, Equals, ErrInvalidRefSpec) + c.Assert(err, Equals, config.ErrRefSpecMalformedSeparator) } func (s *RemoteSuite) TestFetch(c *C) { @@ -287,7 +287,7 @@ func (s *RemoteSuite) TestPushInvalidFetchOptions(c *C) { r := newRemote(nil, &config.RemoteConfig{Name: "foo", URL: "qux://foo"}) invalid := config.RefSpec("^*$ñ") err := r.Push(&PushOptions{RefSpecs: []config.RefSpec{invalid}}) - c.Assert(err, Equals, ErrInvalidRefSpec) + c.Assert(err, Equals, config.ErrRefSpecMalformedSeparator) } func (s *RemoteSuite) TestPushInvalidRefSpec(c *C) { @@ -300,7 +300,7 @@ func (s *RemoteSuite) TestPushInvalidRefSpec(c *C) { err := r.Push(&PushOptions{ RefSpecs: []config.RefSpec{rs}, }) - c.Assert(err, ErrorMatches, ".*invalid.*") + c.Assert(err, Equals, config.ErrRefSpecMalformedSeparator) } func (s *RemoteSuite) TestPushWrongRemoteName(c *C) { |