From 049515d2656a647c2d3a124a7bb560dbcc8b627e Mon Sep 17 00:00:00 2001 From: Osama Faqhruldin Date: Wed, 2 Aug 2023 10:53:30 -0400 Subject: [PATCH 1/4] Check for nil pointer in update rule --- github/repos_rules.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/github/repos_rules.go b/github/repos_rules.go index 4e6f5f13474..45ceaf987e1 100644 --- a/github/repos_rules.go +++ b/github/repos_rules.go @@ -119,14 +119,18 @@ func (r *RepositoryRule) UnmarshalJSON(data []byte) error { r.Parameters = nil case "update": params := UpdateAllowsFetchAndMergeRuleParameters{} - if err := json.Unmarshal(*RepositoryRule.Parameters, ¶ms); err != nil { - return err - } + if RepositoryRule.Parameters != nil { + if err := json.Unmarshal(*RepositoryRule.Parameters, ¶ms); err != nil { + return err + } - bytes, _ := json.Marshal(params) - rawParams := json.RawMessage(bytes) + bytes, _ := json.Marshal(params) + rawParams := json.RawMessage(bytes) - r.Parameters = &rawParams + r.Parameters = &rawParams + } else { + r.Parameters = nil + } case "required_deployments": params := RequiredDeploymentEnvironmentsRuleParameters{} if err := json.Unmarshal(*RepositoryRule.Parameters, ¶ms); err != nil { From a1e836a67814be6931fc571cef56beeed3b58bc8 Mon Sep 17 00:00:00 2001 From: Osama Faqhruldin Date: Wed, 2 Aug 2023 14:26:04 -0400 Subject: [PATCH 2/4] Add test and create update rule based on passed params --- github/repos_rules.go | 7 ++++--- github/repos_rules_test.go | 39 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/github/repos_rules.go b/github/repos_rules.go index 45ceaf987e1..076eeda7421 100644 --- a/github/repos_rules.go +++ b/github/repos_rules.go @@ -118,8 +118,11 @@ func (r *RepositoryRule) UnmarshalJSON(data []byte) error { case "creation", "deletion", "required_linear_history", "required_signatures", "non_fast_forward": r.Parameters = nil case "update": - params := UpdateAllowsFetchAndMergeRuleParameters{} if RepositoryRule.Parameters != nil { + r.Parameters = nil + return nil + } else { + params := UpdateAllowsFetchAndMergeRuleParameters{} if err := json.Unmarshal(*RepositoryRule.Parameters, ¶ms); err != nil { return err } @@ -128,8 +131,6 @@ func (r *RepositoryRule) UnmarshalJSON(data []byte) error { rawParams := json.RawMessage(bytes) r.Parameters = &rawParams - } else { - r.Parameters = nil } case "required_deployments": params := RequiredDeploymentEnvironmentsRuleParameters{} diff --git a/github/repos_rules_test.go b/github/repos_rules_test.go index 582ec89245b..a57137c688d 100644 --- a/github/repos_rules_test.go +++ b/github/repos_rules_test.go @@ -292,6 +292,45 @@ func TestRepositoriesService_GetRulesForBranch(t *testing.T) { }) } +func TestRepositoriesService_GetRulesForBranchEmptyUpdateRule(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/repo/rules/branches/branch", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + fmt.Fprint(w, `[ + { + "type": "update" + } + ]`) + }) + + ctx := context.Background() + rules, _, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch") + if err != nil { + t.Errorf("Repositories.GetRulesForBranch returned error: %v", err) + } + + updateRule := NewUpdateRule(nil) + + want := []*RepositoryRule{ + updateRule, + } + if !cmp.Equal(rules, want) { + t.Errorf("Repositories.GetRulesForBranch returned %+v, want %+v", Stringify(rules), Stringify(want)) + } + + const methodName = "GetRulesForBranch" + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch") + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + func TestRepositoriesService_GetAllRulesets(t *testing.T) { client, mux, _, teardown := setup() defer teardown() From e9cfeacdad7acd8b503b872eeca4a1268b176163 Mon Sep 17 00:00:00 2001 From: Osama Faqhruldin Date: Wed, 2 Aug 2023 14:27:03 -0400 Subject: [PATCH 3/4] Fix typo --- github/repos_rules.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/github/repos_rules.go b/github/repos_rules.go index 076eeda7421..3c154b73aa6 100644 --- a/github/repos_rules.go +++ b/github/repos_rules.go @@ -118,7 +118,7 @@ func (r *RepositoryRule) UnmarshalJSON(data []byte) error { case "creation", "deletion", "required_linear_history", "required_signatures", "non_fast_forward": r.Parameters = nil case "update": - if RepositoryRule.Parameters != nil { + if RepositoryRule.Parameters == nil { r.Parameters = nil return nil } else { @@ -190,13 +190,18 @@ func NewCreationRule() (rule *RepositoryRule) { // NewUpdateRule creates a rule to only allow users with bypass permission to update matching refs. func NewUpdateRule(params *UpdateAllowsFetchAndMergeRuleParameters) (rule *RepositoryRule) { - bytes, _ := json.Marshal(params) + if params != nil { + bytes, _ := json.Marshal(params) - rawParams := json.RawMessage(bytes) + rawParams := json.RawMessage(bytes) + return &RepositoryRule{ + Type: "update", + Parameters: &rawParams, + } + } return &RepositoryRule{ - Type: "update", - Parameters: &rawParams, + Type: "update", } } From 82387253cbd12dbb3924e9ef9aa52e098a61c9ca Mon Sep 17 00:00:00 2001 From: Osama Faqhruldin Date: Wed, 2 Aug 2023 20:48:13 -0400 Subject: [PATCH 4/4] Remove unneeded else statement --- github/repos_rules.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/github/repos_rules.go b/github/repos_rules.go index 3c154b73aa6..38d4255a9aa 100644 --- a/github/repos_rules.go +++ b/github/repos_rules.go @@ -121,17 +121,17 @@ func (r *RepositoryRule) UnmarshalJSON(data []byte) error { if RepositoryRule.Parameters == nil { r.Parameters = nil return nil - } else { - params := UpdateAllowsFetchAndMergeRuleParameters{} - if err := json.Unmarshal(*RepositoryRule.Parameters, ¶ms); err != nil { - return err - } + } + params := UpdateAllowsFetchAndMergeRuleParameters{} + if err := json.Unmarshal(*RepositoryRule.Parameters, ¶ms); err != nil { + return err + } - bytes, _ := json.Marshal(params) - rawParams := json.RawMessage(bytes) + bytes, _ := json.Marshal(params) + rawParams := json.RawMessage(bytes) + + r.Parameters = &rawParams - r.Parameters = &rawParams - } case "required_deployments": params := RequiredDeploymentEnvironmentsRuleParameters{} if err := json.Unmarshal(*RepositoryRule.Parameters, ¶ms); err != nil {