mirror of
https://github.com/FOSSBilling/FOSSBilling.git
synced 2026-11-03 10:36:49 +09:00
main
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Adam Daley
|
85fa3dc1b2 |
Refactor HTTP helpers and inject Filesystem via DI (#3900)
* Remove redundant HTTP helper wrappers * Inject Filesystem via DI across services * Fix constructor and Theme DI wiring |
||
|
FOSSBilling BotandFOSSBilling Bot
|
3c6ab0bd4f |
Apply PHP Code Styling & Quality Changes (#3739)
Co-authored-by: FOSSBilling Bot <fossbilling-bot> |
||
|
Adam Daley
|
5ecfeb335e |
Harden templates and API responses against missing data (#3730)
* Guard Activity index template against missing client/staff nested data
The activity log API at Activity/Api/Admin.php:38-49 only injects a
nested 'client' or 'staff' key when the corresponding *_id is set
(via isset() which returns false for null). System events and
client-only events therefore have no 'client' key, while staff-only
events have no 'client' key.
Under strict_variables, '{% if event.client %}' (line 191) throws
"Key 'client' ... does not exist" for any event without a client.
Switch the conditional to event.client_id / event.staff_id (always
present as SQL columns) and add '|default("")' to the nested
event.client.email / event.client.name / event.staff.email / event.staff.name
accesses so deleted-but-referenced records render gracefully.
* Guard Security iplookup template against empty country/asn data
When the GeoIP or ASN lookup fails (IncompleteRecord from MaxMind), the
Service catches and substitutes [] for the missing data. Under
strict_variables, the empty mapping at record.asn / record.country
causes line 48 to throw "sequence/mapping is empty".
Twig's |default filter on a GetAttrExpression compiles to a safe
is-defined check, so it correctly falls through to the placeholder
when the underlying array is empty or the key is missing.
Verified:
- /admin/security/iplookup?ip=192.168.1.1 (private IP, empty asn)
renders the row with 'Unknown' ASN cells.
- /admin/security/iplookup?ip=8.8.8.8 still renders
'15169' / 'GOOGLE' / 'United States'.
* Initialize notes/rel arrays in Support toApiArray so templates render without errors
Two related issues surfaced under strict_variables on the admin support
ticket page:
1. toApiArray only set $data['notes'] inside the foreach loop over
ticket notes. Tickets without any notes therefore had no 'notes' key
and 'ticket.notes|length' in mod_support_ticket.html.twig:42 threw
'Key "notes" does not exist'.
2. _getRelDetails returned [] (empty array) when a ticket has no
related entity. mod_support_ticket.html.twig sets {% set task =
ticket.rel %} and then accesses task.task, task.type, task.status
etc. on the next lines. Any access into an empty mapping throws
under strict_variables.
Fixes:
- Service::toApiArray: always initialize $data['notes'] = [] for admin
identity so the key is present even when empty.
- Service::_getRelDetails: always return the base structure with all
fields (id, type, task, new_value, status) set to null when there
is no related entity. Skip the DB lookups for the non-rel case.
Template hardening (defense-in-depth, in case other code paths return
a different shape):
- mod_support_ticket.html.twig:42: 'ticket.notes|length' -> 'is defined and
ticket.notes|length'.
- mod_support_ticket.html.twig:222: 'ticket.notes' -> 'ticket.notes|default([])'.
- mod_support_tickets.html.twig:238: same is defined guard.
Verified on dev:
- /admin/support/ticket/1 renders without errors.
- API now returns rel as {id: null, type: null, ...} and notes: [] for
tickets without related data.
- 1777 local tests pass, PHPStan clean.
* Harden tests and add Twig/ToApi linters
* Remove redundant setAccessible calls in tests
* Update cspell.json word list
* Add UrlAwarePermissiveContainer & trim errors
* Add geoip stub and fix ParserFactory usage
* Enforce and apply parentheses for new expressions
|
||
|
Adam DaleyandCopilot Autofix powered by AI
|
7baa28e22f |
Enhance Twig strict variables handling and improve test coverage (#3721)
* Import template macros and fix invoices tab id
* Handle nulls in Twig extensions and add tests
* Enable Twig strict_variables and add tests
* Improve Twig strict-variables test harness
Fix several undefined-variable issues in admin templates and harden the strict-variables test harness.
- Templates: guard against missing variables by using default() for order_actions and checking header/type in partial_extensions.twig.
- Loader: CombinedTwigLoader now registers theme and module template paths with namespaces (module_area), and includes email template directories.
- Variable collection: VariableCollectorVisitor now collects keys passed via {% embed %} and {% include %} with { ... } so embedded templates get proper stubs.
- Renderer: StrictTemplateRenderer gains better relative template naming for module templates, additional logging for kb_article parsing, and broader classification of render errors as test-infra (typed-return mismatches, NumberFormatter issues, callable/iterable errors, loader quirks) to reduce false positives.
- Tests: add baseline marker files for strict findings, adjust StrictVariablesTest to treat infra findings as informational while failing on real-bug findings, and write findings JSON for review.
Overall these changes reduce noise from the render-everything harness and make the strict-variables suite more robust and actionable.
* Bump test memory limit to 2G via ini_set in bootstrap
The strict-variables render-everything harness parses every FOSSBilling
template and allocates many stubs and AST nodes, which exceeds the
default 128M limit and causes the pest process to exit with 255
mid-suite. Setting the limit in tests/Pest.php means it applies to both
local 'composer test' and CI 'pest --ci' invocations without needing
to modify the workflow file.
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Scope error_reporting suppression to the render-everything harness
* Replace unsupported 'now' Twig global with 'now'|date filter
The reserved 'now' Twig global is only defined in some contexts and is
not present in the install or admin email-template environments. Under
strict_variables it throws a fatal 'Variable "now" does not exist'
error, which crashes the installer and the email template playground.
Both call sites use the date filter to format the current year, so the
correct fix is to feed the filter a string literal: 'now'|date('Y').
This is the same pattern we used for the admin_default and huraga theme
layouts.
Discovered by hitting the install page on the dev instance after the
strict_variables rollout.
* Make sitemap tolerant of missing product.updated_at
The product guest API (Box\Mod\Product\Service::toApiArray) returns
updated_at only for admin requests, not for guests. The sitemap uses
the guest API, so product.updated_at was always undefined for it.
Before strict_variables this was harmless: the date filter silently
returned null and the template rendered. With strict_variables enabled,
the missing key throws 'Key updated_at does not exist', which
get_custom_page catches and converts to a 404.
Pipe through |default('now') so we fall back to the current date when
the field is missing, while still using the real value when present.
Discovered by the live test 'urls return ok status' failing on sitemap.xml
in CI.
* Remove stray blank line in StrictTemplateRenderer
* Fix four templates for strict_variables Cypress failures
The Cypress tests run against a fresh install with strict_variables
enabled, which catches undefined key access in templates that the
non-strict mode silently allowed.
1. mod_invoice_index.html.twig:432
productId is a JavaScript variable, not a Twig variable. The
Twig expression tried to evaluate it server-side and threw
'Variable productId does not exist'. Build the URL with JS
template literal interpolation instead.
2. mod_client_profile.html.twig:158
guest.client_custom_fields is not a real field in the guest
API response. The current lookup threw 'sequence/mapping is
empty' when no custom fields exist. Default to an empty
mapping, then null, and add a truthy guard.
3. mod_support_ticket.html.twig:60
ticket.rel_type is only populated for tickets linked to an
order; the guest response omits it otherwise. Default to
empty string so the comparison still works.
4. mod_support_ticket.html.twig:80
message.author exposes only name and role for client-side
messages, no email. Pass through |default('') so the avatar
function gets a string and renders its fallback.
* Fix client invoice page for strict_variables
The tab labels on the client invoice index reference
unpaid_invoices.total and paid_invoices.total on lines 33/37
to show count badges, but the corresponding {% set %} calls
sit inside the table bodies on lines 53/85. Under
strict_variables this throws 'Variable unpaid_invoices does
not exist' before the lists are ever fetched.
Move both fetches to the top of the content block so the
variables are defined when first read.
* Allow guest checkout rendering under strict vars
* Rename test helper and tweak test expectations
* Guard optional adapter form fields in gateway and build_form templates
The admin payment gateway edit page and the domain registrar build_form
macro both test element[1].description directly in an if condition,
and both read gateway.config[name] / values[name] without a default.
Built-in adapters (Stripe, PayPalEmail, Custom, the Custom registrar)
define form fields with only a label and sometimes validators or
required_when, so on a fresh install the page throws:
Key 'description' for sequence/mapping with keys 'label, validators'
does not exist in 'mod_invoice_gateway.html.twig' at line 115.
The outer '?? ''' on the inner expression does not help: the
exception is thrown when evaluating the if condition, not when
rendering the markdown. With strict_variables on, that becomes a 500.
Replace each 'if element[1].description' with the short-circuit
'is defined and ...' pattern already used in the same file for
required_when, and drop the now-redundant '?? ''' from the inner
expression. Guard the gateway.config[name] / values[name] reads
with |default('') (or |default([]) for the iterable check) so
fields that have not been configured yet do not throw on a fresh
or upgraded install.
Verified on the dev instance:
/admin/invoice/gateway/{1,2} HTTP 200
/admin/servicedomain/registrar/{1,2} HTTP 200
PayPal gateway form save+reload round-trips correctly.
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|