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 nil pointer dereference when accessing AccountEvent event header account #84

Closed
wants to merge 2 commits into from

Conversation

weppos
Copy link
Member

@weppos weppos commented Jun 13, 2019

Closes #83

This is a tricky issue. The bug is caused by a naming conflicts in the JSON deserialization of an account even payload.

An AccountEvent is defined as:

type AccountEvent struct {
	EventHeader		AbstractEvent
	Data    *AccountEvent     `json:"data"`
	Account *dnsimple.Account `json:"account"`
}

This is an example of struct composition in Go. The EventHeader is defined as:

type EventHeader struct {
	APIVersion string   `json:"api_version"`
	RequestID  string   `json:"request_identifier"`
	Actor      *Actor   `json:"actor"`
	Account    *Account `json:"account"`
	Name       string   `json:"name"`
	Auto       bool     `json:"auto"`
	payload    []byte
}

The combined AccountEvent struct is:

type AccountEvent struct {
	APIVersion string   `json:"api_version"`
	RequestID  string   `json:"request_identifier"`
	Actor      *Actor   `json:"actor"`
	Account    *Account `json:"account"`
	Name       string   `json:"name"`
	Auto       bool     `json:"auto"`
	payload    []byte
	Data    *AccountEvent     `json:"data"`
	Account *dnsimple.Account `json:"account"`
}

When go tries to parse the payload, the two json:"account" definitions conflicts each other. The issue arises from the fact the AccountEven is a flatten representation of an Even payload that is actually a nested structure.

In Go parsing nested structures is tricky. We took the path to represents events as individual types.

This PR prefixes the event header fields with Event (so Actor becomes EventActor). The event header is parsed independently, and later assigned.

@weppos weppos force-pushed the bugfix/panic-account-webhook branch from 1735b8c to a10d009 Compare June 13, 2019 15:34
@weppos weppos self-assigned this Jun 13, 2019
@weppos weppos added the bug Code defect or incorrect behavior label Jun 13, 2019
@weppos
Copy link
Member Author

weppos commented Jun 17, 2019

Replaced by #85

@weppos weppos closed this Jun 17, 2019
@weppos weppos deleted the bugfix/panic-account-webhook branch April 28, 2020 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Code defect or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nil pointer dereference accessing account webhooks
1 participant