Skip to content

Conversation

@iamibrahimriaz
Copy link
Collaborator

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Security fix
  • Improvement
  • New Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Text changes
  • Other... Please describe:

Description

When changes directory type, the search form markup is replaced via AJAX. The original code bound events directly to elements that existed at initialization.

Before:

  • Events were bound directly: $(field.input_elm).on('keyup', ...)
  • When the DOM was replaced, the old elements (and their handlers) were removed
  • New elements had no handlers, so the AJAX didn't trigger

After (a directory type change):

  • The form markup is replaced
  • New location input fields are added
  • The delegated handler on body catches keyup events on the new inputs
  • The OpenStreetMap AJAX request triggers correctly

Any linked issues

Fixes Taiga 72316

Checklist

@Armanul46 Armanul46 added this to the v8.5.8 milestone Jan 18, 2026
Copy link
Member

@RabbiIslamRony RabbiIslamRony left a comment

Choose a reason for hiding this comment

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

🚨 Critical Issues Found

1. Syntax Bug - String Concatenation (Lines 1776-1785)

Problem: Incorrect comma operator usage causing broken HTML output

// Current (BROKEN):
((res +=
    '<li><a href="#" data-lat=' +
    data[i].lat +
    ' data-lon=' +
    data[i].lon +
    '>' +
    locationIconHTML +
    "<span class='location-address'>" +
    data[i].display_name),
    +'</span></a></li>');

Impact: Location suggestions may not render correctly

Fix Required:

// Should be:
res +=
    '<li><a href="#" data-lat="' +
    data[i].lat +
    '" data-lon="' +
    data[i].lon +
    '">' +
    locationIconHTML +
    "<span class='location-address'>" +
    data[i].display_name +
    '</span></a></li>';

2. Security - XSS Vulnerability (Line 1784)

Problem: data[i].display_name inserted directly into HTML without escaping

Impact: Potential XSS attack if API returns malicious content

Fix Required:

// Add HTML escaping function
function escapeHtml(text) {
    const div = document.createElement('div');
    div.textContent = text;
    return div.innerHTML;
}

// Apply to all user data:
res +=
    '<li><a href="#" data-lat="' +
    escapeHtml(String(data[i].lat)) +
    '" data-lon="' +
    escapeHtml(String(data[i].lon)) +
    '">' +
    locationIconHTML +
    "<span class='location-address'>" +
    escapeHtml(data[i].display_name) +
    '</span></a></li>';

3. URL Encoding Issue (Line 1736)

Problem: Incorrect URL construction using %27+ pattern instead of proper encoding

// Current:
url: 'https://nominatim.openstreetmap.org/?q=%27+'.concat(search, '+%27&format=json')

Impact: Special characters in search queries may break API requests

Fix Required:

// Should be:
url: 'https://nominatim.openstreetmap.org/?q=' + encodeURIComponent(search) + '&format=json&limit=5'

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.

3 participants