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 innerHTML when textContent or 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 innerHTML with DOM element creation and textContent
  • 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 innerHTML usage
  • Consider adding ESLint rule to prevent future innerHTML usage
  • 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

Command icon 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

bluegeek9 created an issue. See original summary.

  • bluegeek9 committed 5ece9448 on 1.0.x
    feat: #3570439 Replace innerHTML with textContent for Error Message...
bluegeek9’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.