Skip to content

FIX: Restore repeater macro filter compatibility without breaking formatted output https://github.com/Crocoblock/jetformbuilder/issues/651#665

Open
Gawuww wants to merge 1 commit into
release/3.6.3from
issue/651
Open

FIX: Restore repeater macro filter compatibility without breaking formatted output https://github.com/Crocoblock/jetformbuilder/issues/651#665
Gawuww wants to merge 1 commit into
release/3.6.3from
issue/651

Conversation

@Gawuww

@Gawuww Gawuww commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Issue - #651

@github-actions

Copy link
Copy Markdown

🤖 AI PR Review

Risk level: medium

Review

Summary

  • This PR updates many built frontend/editor/admin assets (files under assets/build/*) to restore repeater macro filter compatibility while keeping formatted output stable (per PR title). The changes are shipped as updated build artifacts (JS/CSS/asset.php) rather than obvious small source diffs.

What I checked

  • Files changed are almost exclusively compiled assets: assets/build/frontend/.js, assets/build/editor/.asset.php, assets/build/admin/*.asset.php and CSS bundles.
  • Notable compiled modules present in the bundles: calculated.field, conditional.block, dynamic.value, media.field and media.field.restrictions.

Issues / Risks

  1. Security: use of eval()
  • In the compiled frontend/calculated.field.js I found: const millis = eval(millisInput);
  • eval() on input is a high-risk vector: it can execute arbitrary JS if the expression originates from untrusted data (form values, macros, etc.). Even if the caller currently provides only numeric milliseconds, eval is unsafe. This should be replaced with a safe parser (Number(), parseInt/parseFloat with radix, or stricter validation) and explicit sanitization.
  • Please show where millisInput comes from in source code and ensure it cannot be exploited (and preferably remove eval altogether).
  1. Built assets only / missing source changes
  • The PR appears to contain only rebuilt assets. I could not find corresponding source changes (ES modules, PHP hooks) in the diff. Please confirm and either:
    • Add the source changes that produced the build (recommended), or
    • Add an explicit note to maintainers that this PR only includes built artifacts and reference the source commit/branch that was built.
  • Reviewing built/minified bundles makes security and logic review hard; maintainers need the original source to audit the exact logic changes (especially for conditional logic and macros).
  1. Backward compatibility
  • Because the PR replaces compiled bundles, please confirm that no public JS/PHP APIs or global variables were changed/removed. I saw continued usage of global objects (window.JetFormBuilderAbstract, JetPlugins.hooks, deprecated filters). Ensure deprecated filter paths are still honored where intended for backwards compatibility with user code and third-party add-ons.
  1. Performance / bundle size / sourcemaps
  • Many CSS files have their sourceMappingURL referenced in previous bundles; ensure source maps are preserved or regenerated in the build pipeline to ease debugging.
  • Please verify that the rebuild did not accidentally include duplicate libraries or change dependency lists in asset.php files in a way that could enqueue different WP scripts.
  1. Tests & QA
  • This touches repeater/macros/conditional logic and calculated fields — high-risk areas for forms. There are no tests included in the PR. Please provide/attach:
    • Unit or integration tests validating repeater macro filters and formatted output (date/time formatting, calculated fields),
    • Manual QA checklist (multistep forms, repeaters, macros in notifications/actions, media field previews) and test results.

Requested fixes / clarifications

  1. Replace eval(millisInput) with a safe parsing approach and sanitize input. Example suggestions:

    • If millisInput is expected to be numeric: use Number(millisInput) or parseInt(millisInput, 10) and validate isFinite() / !isNaN() before constructing Date.
    • If millisInput is an expression that must be evaluated, please document why eval is required and run it in a safe sandbox; but avoid eval whenever possible.
  2. Provide the original source changes (not only compiled assets) or a link to the commit that produced these bundles so reviewers can inspect the true code changes.

  3. Add/attach tests for repeater macro filters and calculated fields (including date formatting) and run them in CI.

  4. Confirm no public JS globals or filter hooks were removed, and ensure deprecated filter compatibility is preserved (I saw code referring to deprecated applyFilters). Document any intentional breaking changes.

  5. Ensure source maps are included in the release/build artifacts or stored in the repo to help debugging.

Minor

  • Update the changelog entry (see suggested line below).
  • Mark PR as including only built assets if that is intended.

Overall recommendation

  • Do not merge until:
    • eval() usage is removed or justified and sandboxed,
    • source files producing these bundles are included/referenced for review,
    • tests/QA for repeater macros and calculated formatting are provided/passed.

Files to pay attention to (examples found in built bundles)

  • assets/build/frontend/calculated.field.js (eval usage)
  • assets/build/frontend/conditional.block.js
  • assets/build/frontend/dynamic.value.js
  • assets/build/frontend/media.field.js and media.field.restrictions.js
  • updated asset version files: assets/build/**/.asset.php

If you want, paste the corresponding source files (before build) here and I’ll do a focused review of the logic changes and propose concrete patch(es) replacing eval and ensuring safe parsing/formatting.

Suggested changelog entry

- FIX: Restore repeater macro filter compatibility for macros/repeater fields while preserving formatted calculated output

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