Skip to content

Fix: correct minor bugs in the import/export workflow - #656

Open
Rom1-B wants to merge 1 commit into
mainfrom
fix_import
Open

Fix: correct minor bugs in the import/export workflow#656
Rom1-B wants to merge 1 commit into
mainfrom
fix_import

Conversation

@Rom1-B

@Rom1-B Rom1-B commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

  • fixes #N/A
  • Corrects several minor inconsistencies in the import and export workflow.
  • Improves the reliability of checks and data handling used when running and configuring injection models.

Screenshots (if appropriate):

@Rom1-B
Rom1-B requested a review from stonebuzz August 25, 2026 09:53
TemplateRenderer::getInstance()->display('@datainjection/clientinjection_result.html.twig', $data);
}

private static function escapeCsvFormula($value)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

New security function with no unit test. It is a pure function with several edge cases worth pinning (empty string, non-string passthrough, each of the four trigger characters, a value starting with a safe character). Without a test, a future refactor or PHP upgrade could silently break the protection and let formula-injection payloads through.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing PHP 8.2 type declarations. The function accepts any type (non-strings are passed through unchanged) and returns the same type. Project rules require strict typing everywhere.

Suggested change
private static function escapeCsvFormula($value)
private static function escapeCsvFormula(mixed $value): mixed


private static function escapeCsvFormula($value)
{
if (is_string($value) && isset($value[0]) && str_contains('=+-@', $value[0])) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Arguments are in an unconventional order: str_contains reads as "does haystack contain needle", so str_contains('=+-@', $value[0]) means "does '=+-@' contain the first char?"
which is correct but non-obvious. in_array makes the intent explicit and self-documents the set of trigger characters.

Suggested change
if (is_string($value) && isset($value[0]) && str_contains('=+-@', $value[0])) {
if (is_string($value) && isset($value[0]) && in_array($value[0], ['=', '+', '-', '@'], true)) {

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.

2 participants