diff --git a/pkg/stereopair/marge_cleanup.go b/pkg/stereopair/marge_cleanup.go index fba4c034..3e31f993 100644 --- a/pkg/stereopair/marge_cleanup.go +++ b/pkg/stereopair/marge_cleanup.go @@ -1,6 +1,7 @@ package stereopair import ( + "bytes" "encoding/xml" "errors" "fmt" @@ -64,25 +65,67 @@ func DeleteMargeGroupGeneration(httpClient *http.Client, ref GenerationRef) erro return fmt.Errorf("close Marge generation cleanup response: %w", closeErr) } - group, verifyErr := getMargeDeviceGroup(httpClient, ref) - if verifyErr != nil { - return fmt.Errorf("verify Marge group generation deletion: %w", verifyErr) - } - - if group.IsEmpty() || group.ID != ref.GroupID { - return nil - } - - return fmt.Errorf("delete Marge group generation: generation %s is still active", ref.GroupID) + return verifyMargeGroupGenerationDeleted(httpClient, ref) } - body, _ := io.ReadAll(io.LimitReader(response.Body, 1024)) - _ = response.Body.Close() + body, readErr := io.ReadAll(io.LimitReader(response.Body, 1025)) + closeErr := response.Body.Close() + if readErr != nil { + return fmt.Errorf("read Marge generation cleanup response: %w", readErr) + } + if closeErr != nil { + return fmt.Errorf("close Marge generation cleanup response: %w", closeErr) + } + if len(body) > 1024 { + return fmt.Errorf("delete Marge group generation: HTTP %d response exceeds 1024 bytes", response.StatusCode) + } + if response.StatusCode == http.StatusInternalServerError && margeWrappedGroupNotFound(body, ref) { + return verifyMargeGroupGenerationDeleted(httpClient, ref) + } return fmt.Errorf("delete Marge group generation: HTTP %d: %s", response.StatusCode, strings.TrimSpace(string(body))) } +func verifyMargeGroupGenerationDeleted(httpClient *http.Client, ref GenerationRef) error { + group, err := getMargeDeviceGroup(httpClient, ref) + if err != nil { + return fmt.Errorf("verify Marge group generation deletion: %w", err) + } + + if group.IsEmpty() || group.ID != ref.GroupID { + return nil + } + + return fmt.Errorf("delete Marge group generation: generation %s is still active", ref.GroupID) +} + +func margeWrappedGroupNotFound(body []byte, ref GenerationRef) bool { + var response struct { + XMLName xml.Name + Message string `xml:",chardata"` + } + decoder := xml.NewDecoder(bytes.NewReader(body)) + if err := decoder.Decode(&response); err != nil || response.XMLName.Local != "error" { + return false + } + for { + token, err := decoder.Token() + if err == io.EOF { + break + } + if err != nil { + return false + } + if data, ok := token.(xml.CharData); !ok || strings.TrimSpace(string(data)) != "" { + return false + } + } + + want := fmt.Sprintf("Unexpected error: 404: Group %s does not exist in account %s", ref.GroupID, ref.AccountID) + return strings.TrimSpace(response.Message) == want +} + // RenameMargeGroupGeneration updates and verifies the name of one persisted // generation at the backend configured by fresh speaker info. Topology, rather // than the old name, is the retry guard so a degraded rename can converge. diff --git a/pkg/stereopair/marge_cleanup_test.go b/pkg/stereopair/marge_cleanup_test.go index 9730d710..9f9770e2 100644 --- a/pkg/stereopair/marge_cleanup_test.go +++ b/pkg/stereopair/marge_cleanup_test.go @@ -2,6 +2,8 @@ package stereopair import ( "encoding/xml" + "errors" + "io" "net/http" "net/http/httptest" "strings" @@ -176,6 +178,138 @@ func TestDeleteMargeGroupGenerationAcceptsVerifiedAbsenceAfter404(t *testing.T) } } +func TestDeleteMargeGroupGenerationAcceptsVerifiedAbsenceAfterWrapped404(t *testing.T) { + deleteSeen := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deleteSeen = true + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`Unexpected error: 404: Group PAIR1 does not exist in account ACCOUNT1`)) + return + } + + if deleteSeen { + _, _ = w.Write([]byte(``)) + return + } + + _, _ = w.Write([]byte(`LEFT-IDLEFT-IDLEFT192.0.2.10RIGHT-IDRIGHT192.0.2.11`)) + })) + defer server.Close() + + err := DeleteMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }) + if err != nil { + t.Fatalf("DeleteMargeGroupGeneration: %v", err) + } +} + +func TestDeleteMargeGroupGenerationRejectsWrapped404WhileGenerationRemains(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`Unexpected error: 404: Group PAIR1 does not exist in account ACCOUNT1`)) + return + } + + _, _ = w.Write([]byte(`LEFT-IDLEFT-IDLEFT192.0.2.10RIGHT-IDRIGHT192.0.2.11`)) + })) + defer server.Close() + + err := DeleteMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }) + if err == nil || !strings.Contains(err.Error(), "still active") { + t.Fatalf("error = %v, want failed postcondition", err) + } +} + +func TestMargeWrappedGroupNotFoundRequiresExactError(t *testing.T) { + ref := GenerationRef{AccountID: "ACCOUNT1", GroupID: "PAIR1"} + for _, test := range []struct { + name string + body string + want bool + }{ + {name: "exact", body: `Unexpected error: 404: Group PAIR1 does not exist in account ACCOUNT1`, want: true}, + {name: "xml declaration and whitespace", body: xml.Header + ` Unexpected error: 404: Group PAIR1 does not exist in account ACCOUNT1 `, want: true}, + {name: "wrong group", body: `Unexpected error: 404: Group OTHER does not exist in account ACCOUNT1`}, + {name: "wrong account", body: `Unexpected error: 404: Group PAIR1 does not exist in account OTHER`}, + {name: "wrong inner status", body: `Unexpected error: 500: Group PAIR1 does not exist in account ACCOUNT1`}, + {name: "wrong root", body: `Unexpected error: 404: Group PAIR1 does not exist in account ACCOUNT1`}, + {name: "trailing element", body: `Unexpected error: 404: Group PAIR1 does not exist in account ACCOUNT1`}, + {name: "malformed", body: `Unexpected error: 404: Group PAIR1 does not exist in account ACCOUNT1`}, + } { + t.Run(test.name, func(t *testing.T) { + if got := margeWrappedGroupNotFound([]byte(test.body), ref); got != test.want { + t.Fatalf("margeWrappedGroupNotFound() = %t, want %t", got, test.want) + } + }) + } +} + +type failingReadCloser struct{} + +func (failingReadCloser) Read([]byte) (int, error) { return 0, errors.New("read failed") } +func (failingReadCloser) Close() error { return nil } + +type cleanupRoundTripper struct { + responses []*http.Response +} + +func (r *cleanupRoundTripper) RoundTrip(*http.Request) (*http.Response, error) { + response := r.responses[0] + r.responses = r.responses[1:] + return response, nil +} + +func TestDeleteMargeGroupGenerationRejectsUnreadableWrappedResponse(t *testing.T) { + initial := `LEFT-IDLEFT-IDLEFT192.0.2.10RIGHT-IDRIGHT192.0.2.11` + httpClient := &http.Client{Transport: &cleanupRoundTripper{responses: []*http.Response{ + {StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(initial))}, + {StatusCode: http.StatusInternalServerError, Body: failingReadCloser{}}, + }}} + + err := DeleteMargeGroupGeneration(httpClient, GenerationRef{ + MargeURL: "http://example.test", AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }) + if err == nil || !strings.Contains(err.Error(), "read failed") { + t.Fatalf("error = %v, want read failure", err) + } +} + +func TestDeleteMargeGroupGenerationRejectsOversizedWrappedResponse(t *testing.T) { + deleteSeen := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deleteSeen = true + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`Unexpected error: 404: Group PAIR1 does not exist in account ACCOUNT1` + strings.Repeat(" ", 1024))) + return + } + + if deleteSeen { + _, _ = w.Write([]byte(``)) + return + } + + _, _ = w.Write([]byte(`LEFT-IDLEFT-IDLEFT192.0.2.10RIGHT-IDRIGHT192.0.2.11`)) + })) + defer server.Close() + + err := DeleteMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }) + if err == nil || !strings.Contains(err.Error(), "exceeds 1024 bytes") { + t.Fatalf("error = %v, want oversized response rejection", err) + } +} + func TestDeleteMargeGroupGenerationRejectsUnrelatedGenerationBeforeDelete(t *testing.T) { deleteCalls := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {