Skip to content

fix: normalize missing bruteforce_protection and headers_to_exclude on ExApp routes#882

Merged
oleksandr-nc merged 1 commit into
mainfrom
fix/route-defaults-normalization
May 16, 2026
Merged

fix: normalize missing bruteforce_protection and headers_to_exclude on ExApp routes#882
oleksandr-nc merged 1 commit into
mainfrom
fix/route-defaults-normalization

Conversation

@oleksandr-nc
Copy link
Copy Markdown
Contributor

ExAppProxyController::buildHeadersWithExclude crashes with TypeError: json_decode(): Argument #1 must be of type string, null given when a route's headers_to_exclude (or bruteforce_protection) column is NULL or otherwise non-string - the proxy returns HTTP 500 before the request ever leaves Nextcloud.

This PR centralizes the parse in a single static helper ExAppMapper::parseJsonList() that tolerates NULL, malformed JSON, and non-string inputs, and uses it from the three sites that read those columns (ExAppProxyController, HarpService). ExAppService::registerExAppRoutes is also tightened to refetch the ExApp after insert and broaden its catch.

A follow-up PR will add fail-fast validation at registration time so malformed info.xml / --json payloads are rejected upfront with a clear error, instead of being silently coerced to '[]' as they are today.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 16, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 62111de6-d9ea-447d-b070-500cc4c4390c

📥 Commits

Reviewing files that changed from the base of the PR and between 21a31ac and 2e78425.

📒 Files selected for processing (5)
  • lib/Controller/ExAppProxyController.php
  • lib/Db/ExAppMapper.php
  • lib/Service/ExAppService.php
  • lib/Service/HarpService.php
  • tests/php/Db/ExAppMapperTest.php
🚧 Files skipped from review as they are similar to previous changes (4)
  • lib/Service/HarpService.php
  • lib/Db/ExAppMapper.php
  • lib/Controller/ExAppProxyController.php
  • lib/Service/ExAppService.php

📝 Walkthrough

Walkthrough

This PR adds ExAppMapper::parseJsonList to normalize JSON-list DB fields and tests it. Callers were updated to use it: ExAppProxyController (bruteforce_protection, headers_to_exclude) and HarpService (route bruteforce_protection). ExAppService::registerExAppRoutes now returns the ExApp reloaded from persistence and broadens its exception handling.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: introducing a fix that normalizes missing or malformed bruteforce_protection and headers_to_exclude fields on ExApp routes, which directly addresses the core issue described in the PR.
Description check ✅ Passed The description is directly related to the changeset, explaining the problem (TypeError crash), the solution (centralized parseJsonList helper), where it's used (three sites), and future plans.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5dd5e717-920d-4226-963f-1a581d18c6eb

📥 Commits

Reviewing files that changed from the base of the PR and between f9445cb and 21a31ac.

📒 Files selected for processing (5)
  • lib/Controller/ExAppProxyController.php
  • lib/Db/ExAppMapper.php
  • lib/Service/ExAppService.php
  • lib/Service/HarpService.php
  • tests/php/Db/ExAppMapperTest.php

Comment thread lib/Controller/ExAppProxyController.php Outdated
…n ExApp routes

Signed-off-by: Oleksander Piskun <oleksandr2088@icloud.com>
@oleksandr-nc oleksandr-nc force-pushed the fix/route-defaults-normalization branch from 21a31ac to 2e78425 Compare May 16, 2026 07:04
@oleksandr-nc oleksandr-nc merged commit 5134cca into main May 16, 2026
53 checks passed
@oleksandr-nc oleksandr-nc deleted the fix/route-defaults-normalization branch May 16, 2026 07:15
@oleksandr-nc
Copy link
Copy Markdown
Contributor Author

/backport to stable33

@oleksandr-nc
Copy link
Copy Markdown
Contributor Author

/backport to stable34

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant