fix(metrics): truncate header-derived values and validate location length on metrics enter/leave by romanetar · Pull Request #631 · OpenStackweb/summit-api

romanetar · 2026-10-12T12:57:37Z

ref https://app.clickup.com/t/9014802374/86bcg38nf

Problem

PUT /api/v1/summits/{id}/metrics/enter returns 500 when the User-Agent is longer than 255 characters (e.g. Facebook in-app browsers). SummitMetric::build() copies User-Agent, Referer/Origin and Client-IP/X-Forwarded-For unchanged into VARCHAR(255) columns, so MySQL (strict mode in production) rejects the INSERT with Data too long for column 'Browser', the transaction rolls back and the visit is lost. In the 7 days to 2026-10-07, 9 of 16,252 calls failed this way. The location body field has the same weakness (no length validation, stored in VARCHAR(255)).

Changes

SummitMetric: new MaxStringColumnLength = 255 constant; ip, origin and browser go through a private truncate() helper (mb_substr, null-safe) in build(). length is now explicit on the three columns (255 is already the default, so no schema change). One change covers every path, since build() is the only reader of these headers.
OAuth2SummitMetricsApiController: location is now sometimes|string|max:255, so a too-long location returns 412 instead of 500.
OpenAPI: maxLength: 255 on location.

Scope note: `leave` included

The task only mentions enter(), but POST metrics/leave has the same unvalidated location rule, so max:255 and maxLength: 255 are applied there too (SummitMetricLeaveRequest).

Tests

New tests in OAuth2SummitMetricsApiControllerTest:

300-char User-Agent/Referer/X-Forwarded-For are cut to the first 255 characters
Multibyte User-Agent is cut to 255 characters and stays valid UTF-8
User-Agent of 255 characters is unchanged
enter with a 256-char location returns 412, 255 returns 201
leave location length (256 returns 412, 255 returns 201)

Headers are set in $_SERVER (that is what UserClientHelper reads) and restored in tearDown. CI and local MySQL run non-strict, so the 500 does not reproduce there; the tests assert on the browser, origin and ip fields of the response instead. Without the fix, the header, multibyte, enter 412 and leave tests fail; the 255-character ones pass either way as controls. With it, all 10 tests in the class pass.

Post-deploy verification

Over 7 days, Loki should show no Data too long for column 'Browser' lines in summit-api-php, and PUT metrics/enter with a User-Agent over 255 characters should return 201 (baseline: 9 of 9 returned 500).

Risks

The stored User-Agent/Referer/IP of the affected requests loses everything after 255 characters. Nothing reads the full values, and those calls fail entirely today.
A client sending a location over 255 characters gets 412 instead of 500.

🤖 Generated with Claude Code

coderabbitai · 2026-10-12T12:57:49Z

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 9e7e29b7-99d5-46c4-9f10-1cf433e22ca6

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-10-12T12:59:54Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-631/

This page is automatically updated on each push to this PR.

smarcet

@romanetar please review

Copilot

🟢 Approval recommended

The implementation consistently addresses the database length failures and includes appropriate regression coverage.

0 open findings

What changed in this PR

Prevents metric insert failures caused by values exceeding VARCHAR(255) limits.

Changes:

Truncates header-derived metric values safely, including multibyte strings.
Validates and documents the 255-character location limit.
Adds boundary and truncation tests.

File	Description
`SummitMetric.php`	Truncates header-derived values.
`OAuth2SummitMetricsApiController.php`	Validates location length.
`SummitMetricsSchemas.php`	Documents location limits.
`OAuth2SummitMetricsApiControllerTest.php`	Tests truncation and validation boundaries.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-10-19T15:05:29Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-631/

This page is automatically updated on each push to this PR.

github-actions · 2026-10-19T15:10:00Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-631/

This page is automatically updated on each push to this PR.

…ngth on metrics enter/leave SummitMetric::build() copied User-Agent, Referer/Origin and Client-IP/X-Forwarded-For unchanged into VARCHAR(255) columns, so a longer value made MySQL reject the INSERT and metrics/enter returned 500. Cut them to 255 characters with mb_substr and limit location to 255 in the enter and leave payloads (412 instead of 500).

…ith 412 location is client metadata like the header derived values. Validating the raw URL-encoded value rejected requests that fit the column once decoded. SummitMetric::setLocation() now cuts the decoded value to 255 characters, the max:255 rules and OpenAPI maxLength are reverted, and the tests assert truncation and that an encoded location decoding under 255 is kept.

SummitMetric.Location is a TEXT column (Version20201016145706), so long values never caused the insert failure. Revert the location truncation and the VARCHAR(255) mapping; only Ip, Origin and Browser are truncated.

github-actions · 2026-10-19T15:15:47Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-631/

This page is automatically updated on each push to this PR.

romanetar requested a review from smarcet October 12, 2026 13:42

smarcet assigned romanetar Oct 14, 2026

smarcet reviewed Oct 14, 2026

View reviewed changes

Comment thread app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitMetricsApiController.php Outdated

smarcet requested a balanced review from Copilot October 14, 2026 19:32

smarcet requested changes Oct 14, 2026

View reviewed changes

Copilot started reviewing on behalf of smarcet October 14, 2026 19:32

View session

Copilot AI reviewed Oct 14, 2026

View reviewed changes

romanetar requested a review from smarcet October 19, 2026 15:11

romanetar added 3 commits October 19, 2026 17:12

fix(metrics): keep location untruncated, its column is TEXT

afa542e

SummitMetric.Location is a TEXT column (Version20201016145706), so long values never caused the insert failure. Revert the location truncation and the VARCHAR(255) mapping; only Ip, Origin and Browser are truncated.

romanetar force-pushed the fix/metrics-enter-header-truncation branch from b9a5fff to afa542e Compare October 19, 2026 15:13

Navigation Menu

Repository navigation

fix(metrics): truncate header-derived values and validate location length on metrics enter/leave - #631

fix(metrics): truncate header-derived values and validate location length on metrics enter/leave#631
romanetar wants to merge 3 commits into
mainfrom
fix/metrics-enter-header-truncation

romanetar commented Oct 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

Uh oh!

smarcet left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

romanetar commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

Scope note: leave included

Tests

Post-deploy verification

Risks

Uh oh!

coderabbitai Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

Uh oh!

smarcet left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

romanetar commented Oct 12, 2026 •

edited

Loading

Scope note: `leave` included

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading