Skip to content
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 marshalling of URL wrapper with nil value. #303

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/http_config.go
Expand Up @@ -130,7 +130,7 @@ func (u URL) MarshalJSON() ([]byte, error) {
if u.URL != nil {
return json.Marshal(u.URL.String())
}
return nil, nil
return []byte("null"), nil
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[uber-nit] This might seem like a crazy idea, but we could save hard-coding the null by marshaling nil:

Suggested change
return []byte("null"), nil
return json.Marshal(nil)

}

// OAuth2 is the oauth2 client configuration.
Expand Down
77 changes: 77 additions & 0 deletions config/http_config_test.go
Expand Up @@ -1302,6 +1302,83 @@ func TestMarshalURL(t *testing.T) {
}
}

func TestMarshalURLWrapperWithNilValue(t *testing.T) {
u := &URL{}

c, err := json.Marshal(u)
if err != nil {
t.Fatal(err)
}
if string(c) != "null" {
t.Fatalf("URL with nil value not properly marshaled into JSON, got %q", c)
}

c, err = yaml.Marshal(u)
if err != nil {
t.Fatal(err)
}
if string(c) != "null\n" {
t.Fatalf("URL with nil value not properly marshaled into JSON, got %q", c)
}
}

func TestUnmarshalNullURL(t *testing.T) {
b := []byte(`null`)

{
var u URL
err := json.Unmarshal(b, &u)
if err != nil {
t.Fatal(err)
}
if !isEmptyNonNilURL(u.URL) {
t.Fatalf("`null` literal not properly unmarshaled from JSON as URL, got %#v", u.URL)
}
}

{
var u URL
err := yaml.Unmarshal(b, &u)
if err != nil {
t.Fatal(err)
}
if u.URL != nil { // UnmarshalYAML is not called when parsing null literal.
t.Fatalf("`null` literal not properly unmarshaled from YAML as URL, got %#v", u.URL)
}
}
}

func TestUnmarshalEmptyURL(t *testing.T) {
b := []byte(`""`)

{
var u URL
err := json.Unmarshal(b, &u)
if err != nil {
t.Fatal(err)
}
if !isEmptyNonNilURL(u.URL) {
t.Fatalf("empty string not properly unmarshaled from JSON as URL, got %#v", u.URL)
}
}

{
var u URL
err := yaml.Unmarshal(b, &u)
if err != nil {
t.Fatal(err)
}
if !isEmptyNonNilURL(u.URL) {
t.Fatalf("empty string not properly unmarshaled from YAML as URL, got %#v", u.URL)
}
}
}

// checks if u equals to &url.URL{}
func isEmptyNonNilURL(u *url.URL) bool {
return u != nil && *u == url.URL{}
}

func TestUnmarshalURL(t *testing.T) {
b := []byte(`"http://example.com/a b"`)
var u URL
Expand Down