Fix GET /software/versions 422 too many placeholders without per_page (#45737)
Closes #43030 ## Summary - Batches title IDs in `getDisplayNamesByTeamAndTitleIds` (chunks of 32,000) to avoid exceeding MySQL's 65,535 prepared statement placeholder limit - Uses the existing `BatchProcessSimple` utility, matching the pattern already used in `software_titles.go` ## Root cause When `GET /api/v1/fleet/software/versions` is called without a `per_page` parameter, `DefaultPerPage` (1,000,000) is used. `ListSoftware` collects all `titleIDs` from the paginated results and passes them to `getDisplayNamesByTeamAndTitleIds`, which builds an `IN (?)` clause that exceeds MySQL's 65,535 placeholder limit. ## Manual testing 1. Started a local Fleet server with MySQL via `docker compose up` and `fleet serve --dev` 2. Seeded the database with 70,000 software titles, software entries, and software_host_counts records 3. **Before the fix**: `GET /api/latest/fleet/software/versions` (no `per_page`) returned HTTP 422 with `"Prepared statement contains too many placeholders"` 4. **After the fix**: the same request returns HTTP 200 with all 70,000 results 5. `GET /api/latest/fleet/software/versions?per_page=20` continued to work correctly in both cases ## Test plan - [x] Manual reproduction and verification (see above) - [x] `make lint-go-incremental` passes - [x] `go build ./server/datastore/mysql/...` compiles cleanly - [ ] CI passes <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed `GET /api/v1/fleet/software/versions` endpoint to prevent errors when returning results from large software inventories. * **Tests** * Added test coverage for high-volume display name queries. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/45737?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1 @@
|
||||
- Fixed `GET /api/v1/fleet/software/versions` returning HTTP 422 "too many placeholders" when called without a `per_page` parameter on instances with large software inventories.
|
||||
@@ -123,6 +123,7 @@ func TestSoftware(t *testing.T) {
|
||||
{"ListHostSoftwareShPackageForDarwin", testListHostSoftwareShPackageForDarwin},
|
||||
{"HostSWPaginationWithMultipleFMAVersions", testHostSWPaginationWithMultipleFMAVersions},
|
||||
{"SoftwareLiteByID", testSoftwareLiteByID},
|
||||
{"GetDisplayNamesByTeamAndTitleIdsBatching", testGetDisplayNamesByTeamAndTitleIdsBatching},
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
@@ -12173,3 +12174,73 @@ func testListSoftwareVulnerabilitiesBySoftwareIDs(t *testing.T, ds *Datastore) {
|
||||
require.NoError(t, err)
|
||||
require.Nil(t, result)
|
||||
}
|
||||
|
||||
func testGetDisplayNamesByTeamAndTitleIdsBatching(t *testing.T, ds *Datastore) {
|
||||
ctx := t.Context()
|
||||
|
||||
// Insert 35,000 software titles with display names to exercise multiple
|
||||
// batches (batch size is 32,000).
|
||||
const totalTitles = 35_000
|
||||
titleIDs := make([]uint, 0, totalTitles)
|
||||
|
||||
// Batch-insert titles
|
||||
const insertBatch = 1000
|
||||
for start := 0; start < totalTitles; start += insertBatch {
|
||||
end := min(start+insertBatch, totalTitles)
|
||||
valuesSQL := strings.Builder{}
|
||||
args := make([]any, 0, (end-start)*2)
|
||||
for i := start; i < end; i++ {
|
||||
if i > start {
|
||||
valuesSQL.WriteString(",")
|
||||
}
|
||||
valuesSQL.WriteString("(?, 'apps')")
|
||||
args = append(args, fmt.Sprintf("batch-test-sw-%d", i))
|
||||
}
|
||||
res, err := ds.writer(ctx).ExecContext(ctx,
|
||||
"INSERT INTO software_titles (name, source) VALUES "+valuesSQL.String(), args...)
|
||||
require.NoError(t, err)
|
||||
|
||||
lastID, err := res.LastInsertId()
|
||||
require.NoError(t, err)
|
||||
rowsAff, err := res.RowsAffected()
|
||||
require.NoError(t, err)
|
||||
// MySQL returns the first auto-inc ID for a batch insert
|
||||
for j := range rowsAff {
|
||||
titleIDs = append(titleIDs, uint(lastID+j)) //nolint:gosec // test-only, no overflow risk
|
||||
}
|
||||
}
|
||||
require.Len(t, titleIDs, totalTitles)
|
||||
|
||||
// Insert display names for all titles (team_id=0)
|
||||
for start := 0; start < totalTitles; start += insertBatch {
|
||||
end := min(start+insertBatch, totalTitles)
|
||||
valuesSQL := strings.Builder{}
|
||||
args := make([]any, 0, (end-start)*2)
|
||||
for i := start; i < end; i++ {
|
||||
if i > start {
|
||||
valuesSQL.WriteString(",")
|
||||
}
|
||||
valuesSQL.WriteString("(0, ?, ?)")
|
||||
args = append(args, titleIDs[i], fmt.Sprintf("Display Name %d", i))
|
||||
}
|
||||
_, err := ds.writer(ctx).ExecContext(ctx,
|
||||
"INSERT INTO software_title_display_names (team_id, software_title_id, display_name) VALUES "+valuesSQL.String(), args...)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
// Call the function under test with all 35,000 IDs (spans 2 batches: 32K + 3K)
|
||||
result, err := ds.getDisplayNamesByTeamAndTitleIds(ctx, 0, titleIDs)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, result, totalTitles)
|
||||
|
||||
// Verify a sample of results
|
||||
for _, i := range []int{0, 1, 1000, 31999, 32000, 34999} {
|
||||
expected := fmt.Sprintf("Display Name %d", i)
|
||||
assert.Equal(t, expected, result[titleIDs[i]], "mismatch at index %d", i)
|
||||
}
|
||||
|
||||
// Empty input should return empty map, not error
|
||||
result, err = ds.getDisplayNamesByTeamAndTitleIds(ctx, 0, nil)
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, result)
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"database/sql"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
|
||||
common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"
|
||||
"github.com/jmoiron/sqlx"
|
||||
)
|
||||
|
||||
@@ -31,28 +32,38 @@ func (ds *Datastore) getDisplayNamesByTeamAndTitleIds(ctx context.Context, teamI
|
||||
return map[uint]string{}, nil
|
||||
}
|
||||
|
||||
var args []any
|
||||
query := `
|
||||
SELECT software_title_id, display_name
|
||||
FROM software_title_display_names
|
||||
WHERE software_title_id IN (?) AND team_id = ?
|
||||
`
|
||||
query, args, err := sqlx.In(query, titleIDs, teamID)
|
||||
namesBySoftwareTitleID := make(map[uint]string, len(titleIDs))
|
||||
|
||||
// Process in batches to avoid exceeding MySQL's 65,535 prepared statement
|
||||
// placeholder limit when the caller passes a large number of title IDs
|
||||
// (e.g., when per_page is not specified and defaults to 1,000,000).
|
||||
const batchSize = 32000
|
||||
err := common_mysql.BatchProcessSimple(titleIDs, batchSize, func(batch []uint) error {
|
||||
query := `
|
||||
SELECT software_title_id, display_name
|
||||
FROM software_title_display_names
|
||||
WHERE software_title_id IN (?) AND team_id = ?
|
||||
`
|
||||
query, args, err := sqlx.In(query, batch, teamID)
|
||||
if err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "building query for get software title display names")
|
||||
}
|
||||
|
||||
var results []struct {
|
||||
SoftwareTitleID uint `db:"software_title_id"`
|
||||
DisplayName string `db:"display_name"`
|
||||
}
|
||||
if err := sqlx.SelectContext(ctx, ds.reader(ctx), &results, query, args...); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "get software title display names")
|
||||
}
|
||||
|
||||
for _, r := range results {
|
||||
namesBySoftwareTitleID[r.SoftwareTitleID] = r.DisplayName
|
||||
}
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
return nil, ctxerr.Wrap(ctx, err, "building query for get software title display names")
|
||||
}
|
||||
|
||||
var results []struct {
|
||||
SoftwareTitleID uint `db:"software_title_id"`
|
||||
DisplayName string `db:"display_name"`
|
||||
}
|
||||
if err := sqlx.SelectContext(ctx, ds.reader(ctx), &results, query, args...); err != nil {
|
||||
return nil, ctxerr.Wrap(ctx, err, "get software title display names")
|
||||
}
|
||||
|
||||
namesBySoftwareTitleID := make(map[uint]string, len(results))
|
||||
for _, r := range results {
|
||||
namesBySoftwareTitleID[r.SoftwareTitleID] = r.DisplayName
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return namesBySoftwareTitleID, nil
|
||||
|
||||
Reference in New Issue
Block a user