Skip to content
GitLab
Projects Groups Topics Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
  • M mjwshared
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributor statistics
    • Graph
    • Compare revisions
  • Issues 4
    • Issues 4
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 2
    • Merge requests 2
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • Extensions
  • mjwshared
  • Issues
  • #10
Closed
Open
Issue created Feb 14, 2022 by elilisseck@elilisseckContributor

".closest('form').prop('id')" problematic if form has an 'id' element

If a customization, extension, configuration, etc introduces an input with the name or id "id", the .prop() will incorrectly grab that element instead of the form and have a bad ID, breaking Stripe processing. For example we have a shim extension to add a hidden "id" element to alleviate certain session issues that have been cropping up lately by passing event ID from register to confirm, and if someone picked "id" as their honeypot field it would also break Stripe.

This situation doesn't appear in CiviCRM Core, but it could in the future.

Problematic line: https://lab.civicrm.org/extensions/mjwshared/-/blob/master/js/crm.payment.js#L129

Considering the console in the attached example, I believe .attr() will work here instead. Do you see issues with that approach?

test-with-name-and-id.html

Potential MR if this makes sense and your testing also doesn't reveal other issues: !28 (merged)

Edited Feb 14, 2022 by elilisseck
Assignee
Assign to
Time tracking