fix(issues): address review findings for custom field search
Generic: Repo Health / Site Health (push) Has been cancelled
Generic: Repo Health / Access control (push) Has been cancelled
Generic: Repo Health / Site Health (pull_request) Has been cancelled
Universal: PR Check / Branch Policy (pull_request) Has been cancelled
Branch Policy Check / Verify merge target (pull_request) Has been cancelled
Generic: Repo Health / Access control (pull_request) Has been cancelled
Universal: PR Check / Validate PR (pull_request) Has been cancelled
PR RC Release / Build RC Release (pull_request) Has been cancelled
Branch Cleanup / Delete merged branch (pull_request) Has been cancelled
Universal: Build & Release / Promote to RC (pull_request) Has been cancelled
Universal: Build & Release / Build & Release Pipeline (pull_request) Has been cancelled
Generic: Repo Health / Scripts governance (push) Has been cancelled
Generic: Repo Health / Repository health (push) Has been cancelled
Generic: Repo Health / Report Issues (push) Has been cancelled
Generic: Repo Health / Scripts governance (pull_request) Has been cancelled
Generic: Repo Health / Repository health (pull_request) Has been cancelled
Generic: Repo Health / Report Issues (pull_request) Has been cancelled
Universal: PR Check / Build RC Package (pull_request) Has been cancelled
Universal: PR Check / Report Issues (pull_request) Has been cancelled

- Clear custom field filters when field definitions fail to load,
  preventing invisible filtering with no UI to undo
- Log malformed options JSON instead of silently dropping fields
- Extract parseCustomFieldQueryParams helper (web) and
  parseAPICustomFieldFilters helper (API) to deduplicate cf_ parsing
- API returns 400 for non-numeric or non-positive cf_ field IDs
- Validate field IDs are positive in web handler
- Add entity_type='issue' to subquery to prevent cross-entity matching

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jonathan Miller
2026-06-05 17:06:18 -05:00
parent a3c6f54ad3
commit 33076e4e1b
3 changed files with 46 additions and 30 deletions
+1 -1
View File
@@ -220,7 +220,7 @@ func applyCustomFieldCondition(sess *xorm.Session, opts *IssuesOptions) {
// custom_field_value row for every specified field (AND semantics).
for fieldID, value := range opts.CustomFieldFilters {
subQuery := builder.Select("entity_id").From("custom_field_value").Where(
builder.Eq{"field_id": fieldID, "value": value},
builder.Eq{"field_id": fieldID, "value": value, "entity_type": "issue"},
)
sess.And(builder.In("issue.id", subQuery))
}
+25 -20
View File
@@ -289,16 +289,9 @@ func SearchIssues(ctx *context.APIContext) {
}
}
// Parse custom field filters from cf_{fieldID}={value} query params.
cfFilters := make(map[int64]string)
for key, values := range ctx.Req.URL.Query() {
if after, ok := strings.CutPrefix(key, "cf_"); ok && len(values) > 0 && values[0] != "" {
if fieldID, parseErr := strconv.ParseInt(after, 10, 64); parseErr == nil {
cfFilters[fieldID] = values[0]
}
}
}
if len(cfFilters) > 0 {
if cfFilters, cfErr := parseAPICustomFieldFilters(ctx); cfErr != nil {
return
} else if len(cfFilters) > 0 {
searchOpt.CustomFieldFilters = cfFilters
}
@@ -530,16 +523,9 @@ func ListIssues(ctx *context.APIContext) {
searchOpt.MentionID = optional.Some(mentionedByID)
}
// Parse custom field filters from cf_{fieldID}={value} query params.
cfFilters := make(map[int64]string)
for key, values := range ctx.Req.URL.Query() {
if after, ok := strings.CutPrefix(key, "cf_"); ok && len(values) > 0 && values[0] != "" {
if fieldID, parseErr := strconv.ParseInt(after, 10, 64); parseErr == nil {
cfFilters[fieldID] = values[0]
}
}
}
if len(cfFilters) > 0 {
if cfFilters, cfErr := parseAPICustomFieldFilters(ctx); cfErr != nil {
return
} else if len(cfFilters) > 0 {
searchOpt.CustomFieldFilters = cfFilters
}
@@ -579,6 +565,25 @@ func getUserIDForFilter(ctx *context.APIContext, queryName string) int64 {
return user.ID
}
// parseAPICustomFieldFilters extracts cf_{fieldID}=value query parameters.
// Returns an error (and writes a 400 response) if a field ID is non-numeric or non-positive.
func parseAPICustomFieldFilters(ctx *context.APIContext) (map[int64]string, error) {
filters := make(map[int64]string)
for key, values := range ctx.Req.URL.Query() {
after, ok := strings.CutPrefix(key, "cf_")
if !ok || len(values) == 0 || values[0] == "" {
continue
}
fieldID, err := strconv.ParseInt(after, 10, 64)
if err != nil || fieldID <= 0 {
ctx.APIError(http.StatusBadRequest, fmt.Sprintf("invalid custom field filter: cf_%s must use a positive numeric field ID", after))
return nil, fmt.Errorf("invalid cf_ param")
}
filters[fieldID] = values[0]
}
return filters, nil
}
// GetIssue get an issue of a repository
func GetIssue(ctx *context.APIContext) {
// swagger:operation GET /repos/{owner}/{repo}/issues/{index} issue issueGetIssue
+20 -9
View File
@@ -525,19 +525,14 @@ func prepareIssueFilterAndList(ctx *context.Context, milestoneID int64, projectI
prepareIssueFilterExclusiveOrderScopes(ctx, preparedLabelFilter.AllLabels)
// Parse custom field filters from query params (cf_{fieldID}={value}).
customFieldFilters := make(map[int64]string)
for key, values := range ctx.Req.URL.Query() {
if after, ok := strings.CutPrefix(key, "cf_"); ok && len(values) > 0 && values[0] != "" {
if fieldID, err := strconv.ParseInt(after, 10, 64); err == nil {
customFieldFilters[fieldID] = values[0]
}
}
}
customFieldFilters := parseCustomFieldQueryParams(ctx.Req.URL.Query())
// Load custom field definitions for the filter UI.
// If this fails, clear filters so users don't get invisible filtering.
customFieldDefs, cfErr := issues_model.GetCustomFieldsByOwner(ctx, repo.OwnerID, issues_model.CustomFieldScopeIssue)
if cfErr != nil {
log.Error("prepareIssueFilterAndList: GetCustomFieldsByOwner: %v", cfErr)
customFieldFilters = make(map[int64]string)
}
ctx.Data["CustomFieldDefs"] = customFieldDefs
ctx.Data["CustomFieldFilters"] = customFieldFilters
@@ -551,7 +546,9 @@ func prepareIssueFilterAndList(ctx *context.Context, milestoneID int64, projectI
for _, f := range customFieldDefs {
if f.Options != "" {
var opts []string
if err := json.Unmarshal([]byte(f.Options), &opts); err == nil {
if err := json.Unmarshal([]byte(f.Options), &opts); err != nil {
log.Error("prepareIssueFilterAndList: invalid options JSON for field %d (%s): %v", f.ID, f.Name, err)
} else {
fieldOptions[f.ID] = opts
}
}
@@ -810,3 +807,17 @@ func Issues(ctx *context.Context) {
ctx.HTML(http.StatusOK, tplIssues)
}
// parseCustomFieldQueryParams extracts cf_{fieldID}=value query parameters.
// Non-numeric or non-positive field IDs are silently skipped.
func parseCustomFieldQueryParams(query url.Values) map[int64]string {
filters := make(map[int64]string)
for key, values := range query {
if after, ok := strings.CutPrefix(key, "cf_"); ok && len(values) > 0 && values[0] != "" {
if fieldID, err := strconv.ParseInt(after, 10, 64); err == nil && fieldID > 0 {
filters[fieldID] = values[0]
}
}
}
return filters
}