fix: tag urls getting double escaped (#13306)

* fix: tag urls getting double escaped

get rid of weird decodeURIComponent($.param()) usage
$.param returns a string suitable for use in query param string

* add a new test
This commit is contained in:
Barış Soner Uşaklı
2025-04-07 13:23:22 -04:00
committed by GitHub
parent bf243e0778
commit 3526c937cc
14 changed files with 24 additions and 22 deletions

View File

@@ -620,7 +620,7 @@ define('admin/manage/users', [
params.query = query.query; params.query = query.query;
params.page = query.page; params.page = query.page;
params.sortBy = params.sortBy || 'lastonline'; params.sortBy = params.sortBy || 'lastonline';
const qs = decodeURIComponent($.param(params)); const qs = $.param(params);
$.get(config.relative_path + '/api/admin/manage/users?' + qs, function (data) { $.get(config.relative_path + '/api/admin/manage/users?' + qs, function (data) {
renderSearchResults(data); renderSearchResults(data);
const url = config.relative_path + '/admin/manage/users?' + qs; const url = config.relative_path + '/admin/manage/users?' + qs;
@@ -673,7 +673,7 @@ define('admin/manage/users', [
delete params.searchBy; delete params.searchBy;
} }
return decodeURIComponent($.param(params)); return $.param(params);
} }
function handleSort() { function handleSort() {

View File

@@ -59,7 +59,7 @@ define('forum/login', ['hooks', 'translator', 'jquery-form'], function (hooks, t
const params = utils.params({ url: data.next }); const params = utils.params({ url: data.next });
params.loggedin = true; params.loggedin = true;
delete params.register; // clear register message incase it exists delete params.register; // clear register message incase it exists
const qs = decodeURIComponent($.param(params)); const qs = $.param(params);
window.location.href = pathname + '?' + qs; window.location.href = pathname + '?' + qs;
}, },

View File

@@ -85,7 +85,7 @@ define('forum/register', [
const params = utils.params({ url: data.next }); const params = utils.params({ url: data.next });
params.registered = true; params.registered = true;
const qs = decodeURIComponent($.param(params)); const qs = $.param(params);
window.location.href = pathname + '?' + qs; window.location.href = pathname + '?' + qs;
} else if (data.message) { } else if (data.message) {

View File

@@ -72,12 +72,12 @@ define('forum/search', [
let labelText = '[[search:tags]]'; let labelText = '[[search:tags]]';
if (selectedTags.length) { if (selectedTags.length) {
labelText = translator.compile( labelText = translator.compile(
'search:tags-x', selectedTags.map(u => u.valueEscaped).join(', ') 'search:tags-x', selectedTags.map(u => u.value).join(', ')
); );
} }
$('[component="tag/filter/button"]').toggleClass( $('[component="tag/filter/button"]').toggleClass(
'active-filter', isActive 'active-filter', isActive
).find('.filter-label').translateText(labelText); ).find('.filter-label').translateHtml(labelText);
} }
function updateTimeFilter() { function updateTimeFilter() {
@@ -326,7 +326,7 @@ define('forum/search', [
return { return {
value: value, value: value,
valueEscaped: escapedTag, valueEscaped: escapedTag,
valueEncoded: encodeURIComponent(escapedTag), valueEncoded: encodeURIComponent(value),
class: escapedTag.replace(/\s/g, '-'), class: escapedTag.replace(/\s/g, '-'),
}; };
} }
@@ -353,7 +353,7 @@ define('forum/search', [
result.tags = result.tags.slice(0, 20); result.tags = result.tags.slice(0, 20);
const tagMap = {}; const tagMap = {};
result.tags.forEach((tag) => { result.tags.forEach((tag) => {
tagMap[tag.valueEscaped] = tag; tagMap[tag.value] = tag;
}); });
const html = await app.parseAndTranslate('partials/search-filters', 'tagFilterResults', { const html = await app.parseAndTranslate('partials/search-filters', 'tagFilterResults', {
@@ -374,7 +374,7 @@ define('forum/search', [
el.on('click', '[component="tag/filter/delete"]', function () { el.on('click', '[component="tag/filter/delete"]', function () {
const deleteTag = $(this).attr('data-tag'); const deleteTag = $(this).attr('data-tag');
selectedTags = selectedTags.filter(tag => tag.valueEscaped !== deleteTag); selectedTags = selectedTags.filter(tag => tag.value !== deleteTag);
renderSelectedTags(); renderSelectedTags();
}); });

View File

@@ -55,7 +55,7 @@ define('categoryFilter', ['categorySearch', 'api', 'hooks'], function (categoryS
delete currentParams.page; delete currentParams.page;
if (Object.keys(currentParams).length) { if (Object.keys(currentParams).length) {
url += '?' + decodeURIComponent($.param(currentParams)); url += '?' + $.param(currentParams);
} }
ajaxify.go(url); ajaxify.go(url);
} }

View File

@@ -286,7 +286,7 @@ define('search', [
data: data, data: data,
}); });
return decodeURIComponent($.param(query)); return $.param(query);
} }
Search.getSearchPreferences = function () { Search.getSearchPreferences = function () {

View File

@@ -17,7 +17,7 @@ define('sort', ['components'], function (components) {
const newSetting = $(this).attr('data-sort'); const newSetting = $(this).attr('data-sort');
const urlParams = utils.params(); const urlParams = utils.params();
urlParams.sort = newSetting; urlParams.sort = newSetting;
const qs = decodeURIComponent($.param(urlParams)); const qs = $.param(urlParams);
ajaxify.go(gotoOnSave + (qs ? '?' + qs : '')); ajaxify.go(gotoOnSave + (qs ? '?' + qs : ''));
}); });
}; };

View File

@@ -98,7 +98,7 @@ define('tagFilter', ['hooks', 'alerts', 'bootstrap'], function (hooks, alerts, b
} }
delete currentParams.page; delete currentParams.page;
if (Object.keys(currentParams).length) { if (Object.keys(currentParams).length) {
url += '?' + decodeURIComponent($.param(currentParams)); url += '?' + $.param(currentParams);
} }
ajaxify.go(url); ajaxify.go(url);
} }
@@ -159,7 +159,7 @@ define('tagFilter', ['hooks', 'alerts', 'bootstrap'], function (hooks, alerts, b
function renderList(tags) { function renderList(tags) {
const selectedTags = options.selectedTags; const selectedTags = options.selectedTags;
tags.forEach(function (tag) { tags.forEach(function (tag) {
tag.selected = selectedTags.includes(tag.valueEscaped); tag.selected = selectedTags.includes(tag.value);
}); });
app.parseAndTranslate(options.template, { app.parseAndTranslate(options.template, {

View File

@@ -6,7 +6,7 @@ window.overrides = window.overrides || {};
function translate(elements, type, str) { function translate(elements, type, str) {
return elements.each(function () { return elements.each(function () {
var el = $(this); const el = $(this);
translator.translate(str, function (translated) { translator.translate(str, function (translated) {
el[type](translated); el[type](translated);
}); });

View File

@@ -362,11 +362,11 @@ helpers.getSelectedTag = function (tags) {
tags = [tags]; tags = [tags];
} }
tags = tags || []; tags = tags || [];
const tagData = tags.map(t => validator.escape(String(t))); const tagData = tags.map(t => String(t));
let selectedTag = null; let selectedTag = null;
if (tagData.length) { if (tagData.length) {
selectedTag = { selectedTag = {
label: tagData.join(', '), label: validator.escape(tagData.join(', ')),
}; };
} }
return { return {

View File

@@ -41,8 +41,8 @@ tagsController.getTag = async function (req, res) {
const stop = start + settings.topicsPerPage - 1; const stop = start + settings.topicsPerPage - 1;
const [topicCount, tids] = await Promise.all([ const [topicCount, tids] = await Promise.all([
topics.getTagTopicCount(tag, cids), topics.getTagTopicCount(req.params.tag, cids),
topics.getTagTidsByCids(tag, cids, start, stop), topics.getTagTidsByCids(req.params.tag, cids, start, stop),
]); ]);
templateData.topics = await topics.getTopics(tids, req.uid); templateData.topics = await topics.getTopics(tids, req.uid);

View File

@@ -134,7 +134,7 @@ function modifyTopic(topic, fields) {
return { return {
value: tag, value: tag,
valueEscaped: escaped, valueEscaped: escaped,
valueEncoded: encodeURIComponent(escaped), valueEncoded: encodeURIComponent(tag),
class: escaped.replace(/\s/g, '-'), class: escaped.replace(/\s/g, '-'),
}; };
}); });

View File

@@ -323,7 +323,7 @@ module.exports = function (Topics) {
} }
tags.forEach((tag) => { tags.forEach((tag) => {
tag.valueEscaped = validator.escape(String(tag.value)); tag.valueEscaped = validator.escape(String(tag.value));
tag.valueEncoded = encodeURIComponent(tag.valueEscaped); tag.valueEncoded = encodeURIComponent(tag.value);
tag.class = tag.valueEscaped.replace(/\s/g, '-'); tag.class = tag.valueEscaped.replace(/\s/g, '-');
}); });
return tags; return tags;

View File

@@ -1432,6 +1432,7 @@ describe('Topic\'s', () => {
before(async () => { before(async () => {
await topics.post({ uid: adminUid, tags: ['php', 'nosql', 'psql', 'nodebb', 'node icon'], title: 'topic title 1', content: 'topic 1 content', cid: topic.categoryId }); await topics.post({ uid: adminUid, tags: ['php', 'nosql', 'psql', 'nodebb', 'node icon'], title: 'topic title 1', content: 'topic 1 content', cid: topic.categoryId });
await topics.post({ uid: adminUid, tags: ['javascript', 'mysql', 'python', 'nodejs'], title: 'topic title 2', content: 'topic 2 content', cid: topic.categoryId }); await topics.post({ uid: adminUid, tags: ['javascript', 'mysql', 'python', 'nodejs'], title: 'topic title 2', content: 'topic 2 content', cid: topic.categoryId });
await topics.post({ uid: adminUid, tags: ['signal & slot', 'node & c++'], title: 'topic title 3', content: 'topic 3 content', cid: topic.categoryId });
}); });
it('should return empty array if query is falsy', (done) => { it('should return empty array if query is falsy', (done) => {
@@ -1483,10 +1484,11 @@ describe('Topic\'s', () => {
it('should search and load tags', (done) => { it('should search and load tags', (done) => {
socketTopics.searchAndLoadTags({ uid: adminUid }, { query: 'no' }, (err, data) => { socketTopics.searchAndLoadTags({ uid: adminUid }, { query: 'no' }, (err, data) => {
assert.ifError(err); assert.ifError(err);
assert.equal(data.matchCount, 4); assert.equal(data.matchCount, 5);
assert.equal(data.pageCount, 1); assert.equal(data.pageCount, 1);
const tagData = [ const tagData = [
{ value: 'nodebb', valueEscaped: 'nodebb', valueEncoded: 'nodebb', score: 3, class: 'nodebb' }, { value: 'nodebb', valueEscaped: 'nodebb', valueEncoded: 'nodebb', score: 3, class: 'nodebb' },
{ value: 'node & c++', valueEscaped: 'node & c++', valueEncoded: 'node%20%26%20c%2B%2B', score: 1, class: 'node-&-c++' },
{ value: 'node icon', valueEscaped: 'node icon', valueEncoded: 'node%20icon', score: 1, class: 'node-icon' }, { value: 'node icon', valueEscaped: 'node icon', valueEncoded: 'node%20icon', score: 1, class: 'node-icon' },
{ value: 'nodejs', valueEscaped: 'nodejs', valueEncoded: 'nodejs', score: 1, class: 'nodejs' }, { value: 'nodejs', valueEscaped: 'nodejs', valueEncoded: 'nodejs', score: 1, class: 'nodejs' },
{ value: 'nosql', valueEscaped: 'nosql', valueEncoded: 'nosql', score: 1, class: 'nosql' }, { value: 'nosql', valueEscaped: 'nosql', valueEncoded: 'nosql', score: 1, class: 'nosql' },