Problem/Motivation
The JavaScript validation code uses innerHTML to insert an error message. While the content comes from Drupal.t() which should be safe, using innerHTML is generally discouraged as it can introduce XSS vulnerabilities if the translation system is ever compromised or if translation strings include dynamic content.
Location: js/primary-selection.js, line 223
Current Code
errorContainer.innerHTML = `<div class="messages__content">${Drupal.t( 'Please select a primary item before saving.', )}</div>`;
Risk Assessment
- Current Risk: Very low - The string is static and translated through
Drupal.t() - Future Risk: Could become an issue if:
- Translation strings ever include user-provided content
- The translation system is compromised
- Code is copied/modified without understanding the security implications
- Best Practice: Avoid
innerHTMLwhentextContentor DOM manipulation suffices
Proposed resolution
Replace innerHTML usage with proper DOM element creation using textContent. This follows security best practices and prevents any possibility of XSS injection.
Recommended Implementation
// Create the message content element const messageContent = document.createElement('div'); messageContent.className = 'messages__content'; messageContent.textContent = Drupal.t( 'Please select a primary item before saving.', ); // Clear and append to container errorContainer.innerHTML = ''; errorContainer.appendChild(messageContent);
Alternative Approaches
Option 1: Use textContent directly on container (simpler but loses wrapper div)
errorContainer.textContent = Drupal.t( 'Please select a primary item before saving.', );
Option 2: Use createTextNode (more explicit but more verbose)
const messageContent = document.createElement('div'); messageContent.className = 'messages__content'; messageContent.appendChild( document.createTextNode(Drupal.t('Please select a primary item before saving.')) ); errorContainer.innerHTML = ''; errorContainer.appendChild(messageContent);
The recommended implementation (using textContent with element creation) provides the best balance of security, readability, and maintaining the expected HTML structure.
Remaining tasks
- Replace
innerHTMLwith DOM element creation andtextContent - Test the error message display still works correctly
- Test the error message styling is preserved (CSS should still apply)
- Review other JavaScript files for similar
innerHTMLusage - Consider adding ESLint rule to prevent future
innerHTMLusage - Update JavaScript coding standards documentation if needed
User interface changes
None. The error message will look identical to users. This is an internal implementation change that maintains the same visual output.
API changes
None
Data model changes
None
Issue fork primary_entity_reference-3570439
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
bluegeek9 commented