From 4ee8f7a62a390103abef6bb9b610eef49fa20532 Mon Sep 17 00:00:00 2001 From: Tmwakalasya Date: Thu, 29 Oct 2026 16:43:15 -0400 Subject: [PATCH] fix(changeprovider): reject null Phabricator diffs --- .../changeprovider/phabricator/conduit.go | 2 +- .../phabricator/provider_test.go | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/submitqueue/extension/changeprovider/phabricator/conduit.go b/submitqueue/extension/changeprovider/phabricator/conduit.go index 6b10cf1a7..0cc06a05e 100644 --- a/submitqueue/extension/changeprovider/phabricator/conduit.go +++ b/submitqueue/extension/changeprovider/phabricator/conduit.go @@ -93,7 +93,7 @@ func parseConduitResponse(resp *http.Response, diffIDs []int) (map[int]*diffResu results := make(map[int]*diffResult, len(diffIDs)) for _, id := range diffIDs { diff, ok := rawMap[strconv.Itoa(id)] - if !ok { + if !ok || diff == nil { return nil, fmt.Errorf("diff %d not found in querydiffs response", id) } results[id] = diff diff --git a/submitqueue/extension/changeprovider/phabricator/provider_test.go b/submitqueue/extension/changeprovider/phabricator/provider_test.go index eebafe207..2ee6432c5 100644 --- a/submitqueue/extension/changeprovider/phabricator/provider_test.go +++ b/submitqueue/extension/changeprovider/phabricator/provider_test.go @@ -100,6 +100,29 @@ func TestProvider_Get(t *testing.T) { } } +func TestProvider_Get_UnavailableDiff(t *testing.T) { + for _, tt := range []struct { + name string + result map[string]*diffResult + }{ + {name: "missing diff", result: validDiffResponse()}, + {name: "null diff", result: map[string]*diffResult{"100": validDiffResponse()["100"], "101": nil}}, + } { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + serveConduit(t, w, tt.result) + })) + t.Cleanup(server.Close) + p := newTestProvider(t, &http.Client{Transport: &testTransport{baseURL: server.URL}}) + infos, err := p.Get(t.Context(), entity.Request{Change: change.Change{ + URIs: []string{"phab://phab.example.com/D200/100", "phab://phab.example.com/D201/101"}, + }}) + require.Error(t, err) + assert.Nil(t, infos) + }) + } +} + func TestProvider_Get_MultipleDiffs(t *testing.T) { callCount := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {