From e1b402856418d11137fb60ec25c8b90342b7b01f Mon Sep 17 00:00:00 2001 From: hgaol Date: Tue, 9 Jun 2026 15:26:11 +0000 Subject: [PATCH] fix: accept answer fails when short links enabled (#1541) The ownership check added to AcceptAnswer compared the answer's QuestionID against the request's QuestionID directly. When short links are enabled, answerRepo.GetByID re-encodes QuestionID to its short form while the controller de-shorts req.QuestionID to its long form, so the two encodings of the same question never matched and every accept returned "Answer do not found". Normalize both ids via uid.DeShortID before comparing, preserving the privilege-escalation guard for answers that truly belong to another question. --- internal/service/content/answer_service.go | 8 +- .../service/content/answer_service_test.go | 81 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 internal/service/content/answer_service_test.go diff --git a/internal/service/content/answer_service.go b/internal/service/content/answer_service.go index d7506d6a8..25b0050ea 100644 --- a/internal/service/content/answer_service.go +++ b/internal/service/content/answer_service.go @@ -471,7 +471,7 @@ func (as *AnswerService) AcceptAnswer(ctx context.Context, req *schema.AcceptAns } // check answer belong to question - if acceptedAnswerInfo.QuestionID != req.QuestionID { + if !sameObjectID(acceptedAnswerInfo.QuestionID, req.QuestionID) { return errors.BadRequest(reason.AnswerNotFound) } acceptedAnswerInfo.ID = uid.DeShortID(acceptedAnswerInfo.ID) @@ -513,6 +513,12 @@ func (as *AnswerService) AcceptAnswer(ctx context.Context, req *schema.AcceptAns return nil } +// sameObjectID reports whether two object ids refer to the same row, +// regardless of whether each is in short-id or long-id form. +func sameObjectID(a, b string) bool { + return uid.DeShortID(a) == uid.DeShortID(b) +} + func (as *AnswerService) updateAnswerRank(ctx context.Context, userID string, questionInfo *entity.Question, newAnswerInfo *entity.Answer, oldAnswerInfo *entity.Answer, ) { diff --git a/internal/service/content/answer_service_test.go b/internal/service/content/answer_service_test.go new file mode 100644 index 000000000..3cfcb1056 --- /dev/null +++ b/internal/service/content/answer_service_test.go @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package content + +import ( + "testing" + + "github.com/apache/answer/pkg/uid" +) + +// TestSameObjectID guards the AcceptAnswer ownership check (issue #1541). +// +// When short links are enabled, answerRepo.GetByID re-encodes the answer's +// QuestionID to its short form while the controller de-shorts req.QuestionID +// to its long form. The two encodings of the same question must be treated as +// equal, or accepting any answer fails with "Answer do not found". +func TestSameObjectID(t *testing.T) { + const longQID = "10010000000000001" + shortQID := uid.EnShortID(longQID) // e.g. "D1D1" + if shortQID == "" || shortQID == longQID { + t.Fatalf("precondition failed: EnShortID(%q)=%q, want a distinct short id", longQID, shortQID) + } + otherLongQID := "10010000000000002" + + tests := []struct { + name string + a string + b string + want bool + }{ + { + name: "short answer-side id vs long request-side id, same question (the bug)", + a: shortQID, + b: longQID, + want: true, + }, + { + name: "both long, same question (default permalink)", + a: longQID, + b: longQID, + want: true, + }, + { + name: "both short, same question", + a: shortQID, + b: shortQID, + want: true, + }, + { + name: "different questions must stay rejected (privilege-escalation guard)", + a: shortQID, + b: otherLongQID, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := sameObjectID(tt.a, tt.b); got != tt.want { + t.Errorf("sameObjectID(%q, %q) = %v, want %v", tt.a, tt.b, got, tt.want) + } + }) + } +}