From f7be13ebdfa8021475fd451f76b0cb036b3447c2 Mon Sep 17 00:00:00 2001 From: mgutt <10757176+mgutt@users.noreply.github.com> Date: Thu, 15 Jan 2026 22:39:20 +0100 Subject: [PATCH 1/3] Fix #2514: Complete migration to JSON API for file_manager operations This PR completes the migration from HTML string responses to a unified JSON API format for all file_manager operations, building on the work started in PR #2429. Backend changes (nchan/file_manager): - Migrate remaining actions to JSON format: * case 0 (create folder): {action: 0, text: ['Creating...']} * case 2/7 (rename): {action: 2/7, text: ['Renaming...']} * case 11 (chown): {action: 11, text: ['Updating... filename']} * case 12 (chmod): {action: 12, text: ['Updating... filename']} * case 15 (search): {action: 15, text: ['Searching... count']} * completion: {action: X, text: ['Done']} for all non-search operations - Convert search results to JSON structure: * {action: 15, results: [{location, path}, ...]} * Replaces legacy #cat# string format with structured data - Convert cat() function to return array of objects instead of string - Send errors as plain text (not HTML) for safer frontend handling Frontend changes (BrowseButton.page): - Implement unified JSON parser for all status updates - Show footer only for progress-tracking operations (delete, copy, move, search) - No footer for quick operations (create, rename, chown, chmod) - Parse search results from JSON structure: * Display as properly paired dt/dd elements * Clear previous results before showing new ones * Show 'No files found' for empty results - Remove all legacy string parsing code - Use .text() with white-space:pre-line for error display (prevents XSS) - Add error logging for JSON parse failures Frontend changes (Browse.page): - Add try-catch for Control.php JSON parsing - Handle empty responses gracefully All file_manager operations now use a consistent, type-safe JSON API with proper error handling and no legacy code paths remaining. --- emhttp/plugins/dynamix/Browse.page | 18 +++-- emhttp/plugins/dynamix/BrowseButton.page | 80 ++++++++++------------- emhttp/plugins/dynamix/nchan/file_manager | 77 ++++++++++++++++++---- 3 files changed, 115 insertions(+), 60 deletions(-) diff --git a/emhttp/plugins/dynamix/Browse.page b/emhttp/plugins/dynamix/Browse.page index 59802b441..f52893e8b 100644 --- a/emhttp/plugins/dynamix/Browse.page +++ b/emhttp/plugins/dynamix/Browse.page @@ -874,8 +874,13 @@ function doAction(action, title, id) { dfm_fileManager('start'); $.post('/webGui/include/Control.php',{mode:'file',action:action,title:encodeURIComponent(title),source:encodeURIComponent(source),target:encodeURIComponent(target),hdlink:hdlink,sparse:dfm.window.find('#dfm_sparse').val(),exist:dfm.window.find('#dfm_exist').val(),zfs:encodeURIComponent(zfs)},function(){ $.post('/webGui/include/Control.php',{mode:'read'},function(data){ - dfm_read = JSON.parse(data); - dfm_read.action = parseInt(dfm_read.action); + try { + dfm_read = data ? JSON.parse(data) : {}; + dfm_read.action = parseInt(dfm_read.action); + } catch(e) { + console.error('Failed to parse Control.php response:', e, data); + dfm_read = {}; + } }); }); }, @@ -1158,8 +1163,13 @@ function doActions(action, title) { dfm_fileManager('start'); $.post('/webGui/include/Control.php',{mode:'file',action:action,title:encodeURIComponent(title),source:encodeURIComponent(source.join('\r')),target:encodeURIComponent(target),hdlink:hdlink,sparse:dfm.window.find('#dfm_sparse').val(),exist:dfm.window.find('#dfm_exist').val(),zfs:encodeURIComponent(zfs.join('\r'))},function(){ $.post('/webGui/include/Control.php',{mode:'read'},function(data){ - dfm_read = JSON.parse(data); - dfm_read.action = parseInt(dfm_read.action); + try { + dfm_read = data ? JSON.parse(data) : {}; + dfm_read.action = parseInt(dfm_read.action); + } catch(e) { + console.error('Failed to parse Control.php response:', e, data); + dfm_read = {}; + } }); }); }, diff --git a/emhttp/plugins/dynamix/BrowseButton.page b/emhttp/plugins/dynamix/BrowseButton.page index f5d421451..6aee966d0 100644 --- a/emhttp/plugins/dynamix/BrowseButton.page +++ b/emhttp/plugins/dynamix/BrowseButton.page @@ -152,6 +152,29 @@ function dfm_showProgress(data) { try { let parsed = JSON.parse(data); + // Handle search results (action 15 with results array) + if (parsed.action === 15 && Array.isArray(parsed.results)) { + if (parsed.results.length > 0) { + // Build HTML as pairs of location/path elements + let html = ''; + for (let result of parsed.results) { + html += '
' + dfm_escapeHTML(result.location) + '
'; + html += '
' + dfm_escapeHTML(result.path.dfm_wedge(80)) + '
'; + } + // Remove all existing result dt/dd pairs (keep only the first 2 dt/dd which are the form fields) + let $dl = dfm.window.find('dl'); + $dl.find('dt:gt(1), dd:gt(1)').remove(); + // Append the new results + $dl.append(html); + dfm.window.find('#dfm_files').html(parsed.results.length+" "+"_(files)_"); + } else { + dfm.window.find('.dfm_loc').html(''); + dfm.window.find('.dfm_text').html('_(No files found)_'); + dfm.window.find('#dfm_files').html('0 '+"_(files)_"); + } + return parsed.results.length; + } + // Universal JSON format: {action: int, text: [text0, text1]} // text[0] = file/main text, text[1] = progress info (optional) if (parsed.action !== undefined && Array.isArray(parsed.text)) { @@ -212,54 +235,23 @@ function dfm_showProgress(data) { } // Build footer text (progress info only) - let footerText = parsed.text[1] || parsed.text[0] || ''; - dfm_footer('write', footerText, $icon); - dfm.previous = footerText; + // Show footer for actions with actual progress: delete (1,6), copy (3,8), move (4,9), search (15) + // No footer for quick operations: create (0), rename (2,7), chown (11), chmod (12) + let showFooter = [1, 3, 4, 6, 8, 9, 15].includes(parsed.action); + if (showFooter) { + let footerText = parsed.text[1] || parsed.text[0] || ''; + dfm_footer('write', footerText, $icon); + dfm.previous = footerText; + } + return 0; } } catch(e) { - // Not JSON, fallback to old string parsing - } - - // Legacy string parsing for non-migrated operations - let file = null; - let text = data.split('\n'); - let line = text[0].split('... '); - let strict = /^mnt|^boot/; - let footer = false; - if (text[0] == '#cat#') { - let loc = [], cat = []; - for (let i=1,row; row=text[i]; i++) { - if (!row) continue; - row = row.split('\0'); - loc.push(row[0]); - cat.push(row[1].dfm_wedge(80)); - } - if (cat.length > 0) { - dfm.window.find('.dfm_loc').html(loc.join('
')); - dfm.window.find('.dfm_text').html(cat.join('
')); - dfm.window.find('#dfm_files').html(loc.length+" "+"_(files)_"); - } - return cat.length; - } else if (text.length == 1) { - text = text[0].dfm_wedge(80); - footer = text.indexOf("_(Searching)_") != -1; - } else { - if (strict.test(text[1])) { - file = text[1]; - text = dfm.previous; - } else { - file = line[1]; - text = text[1].split(/\s+/); - text = "_(Completed)_: "+text[1]+",  _(Speed)_: "+text[2]+",  _(ETA)_: "+text[3]; - dfm.previous = text; - footer = true; - } + // Not JSON, log error + console.error('Failed to parse status as JSON:', e, data); + return 0; } - if (file == null || strict.test(file)) dfm.window.find('.dfm_text').html((file?line[0]+'... /'+dfm_escapeHTML(file.dfm_wedge())+'
':'')+text); - if (footer) dfm_footer('write',text); - return 0; } function dfm_fileManager(action) { @@ -420,7 +412,7 @@ nchan_filemanager.on('message', function(msg) { let data = $.parseJSON(msg); if (data.error) { dfm_fileManager('stop'); - dfm.window.find('.dfm_text').addClass('orange-text').html(data.error); + dfm.window.find('.dfm_text').addClass('orange-text').css('white-space', 'pre-line').text(data.error); dfm.window.find('#dfm_target').prop('disabled',false); dfm.window.find('#dfm_sparse').prop('disabled',false); dfm.window.find('#dfm_exist').prop('disabled',false); diff --git a/emhttp/plugins/dynamix/nchan/file_manager b/emhttp/plugins/dynamix/nchan/file_manager index 8ada3fedd..f87750856 100755 --- a/emhttp/plugins/dynamix/nchan/file_manager +++ b/emhttp/plugins/dynamix/nchan/file_manager @@ -297,7 +297,8 @@ function update_translation($locale) { function cat($file) { global $null; - $cat = $set = []; + $results = []; + $set = []; $rows = file($file, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES); if (count($rows) > 0) { natcasesort($rows); @@ -313,12 +314,20 @@ function cat($file) { } foreach (array_diff($rows, $user) as $row) { [$none, $root, $main] = explode('/',$row,4); - $cat[] = ($root == 'mnt' ? $main : ($root == 'boot' ? 'flash' : '---'))."\0".$row; + $results[] = [ + 'location' => ($root == 'mnt' ? $main : ($root == 'boot' ? 'flash' : '---')), + 'path' => $row + ]; } $i = 0; - foreach ($user as $row) $cat[] = $set[++$i]."\0".$row; + foreach ($user as $row) { + $results[] = [ + 'location' => $set[++$i], + 'path' => $row + ]; + } } - return "#cat#\n".implode("\n",$cat)."\n"; + return $results; } function escape($name) {return escapeshellarg(validname($name));} @@ -388,8 +397,15 @@ while (true) { $source = explode("\r", $source); switch ($action) { case 0: // create folder + + // return status of running action if (!empty($pid)) { - $reply['status'] = ''._('Creating').'...'; + $reply['status'] = json_encode([ + 'action' => $action, + 'text' => [_('Creating').'...'] + ]); + + // start action } else { $dir = $source[0].'/'.$target; exec("mkdir -pm0777 ".quoted($dir)." 1>$null 2>$error & echo $!", $pid); @@ -413,8 +429,15 @@ while (true) { break; case 2: // rename folder case 7: // rename file + + // return status of running action if (!empty($pid)) { - $reply['status'] = ''._('Renaming').'...'; + $reply['status'] = json_encode([ + 'action' => $action, + 'text' => [_('Renaming').'...'] + ]); + + // start action } else { $path = dirname($source[0]); exec("mv -f ".quoted($source)." ".quoted("$path/$target")." 1>$null 2>$error & echo \$!", $pid); @@ -605,22 +628,46 @@ while (true) { } break; case 11: // change owner + + // return status of running action if (!empty($pid)) { - $reply['status'] = ''._('Updating').'... '.exec("tail -2 $status | grep -Pom1 \"^.+ of '\\K[^']+\""); + $file_line = exec("tail -2 $status | grep -Pom1 \"^.+ of '\\K[^']+\""); + $reply['status'] = json_encode([ + 'action' => $action, + 'text' => [_('Updating').'... '.htmlspecialchars(mb_strimhalf($file_line, 70, '...'), ENT_QUOTES, 'UTF-8')] + ]); + + // start action } else { exec("chown -Rfv $target ".quoted($source)." 1>$status 2>$error & echo \$!", $pid); } break; case 12: // change permission + + // return status of running action if (!empty($pid)) { - $reply['status'] = ''._('Updating').'... '.exec("tail -2 $status | grep -Pom1 \"^.+ of '\\K[^']+\""); + $file_line = exec("tail -2 $status | grep -Pom1 \"^.+ of '\\K[^']+\""); + $reply['status'] = json_encode([ + 'action' => $action, + 'text' => [_('Updating').'... '.htmlspecialchars(mb_strimhalf($file_line, 70, '...'), ENT_QUOTES, 'UTF-8')] + ]); + + // start action } else { exec("chmod -Rfv $target ".quoted($source)." 1>$status 2>$error & echo \$!", $pid); } break; case 15: // search + + // return status of running action if (!empty($pid)) { - $reply['status'] = ''._('Searching').'... '.exec("wc -l $status | grep -Pom1 '^[0-9]+'"); + $count = exec("wc -l $status | grep -Pom1 '^[0-9]+'"); + $reply['status'] = json_encode([ + 'action' => $action, + 'text' => [_('Searching').'... '.$count] + ]); + + // start action } else { exec("find ".source($source)." -iname ".escapeshellarg($target)." 1>$status 2>$null & echo \$!", $pid); } @@ -648,10 +695,16 @@ while (true) { $pid = pid_exists($pid); } else { if ($action != 15) { - $reply['status'] = _('Done'); + $reply['status'] = json_encode([ + 'action' => $action, + 'text' => [_('Done')] + ]); $reply['done'] = 1; } else { - $reply['status'] = cat($status); + $reply['status'] = json_encode([ + 'action' => $action, + 'results' => cat($status) + ]); $reply['done'] = 2; } if ($zfs) { @@ -662,7 +715,7 @@ while (true) { foreach ($datasets as $dataset) if (exec("ls --indicator-style=none /mnt/$dataset|wc -l")==0) exec("zfs destroy $dataset 2>/dev/null"); } } - if (file_exists($error)) $reply['error'] = str_replace("\n","
", trim(file_get_contents($error))); + if (file_exists($error)) $reply['error'] = trim(file_get_contents($error)); delete_file($active, $pid_file, $status, $error); unset($pid); $delete_empty_dirs = null; From acf0e64bc9b2f796608325f4b93a74b849faf65e Mon Sep 17 00:00:00 2001 From: mgutt <10757176+mgutt@users.noreply.github.com> Date: Thu, 15 Jan 2026 22:55:31 +0100 Subject: [PATCH 2/3] Address code review feedback - Guard against undefined array access in cat() using null coalescing operator - Clear stale 'Searching...' text after search results render --- emhttp/plugins/dynamix/BrowseButton.page | 2 ++ emhttp/plugins/dynamix/nchan/file_manager | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/emhttp/plugins/dynamix/BrowseButton.page b/emhttp/plugins/dynamix/BrowseButton.page index 6aee966d0..8ba348fdc 100644 --- a/emhttp/plugins/dynamix/BrowseButton.page +++ b/emhttp/plugins/dynamix/BrowseButton.page @@ -167,6 +167,8 @@ function dfm_showProgress(data) { // Append the new results $dl.append(html); dfm.window.find('#dfm_files').html(parsed.results.length+" "+"_(files)_"); + // Clear stale progress text + dfm.window.find('.dfm_text').html(''); } else { dfm.window.find('.dfm_loc').html(''); dfm.window.find('.dfm_text').html('_(No files found)_'); diff --git a/emhttp/plugins/dynamix/nchan/file_manager b/emhttp/plugins/dynamix/nchan/file_manager index f87750856..73e18a4ae 100755 --- a/emhttp/plugins/dynamix/nchan/file_manager +++ b/emhttp/plugins/dynamix/nchan/file_manager @@ -322,7 +322,7 @@ function cat($file) { $i = 0; foreach ($user as $row) { $results[] = [ - 'location' => $set[++$i], + 'location' => $set[++$i] ?? '---', 'path' => $row ]; } From f5d032fe46bf71814d857b97e201db755fc7da59 Mon Sep 17 00:00:00 2001 From: mgutt <10757176+mgutt@users.noreply.github.com> Date: Thu, 15 Jan 2026 23:01:41 +0100 Subject: [PATCH 3/3] Remove redundant htmlspecialchars() calls Since the frontend uses createTextNode() which is already XSS-safe and doesn't decode HTML entities, the backend htmlspecialchars() encoding is unnecessary and causes literal entity codes to appear in the UI. --- emhttp/plugins/dynamix/nchan/file_manager | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/emhttp/plugins/dynamix/nchan/file_manager b/emhttp/plugins/dynamix/nchan/file_manager index 73e18a4ae..3d1f4e762 100755 --- a/emhttp/plugins/dynamix/nchan/file_manager +++ b/emhttp/plugins/dynamix/nchan/file_manager @@ -259,7 +259,7 @@ function parse_rsync_progress($status, $action_label, $reset = false) { // note: -v is used to obtain the opposite of the progress line regex and to filter out empty lines $file_line = exec("tac $status | grep -m1 -v -E '(^\$|^ .*[0-9]+%.*[0-9]+:[0-9]+:[0-9]+)' | tr -s ' ' || echo ''"); if ($file_line) { - $text[0] .= htmlspecialchars(mb_strimhalf($file_line, 70, '...'), ENT_QUOTES, 'UTF-8'); + $text[0] .= mb_strimhalf($file_line, 70, '...'); } return $text; @@ -419,7 +419,7 @@ while (true) { if (!empty($pid)) { $reply['status'] = json_encode([ 'action' => $action, - 'text' => [htmlspecialchars(mb_strimhalf(exec("tail -1 $status"), 70, '...'), ENT_QUOTES, 'UTF-8')] + 'text' => [mb_strimhalf(exec("tail -1 $status"), 70, '...')] ]); // start action @@ -486,7 +486,7 @@ while (true) { if ($delete_empty_dirs === false) { $reply['status'] = json_encode([ 'action' => $action, - 'text' => [htmlspecialchars(mb_strimhalf(exec("tail -1 $status"), 70, '...'), ENT_QUOTES, 'UTF-8')] + 'text' => [mb_strimhalf(exec("tail -1 $status"), 70, '...')] ]); // moving: progress @@ -634,7 +634,7 @@ while (true) { $file_line = exec("tail -2 $status | grep -Pom1 \"^.+ of '\\K[^']+\""); $reply['status'] = json_encode([ 'action' => $action, - 'text' => [_('Updating').'... '.htmlspecialchars(mb_strimhalf($file_line, 70, '...'), ENT_QUOTES, 'UTF-8')] + 'text' => [_('Updating').'... '.mb_strimhalf($file_line, 70, '...')] ]); // start action @@ -649,7 +649,7 @@ while (true) { $file_line = exec("tail -2 $status | grep -Pom1 \"^.+ of '\\K[^']+\""); $reply['status'] = json_encode([ 'action' => $action, - 'text' => [_('Updating').'... '.htmlspecialchars(mb_strimhalf($file_line, 70, '...'), ENT_QUOTES, 'UTF-8')] + 'text' => [_('Updating').'... '.mb_strimhalf($file_line, 70, '...')] ]); // start action