New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a bug that causes the parsing to fail even though there aren't any errors #56

Closed
wants to merge 1 commit into
base: v2
from

Conversation

Projects
None yet
2 participants
@jawher

jawher commented Nov 21, 2014

This bug is triggered when using a custom unmarsheller which calls the unmarshal callback multiple times with different types.

For example, suppose we want to parse a list of commands in a yaml file:

commands:
  - echo $PATH
  - who am i

And we'd also like to support this short format:

commands: "ls -al"

We should be able to handle this using a custom marshaller which tries to parse the value as a string first, and if that fails, retry the parse but as a string slice:

type OneOrMany []string

func (c * OneOrMany) UnmarshalYAML(unmarshal func(interface{}) error) error {
    single := ""
    if err := unmarshal(&single); err == nil {
        *c = []string{single}
        return nil
    }

    multiple := []string{}
    if err := unmarshal(&multiple); err != nil {
        return err
    }
    *c = multiple
    return nil
}

But the code above fails with the current version of yaml.v2.
Indeed, when a call to the unmarshal callback fails, the terrors field of decoders is no longer nil (it contains one error).
Afterwards, in decode.go line 230:

d.terrors = d.terrors[:terrlen]

the error is removed from the terrors slice, but terrors is now a zero-length slice and not nil.
So even though a later call to unmarshal succeeds, the whole parse would be marked as failed by the follozing test in yaml.go line 93:

if d.terrors != nil {

And we end up with an empty error.

Fix a bug that causes the parsing to fail even though there aren't an…
…y errors

This bug is triggered when using a custom unmarsheller which calls the unmarshal callback multiple times with different types.

For example, suppose we want to parse a list of commands in a yaml file:

```yaml
commands:
  - echo $PATH
  - who am i
```

And we'd also like to support this short format:

```yaml
commands: "ls -al"
```

We should be able to handle this using a custom marshaller which tries to parse the value as a string first, and if that fails, retry the parse but as a string slice:

```go
type OneOrMany []string

func (c * OneOrMany) UnmarshalYAML(unmarshal func(interface{}) error) error {
	single := ""
	if err := unmarshal(&single); err == nil {
		*c = []string{single}
		return nil
	}

	multiple := []string{}
	if err := unmarshal(&multiple); err != nil {
		return err
	}
	*c = multiple
	return nil
}
```

But the code above fails with the current version of yaml.v2.
Indeed, when a call to the unmarshal callback fails, the terrors field of decoders is no longer nil (it contains one error).
Afterwards, in decode.go line 230:

```go
d.terrors = d.terrors[:terrlen]
```

the error is removed from the terrors slice, but terrors is now a zero-length slice and not nil.
So even though a later call to unmarshal succeeds, the whole parse would be marked as failed by the follozing test in yaml.go line 93:

```go
if d.terrors != nil {
```

And we end up with an empty error.

@julienvey julienvey referenced this pull request Nov 21, 2014

Closed

Improve Yaml support #8

@niemeyer

This comment has been minimized.

Show comment
Hide comment
@niemeyer

niemeyer Jan 16, 2015

Contributor

Thanks, but this had been fixed on #52 already. I was just slow there, sorry.

Contributor

niemeyer commented Jan 16, 2015

Thanks, but this had been fixed on #52 already. I was just slow there, sorry.

@niemeyer niemeyer closed this Jan 16, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment