[PATCH 1/7] pakfire.cgi: Separate command processing and HTML generation

[PATCH 1/7] pakfire.cgi: Separate command processing and HTML generation

[Leo-Andres Hofmann]

[-- Attachment #1: Type: text/plain, Size: 4818 bytes --]

Move most of the command execution away from the HTML output.
This makes it easier to modify or extend individual commands.

Also load Pakfire settings earlier to ensure that they are
available during command execution.

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/pakfire.cgi | 61 +++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
index 65c67fb90..b908ec6a0 100644
--- a/html/cgi-bin/pakfire.cgi
+++ b/html/cgi-bin/pakfire.cgi
@@ -39,11 +39,12 @@ my %mainsettings = ();
 
 # Load general settings
 &General::readhash("${General::swroot}/main/settings", \%mainsettings);
+&General::readhash("${General::swroot}/pakfire/settings", \%pakfiresettings);
 &General::readhash("/srv/web/ipfire/html/themes/ipfire/include/colors.txt", \%color);
 
 # Get CGI request data
 $cgiparams{'ACTION'} = '';
-$cgiparams{'VALID'} = '';
+$cgiparams{'FORCE'} = '';
 
 $cgiparams{'INSPAKS'} = '';
 $cgiparams{'DELPAKS'} = '';
@@ -94,6 +95,35 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') {
 	exit;
 }
 
+### Process Pakfire install/update commands ###
+if(($cgiparams{'ACTION'} ne '') && (! &_is_pakfire_busy())) {
+	if(($cgiparams{'ACTION'} eq 'install') && ($cgiparams{'FORCE'} eq 'on')) {
+		my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
+		&General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive", "--no-colors", @pkgs);
+	} elsif(($cgiparams{'ACTION'} eq 'remove') && ($cgiparams{'FORCE'} eq 'on')) {
+		my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
+		&General::system_background("/usr/local/bin/pakfire", "remove", "--non-interactive", "--no-colors", @pkgs);
+	} elsif($cgiparams{'ACTION'} eq 'update') {
+		&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
+	} elsif($cgiparams{'ACTION'} eq 'upgrade') {
+		&General::system_background("/usr/local/bin/pakfire", "upgrade", "-y", "--no-colors");
+	} elsif($cgiparams{'ACTION'} eq $Lang::tr{'save'}) {
+		$pakfiresettings{"TREE"} = $cgiparams{"TREE"};
+
+		# Check for valid input
+		if ($pakfiresettings{"TREE"} !~ m/^(stable|testing|unstable)$/) {
+			$errormessage .= $Lang::tr{'pakfire invalid tree'};
+		}
+
+		unless ($errormessage) {
+			&General::writehash("${General::swroot}/pakfire/settings", \%pakfiresettings);
+
+			# Update lists
+			&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
+		}
+	}
+}
+
 ### Start pakfire page ###
 &Header::showhttpheaders();
 
@@ -185,9 +215,6 @@ END
 # Process Pakfire commands
 if (($cgiparams{'ACTION'} eq 'install') && (! &_is_pakfire_busy())) {
 	my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
-	if ("$cgiparams{'FORCE'}" eq "on") {
-		&General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive", "--no-colors", @pkgs);
-	} else {
 		&Header::openbox("100%", "center", $Lang::tr{'request'});
 		my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
 		print <<END;
@@ -222,12 +249,9 @@ END
 		&Header::closebigbox();
 		&Header::closepage();
 		exit;
-	}
+
 } elsif (($cgiparams{'ACTION'} eq 'remove') && (! &_is_pakfire_busy())) {
 	my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
-	if ("$cgiparams{'FORCE'}" eq "on") {
-		&General::system_background("/usr/local/bin/pakfire", "remove", "--non-interactive", "--no-colors", @pkgs);
-	} else {
 		&Header::openbox("100%", "center", $Lang::tr{'request'});
 		my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
 		print <<END;
@@ -262,30 +286,9 @@ END
 		&Header::closebigbox();
 		&Header::closepage();
 		exit;
-	}
-
-} elsif (($cgiparams{'ACTION'} eq 'update') && (! &_is_pakfire_busy())) {
-	&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
-} elsif (($cgiparams{'ACTION'} eq 'upgrade') && (! &_is_pakfire_busy())) {
-	&General::system_background("/usr/local/bin/pakfire", "upgrade", "-y", "--no-colors");
-} elsif ($cgiparams{'ACTION'} eq "$Lang::tr{'save'}") {
-	$pakfiresettings{"TREE"} = $cgiparams{"TREE"};
-
-	# Check for valid input
-	if ($pakfiresettings{"TREE"} !~ m/^(stable|testing|unstable)$/) {
-		$errormessage .= $Lang::tr{'pakfire invalid tree'};
-	}
-
-	unless ($errormessage) {
-		&General::writehash("${General::swroot}/pakfire/settings", \%pakfiresettings);
 
-		# Update lists
-		&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
-	}
 }
 
-&General::readhash("${General::swroot}/pakfire/settings", \%pakfiresettings);
-
 my %selected=();
 my %checked=();
 
-- 
2.27.0.windows.1

[PATCH 2/7] pakfire.cgi: Fix indentation

[Leo-Andres Hofmann]

[-- Attachment #1: Type: text/plain, Size: 5663 bytes --]

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/pakfire.cgi | 111 ++++++++++++++++++++-------------------
 1 file changed, 56 insertions(+), 55 deletions(-)

diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
index b908ec6a0..535168547 100644
--- a/html/cgi-bin/pakfire.cgi
+++ b/html/cgi-bin/pakfire.cgi
@@ -214,79 +214,80 @@ END
 
 # Process Pakfire commands
 if (($cgiparams{'ACTION'} eq 'install') && (! &_is_pakfire_busy())) {
+	&Header::openbox("100%", "center", $Lang::tr{'request'});
+
 	my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
-		&Header::openbox("100%", "center", $Lang::tr{'request'});
-		my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
-		print <<END;
-		<table><tr><td colspan='2'>$Lang::tr{'pakfire install package'} @pkgs $Lang::tr{'pakfire possible dependency'}
+	my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
+	print <<END;
+	<table><tr><td colspan='2'>$Lang::tr{'pakfire install package'} @pkgs $Lang::tr{'pakfire possible dependency'}
 		<pre>
 END
-		foreach (@output) {
-		  $_ =~ s/\^[\[[0-1]\;[0-9]+m//g;
-			print "$_\n";
-		}
-		print <<END;
+	foreach (@output) {
+		$_ =~ s/\^[\[[0-1]\;[0-9]+m//g;
+		print "$_\n";
+	}
+	print <<END;
 		</pre></td></tr>
 		<tr><td colspan='2'>$Lang::tr{'pakfire accept all'}</td></tr>
 		<tr><td colspan='2'>&nbsp;</td></tr>
 		<tr><td align='right'><form method='post' action='$ENV{'SCRIPT_NAME'}'>
-							<input type='hidden' name='INSPAKS' value='$cgiparams{'INSPAKS'}' />
-							<input type='hidden' name='FORCE' value='on' />
-							<input type='hidden' name='ACTION' value='install' />
-							<input type='image' alt='$Lang::tr{'install'}' title='$Lang::tr{'install'}' src='/images/go-next.png' />
-						</form>
-				</td>
-				<td align='left'>
-						<form method='post' action='$ENV{'SCRIPT_NAME'}'>
-							<input type='hidden' name='ACTION' value='' />
-							<input type='image' alt='$Lang::tr{'abort'}' title='$Lang::tr{'abort'}' src='/images/dialog-error.png' />
-						</form>
-				</td>
-			</tr>
-		</table>
+					<input type='hidden' name='INSPAKS' value='$cgiparams{'INSPAKS'}' />
+					<input type='hidden' name='FORCE' value='on' />
+					<input type='hidden' name='ACTION' value='install' />
+					<input type='image' alt='$Lang::tr{'install'}' title='$Lang::tr{'install'}' src='/images/go-next.png' />
+				</form>
+			</td>
+			<td align='left'>
+				<form method='post' action='$ENV{'SCRIPT_NAME'}'>
+					<input type='hidden' name='ACTION' value='' />
+					<input type='image' alt='$Lang::tr{'abort'}' title='$Lang::tr{'abort'}' src='/images/dialog-error.png' />
+				</form>
+			</td>
+		</tr>
+	</table>
 END
-		&Header::closebox();
-		&Header::closebigbox();
-		&Header::closepage();
-		exit;
+	&Header::closebox();
+	&Header::closebigbox();
+	&Header::closepage();
+	exit;
 
 } elsif (($cgiparams{'ACTION'} eq 'remove') && (! &_is_pakfire_busy())) {
+	&Header::openbox("100%", "center", $Lang::tr{'request'});
+
 	my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
-		&Header::openbox("100%", "center", $Lang::tr{'request'});
-		my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
-		print <<END;
-		<table><tr><td colspan='2'>$Lang::tr{'pakfire uninstall package'} @pkgs $Lang::tr{'pakfire possible dependency'}
+	my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
+	print <<END;
+	<table><tr><td colspan='2'>$Lang::tr{'pakfire uninstall package'} @pkgs $Lang::tr{'pakfire possible dependency'}
 		<pre>
 END
-		foreach (@output) {
-		  $_ =~ s/\^[\[[0-1]\;[0-9]+m//g;
-			print "$_\n";
-		}
-		print <<END;
+	foreach (@output) {
+		$_ =~ s/\^[\[[0-1]\;[0-9]+m//g;
+		print "$_\n";
+	}
+	print <<END;
 		</pre></td></tr>
 		<tr><td colspan='2'>$Lang::tr{'pakfire uninstall all'}</td></tr>
 		<tr><td colspan='2'>&nbsp;</td></tr>
 		<tr><td align='right'><form method='post' action='$ENV{'SCRIPT_NAME'}'>
-							<input type='hidden' name='DELPAKS' value='$cgiparams{'DELPAKS'}' />
-							<input type='hidden' name='FORCE' value='on' />
-							<input type='hidden' name='ACTION' value='remove' />
-							<input type='image' alt='$Lang::tr{'uninstall'}' title='$Lang::tr{'uninstall'}' src='/images/go-next.png' />
-						</form>
-				</td>
-				<td align='left'>
-						<form method='post' action='$ENV{'SCRIPT_NAME'}'>
-							<input type='hidden' name='ACTION' value='' />
-							<input type='image' alt='$Lang::tr{'abort'}' title='$Lang::tr{'abort'}' src='/images/dialog-error.png' />
-						</form>
-				</td>
-			</tr>
-		</table>
+					<input type='hidden' name='DELPAKS' value='$cgiparams{'DELPAKS'}' />
+					<input type='hidden' name='FORCE' value='on' />
+					<input type='hidden' name='ACTION' value='remove' />
+					<input type='image' alt='$Lang::tr{'uninstall'}' title='$Lang::tr{'uninstall'}' src='/images/go-next.png' />
+				</form>
+			</td>
+			<td align='left'>
+				<form method='post' action='$ENV{'SCRIPT_NAME'}'>
+					<input type='hidden' name='ACTION' value='' />
+					<input type='image' alt='$Lang::tr{'abort'}' title='$Lang::tr{'abort'}' src='/images/dialog-error.png' />
+				</form>
+			</td>
+		</tr>
+	</table>
 END
-		&Header::closebox();
-		&Header::closebigbox();
-		&Header::closepage();
-		exit;
-
+	&Header::closebox();
+	&Header::closebigbox();
+	&Header::closepage();
+	exit;
 }
 
 my %selected=();
-- 
2.27.0.windows.1

[PATCH 3/7] pakfire.cgi: Show error and log messages earlier

[Leo-Andres Hofmann]

[-- Attachment #1: Type: text/plain, Size: 3464 bytes --]

The main page cannot be used while an installation is running.
Therefore it makes more sense to generate the log output first.

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/pakfire.cgi | 79 ++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 39 deletions(-)

diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
index 535168547..daa82e34c 100644
--- a/html/cgi-bin/pakfire.cgi
+++ b/html/cgi-bin/pakfire.cgi
@@ -212,7 +212,45 @@ END
 &Header::openpage($Lang::tr{'pakfire configuration'}, 1, $extraHead);
 &Header::openbigbox('100%', 'left', '', $errormessage);
 
-# Process Pakfire commands
+# Show error message
+if ($errormessage) {
+	&Header::openbox('100%', 'left', $Lang::tr{'error messages'});
+	print "<font class='base'>$errormessage&nbsp;</font>\n";
+	&Header::closebox();
+}
+
+# Show log output while Pakfire is running
+if(&_is_pakfire_busy()) {
+	&Header::openbox("100%", "center", "Pakfire");
+
+	print <<END
+<section id="pflog-header">
+	<div><img src="/images/indicator.gif" alt="$Lang::tr{'active'}" title="$Lang::tr{'pagerefresh'}"></div>
+	<div>
+		<span id="pflog-status">$Lang::tr{'pakfire working'}</span><br>
+		<span id="pflog-time"></span><br>
+		<span id="pflog-action"></span>
+	</div>
+	<div><a href="$ENV{'SCRIPT_NAME'}"><img src="/images/view-refresh.png" alt="$Lang::tr{'refresh'}" title="$Lang::tr{'refresh'}"></a></div>
+</section>
+
+<!-- Pakfire log messages -->
+<pre id="pflog-messages"></pre>
+<script>
+	// Start automatic log refresh
+	pakfire.running = true;
+</script>
+
+END
+;
+
+	&Header::closebox();
+	&Header::closebigbox();
+	&Header::closepage();
+	exit;
+}
+
+# Show Pakfire install/remove dependencies and confirm form
 if (($cgiparams{'ACTION'} eq 'install') && (! &_is_pakfire_busy())) {
 	&Header::openbox("100%", "center", $Lang::tr{'request'});
 
@@ -290,6 +328,7 @@ END
 	exit;
 }
 
+# Show Pakfire main page
 my %selected=();
 my %checked=();
 
@@ -299,44 +338,6 @@ $selected{"TREE"}{"testing"} = "";
 $selected{"TREE"}{"unstable"} = "";
 $selected{"TREE"}{$pakfiresettings{"TREE"}} = "selected";
 
-# DPC move error message to top so it is seen!
-if ($errormessage) {
-	&Header::openbox('100%', 'left', $Lang::tr{'error messages'});
-	print "<font class='base'>$errormessage&nbsp;</font>\n";
-	&Header::closebox();
-}
-
-# Show log output while Pakfire is running
-if(&_is_pakfire_busy()) {
-	&Header::openbox("100%", "center", "Pakfire");
-
-	print <<END
-<section id="pflog-header">
-	<div><img src="/images/indicator.gif" alt="$Lang::tr{'active'}" title="$Lang::tr{'pagerefresh'}"></div>
-	<div>
-		<span id="pflog-status">$Lang::tr{'pakfire working'}</span><br>
-		<span id="pflog-time"></span><br>
-		<span id="pflog-action"></span>
-	</div>
-	<div><a href="$ENV{'SCRIPT_NAME'}"><img src="/images/view-refresh.png" alt="$Lang::tr{'refresh'}" title="$Lang::tr{'refresh'}"></a></div>
-</section>
-
-<!-- Pakfire log messages -->
-<pre id="pflog-messages"></pre>
-<script>
-	// Start automatic log refresh
-	pakfire.running = true;
-</script>
-
-END
-;
-
-	&Header::closebox();
-	&Header::closebigbox();
-	&Header::closepage();
-	exit;
-}
-
 my $core_release = `cat /opt/pakfire/db/core/mine 2>/dev/null`;
 chomp($core_release);
 my $core_update_age = &General::age("/opt/pakfire/db/core/mine");
-- 
2.27.0.windows.1

[PATCH 4/7] pakfire.cgi: Notify user if Pakfire is already performing a task

[Leo-Andres Hofmann]

[-- Attachment #1: Type: text/plain, Size: 2398 bytes --]

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/pakfire.cgi | 6 ++++--
 langs/de/cgi-bin/de.pl   | 1 +
 langs/en/cgi-bin/en.pl   | 1 +
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
index daa82e34c..ec3ee2cc6 100644
--- a/html/cgi-bin/pakfire.cgi
+++ b/html/cgi-bin/pakfire.cgi
@@ -96,8 +96,10 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') {
 }
 
 ### Process Pakfire install/update commands ###
-if(($cgiparams{'ACTION'} ne '') && (! &_is_pakfire_busy())) {
-	if(($cgiparams{'ACTION'} eq 'install') && ($cgiparams{'FORCE'} eq 'on')) {
+if($cgiparams{'ACTION'} ne '') {
+	if(&_is_pakfire_busy()) {
+		$errormessage = $Lang::tr{'pakfire already busy'};
+	} elsif(($cgiparams{'ACTION'} eq 'install') && ($cgiparams{'FORCE'} eq 'on')) {
 		my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
 		&General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive", "--no-colors", @pkgs);
 	} elsif(($cgiparams{'ACTION'} eq 'remove') && ($cgiparams{'FORCE'} eq 'on')) {
diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
index a841b39f9..7a39e233b 100644
--- a/langs/de/cgi-bin/de.pl
+++ b/langs/de/cgi-bin/de.pl
@@ -1990,6 +1990,7 @@
 'pagerefresh' => 'Seite wird aktualisiert. Bitte warten.',
 'pakfire accept all' => 'Möchten Sie der Installation aller Pakete zustimmen?',
 'pakfire ago' => 'her.',
+'pakfire already busy' => 'Pakfire führt bereits eine Aufgabe aus. Bitte versuchen Sie es später erneut.',
 'pakfire available addons' => 'Verfügbare Addons:',
 'pakfire configuration' => 'Pakfire Konfiguration',
 'pakfire core update auto' => 'Core- und Addon-Updates automatisch installieren:',
diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
index e30ee4f69..f90e3103b 100644
--- a/langs/en/cgi-bin/en.pl
+++ b/langs/en/cgi-bin/en.pl
@@ -2041,6 +2041,7 @@
 'pagerefresh' => 'Page is beeing refreshed, please wait.',
 'pakfire accept all' => 'Do you want to install all packages?',
 'pakfire ago' => 'ago.',
+'pakfire already busy' => 'Pakfire is already performing a task. Please try again later.',
 'pakfire available addons' => 'Available Addons:',
 'pakfire configuration' => 'Pakfire Configuration',
 'pakfire core update auto' => 'Install core and addon updates automatically:',
-- 
2.27.0.windows.1

[PATCH 5/7] pakfire.cgi: Implement Post/Redirect/Get pattern

[Leo-Andres Hofmann]

[-- Attachment #1: Type: text/plain, Size: 5851 bytes --]

Refreshing the Pakfire page may cause a command to be
executed multiple times and induce odd errors.

This patch implements a HTTP 303 redirect after form processing,
which causes the browser to discard the POST form data.
Navigating backward or reloading the page now does not trigger
multiple executions anymore.

Fixes: #12781

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/pakfire.cgi | 56 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
index ec3ee2cc6..6fade81bd 100644
--- a/html/cgi-bin/pakfire.cgi
+++ b/html/cgi-bin/pakfire.cgi
@@ -21,6 +21,7 @@
 
 use strict;
 use List::Util qw(any);
+use URI;
 
 # enable only the following on debugging purpose
 #use warnings;
@@ -37,12 +38,17 @@ my %color = ();
 my %pakfiresettings = ();
 my %mainsettings = ();
 
+# The page mode is used to explictly switch between user interface functions:
+my $PM_DEFAULT = 'default'; # Default user interface with command processing
+my $PM_LOGREAD = 'logread'; # Log messages viewer (ignores all commands)
+my $pagemode = $PM_DEFAULT;
+
 # Load general settings
 &General::readhash("${General::swroot}/main/settings", \%mainsettings);
 &General::readhash("${General::swroot}/pakfire/settings", \%pakfiresettings);
 &General::readhash("/srv/web/ipfire/html/themes/ipfire/include/colors.txt", \%color);
 
-# Get CGI request data
+# Get CGI POST request data
 $cgiparams{'ACTION'} = '';
 $cgiparams{'FORCE'} = '';
 
@@ -51,6 +57,17 @@ $cgiparams{'DELPAKS'} = '';
 
 &Header::getcgihash(\%cgiparams);
 
+# Get CGI GET request data (if available)
+if($ENV{'QUERY_STRING'}) {
+	my $uri = URI->new($ENV{'REQUEST_URI'});
+	my %query = $uri->query_form;
+
+	my $mode = lc($query{'mode'} // '');
+	if(($mode eq $PM_DEFAULT) || ($mode eq $PM_LOGREAD)) {
+		$pagemode = $mode; # Limit to existing modes
+	}
+}
+
 ### Process AJAX/JSON request ###
 if($cgiparams{'ACTION'} eq 'json-getstatus') {
 	# Send HTTP headers
@@ -96,19 +113,24 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') {
 }
 
 ### Process Pakfire install/update commands ###
-if($cgiparams{'ACTION'} ne '') {
+if(($cgiparams{'ACTION'} ne '') && ($pagemode eq $PM_DEFAULT)) {
 	if(&_is_pakfire_busy()) {
 		$errormessage = $Lang::tr{'pakfire already busy'};
+		$pagemode = $PM_LOGREAD; # Running Pakfire instance found, switch to log viewer mode
 	} elsif(($cgiparams{'ACTION'} eq 'install') && ($cgiparams{'FORCE'} eq 'on')) {
 		my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
 		&General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive", "--no-colors", @pkgs);
+		&_http_pagemode_redirect($PM_LOGREAD, 1);
 	} elsif(($cgiparams{'ACTION'} eq 'remove') && ($cgiparams{'FORCE'} eq 'on')) {
 		my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
 		&General::system_background("/usr/local/bin/pakfire", "remove", "--non-interactive", "--no-colors", @pkgs);
+		&_http_pagemode_redirect($PM_LOGREAD, 1);
 	} elsif($cgiparams{'ACTION'} eq 'update') {
 		&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
+		&_http_pagemode_redirect($PM_LOGREAD, 1);
 	} elsif($cgiparams{'ACTION'} eq 'upgrade') {
 		&General::system_background("/usr/local/bin/pakfire", "upgrade", "-y", "--no-colors");
+		&_http_pagemode_redirect($PM_LOGREAD, 1);
 	} elsif($cgiparams{'ACTION'} eq $Lang::tr{'save'}) {
 		$pakfiresettings{"TREE"} = $cgiparams{"TREE"};
 
@@ -122,6 +144,7 @@ if($cgiparams{'ACTION'} ne '') {
 
 			# Update lists
 			&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
+			&_http_pagemode_redirect($PM_LOGREAD, 1);
 		}
 	}
 }
@@ -221,8 +244,8 @@ if ($errormessage) {
 	&Header::closebox();
 }
 
-# Show log output while Pakfire is running
-if(&_is_pakfire_busy()) {
+# Show only log output while Pakfire is running and stop afterwards
+if(($pagemode eq $PM_LOGREAD) || (&_is_pakfire_busy())) {
 	&Header::openbox("100%", "center", "Pakfire");
 
 	print <<END
@@ -253,7 +276,8 @@ END
 }
 
 # Show Pakfire install/remove dependencies and confirm form
-if (($cgiparams{'ACTION'} eq 'install') && (! &_is_pakfire_busy())) {
+# (_is_pakfire_busy status was checked before and can be omitted)
+if (($cgiparams{'ACTION'} eq 'install') && ($pagemode eq $PM_DEFAULT)) {
 	&Header::openbox("100%", "center", $Lang::tr{'request'});
 
 	my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
@@ -291,7 +315,7 @@ END
 	&Header::closepage();
 	exit;
 
-} elsif (($cgiparams{'ACTION'} eq 'remove') && (! &_is_pakfire_busy())) {
+} elsif (($cgiparams{'ACTION'} eq 'remove') && ($pagemode eq $PM_DEFAULT)) {
 	&Header::openbox("100%", "center", $Lang::tr{'request'});
 
 	my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
@@ -476,3 +500,23 @@ sub _start_json_output {
 	print "Content-Type: application/json\n";
 	print "\n"; # End of HTTP headers
 }
+
+# Send HTTP 303 redirect headers to change page mode
+# GET is always used to display the redirected page, which will remove already processed POST form data.
+# Note: Custom headers must be sent before the HTML output is started by &Header::showhttpheaders().
+# If switch_mode is set to true, the global page mode variable ("$pagemode") is also updated immediately.
+sub _http_pagemode_redirect {
+	my ($mode, $switch_mode) = @_;
+	$mode //= $PM_DEFAULT;
+	$switch_mode //= 0;
+
+	# Send HTTP redirect with GET parameter
+	my $location = "https://$ENV{'SERVER_NAME'}:$ENV{'SERVER_PORT'}$ENV{'SCRIPT_NAME'}?mode=${mode}";
+	print "Status: 303 See Other\n";
+	print "Location: $location\n";
+
+	# Change global page mode
+	if($switch_mode) {
+		$pagemode = $mode;
+	}
+}
-- 
2.27.0.windows.1

[PATCH 6/7] pakfire.cgi: Discard tac stderr output

[Leo-Andres Hofmann]

[-- Attachment #1: Type: text/plain, Size: 974 bytes --]

Prevents meaningless "broken pipe" messages in the httpd error log.

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/pakfire.cgi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
index 6fade81bd..489b07a6d 100644
--- a/html/cgi-bin/pakfire.cgi
+++ b/html/cgi-bin/pakfire.cgi
@@ -75,7 +75,7 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') {
 
 	# Read /var/log/messages backwards until a "Pakfire started" header is found,
 	# to capture all messages of the last (i.e. current) Pakfire run
-	my @messages = `tac /var/log/messages | sed -n '/pakfire:/{p;/Pakfire.*started/q}'`;
+	my @messages = `tac /var/log/messages 2>/dev/null | sed -n '/pakfire:/{p;/Pakfire.*started/q}'`;
 
 	# Test if the log contains an error message (fastest implementation, stops at first match)
 	my $failure = any{ index($_, 'ERROR') != -1 } @messages;
-- 
2.27.0.windows.1

[PATCH 7/7] pakfire.cgi: Cosmetic fixes

[Leo-Andres Hofmann]

[-- Attachment #1: Type: text/plain, Size: 2403 bytes --]

Add formatting to improve readability of dependencies list header.

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/pakfire.cgi | 6 ++++--
 langs/fr/cgi-bin/fr.pl   | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
index 489b07a6d..3e8dc5460 100644
--- a/html/cgi-bin/pakfire.cgi
+++ b/html/cgi-bin/pakfire.cgi
@@ -283,7 +283,8 @@ if (($cgiparams{'ACTION'} eq 'install') && ($pagemode eq $PM_DEFAULT)) {
 	my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
 	my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
 	print <<END;
-	<table><tr><td colspan='2'>$Lang::tr{'pakfire install package'} @pkgs $Lang::tr{'pakfire possible dependency'}
+	<table style="width: 100%"><tr><td colspan='2'><p>$Lang::tr{'pakfire install package'} <strong>@{pkgs}</strong><br>
+		$Lang::tr{'pakfire possible dependency'}</p>
 		<pre>
 END
 	foreach (@output) {
@@ -321,7 +322,8 @@ END
 	my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
 	my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
 	print <<END;
-	<table><tr><td colspan='2'>$Lang::tr{'pakfire uninstall package'} @pkgs $Lang::tr{'pakfire possible dependency'}
+	<table style="width: 100%"><tr><td colspan='2'><p>$Lang::tr{'pakfire uninstall package'} <strong>@{pkgs}</strong><br>
+		$Lang::tr{'pakfire possible dependency'}</p>
 		<pre>
 END
 	foreach (@output) {
diff --git a/langs/fr/cgi-bin/fr.pl b/langs/fr/cgi-bin/fr.pl
index bd17df4ee..6dbeebc16 100644
--- a/langs/fr/cgi-bin/fr.pl
+++ b/langs/fr/cgi-bin/fr.pl
@@ -2055,7 +2055,7 @@
 'pakfire last package update' => 'Dernière mise à jour de la liste des paquets : ',
 'pakfire last serverlist update' => 'Dernière mise à jour de la liste des serveurs : ',
 'pakfire last update' => 'Dernière mise à jour : ',
-'pakfire possible dependency' => '<br><br>Il y a peut-être des dépendances, voici la liste des paquets qu\'il faut (dés)installer.<br>',
+'pakfire possible dependency' => 'Il y a peut-être des dépendances, voici la liste des paquets qu\'il faut (dés)installer.',
 'pakfire register' => 'S\'inscrire au serveur pakfire :',
 'pakfire return' => 'Retour à Pakfire',
 'pakfire system state' => 'Statut système PakFire ',
-- 
2.27.0.windows.1

Re: [PATCH 1/7] pakfire.cgi: Separate command processing and HTML generation

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 5088 bytes --]

Acked-by: Peter Müller <peter.muelle(a)ipfire.org>

> Move most of the command execution away from the HTML output.
> This makes it easier to modify or extend individual commands.
> 
> Also load Pakfire settings earlier to ensure that they are
> available during command execution.
> 
> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
> ---
>  html/cgi-bin/pakfire.cgi | 61 +++++++++++++++++++++-------------------
>  1 file changed, 32 insertions(+), 29 deletions(-)
> 
> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
> index 65c67fb90..b908ec6a0 100644
> --- a/html/cgi-bin/pakfire.cgi
> +++ b/html/cgi-bin/pakfire.cgi
> @@ -39,11 +39,12 @@ my %mainsettings = ();
>  
>  # Load general settings
>  &General::readhash("${General::swroot}/main/settings", \%mainsettings);
> +&General::readhash("${General::swroot}/pakfire/settings", \%pakfiresettings);
>  &General::readhash("/srv/web/ipfire/html/themes/ipfire/include/colors.txt", \%color);
>  
>  # Get CGI request data
>  $cgiparams{'ACTION'} = '';
> -$cgiparams{'VALID'} = '';
> +$cgiparams{'FORCE'} = '';
>  
>  $cgiparams{'INSPAKS'} = '';
>  $cgiparams{'DELPAKS'} = '';
> @@ -94,6 +95,35 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') {
>  	exit;
>  }
>  
> +### Process Pakfire install/update commands ###
> +if(($cgiparams{'ACTION'} ne '') && (! &_is_pakfire_busy())) {
> +	if(($cgiparams{'ACTION'} eq 'install') && ($cgiparams{'FORCE'} eq 'on')) {
> +		my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
> +		&General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive", "--no-colors", @pkgs);
> +	} elsif(($cgiparams{'ACTION'} eq 'remove') && ($cgiparams{'FORCE'} eq 'on')) {
> +		my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
> +		&General::system_background("/usr/local/bin/pakfire", "remove", "--non-interactive", "--no-colors", @pkgs);
> +	} elsif($cgiparams{'ACTION'} eq 'update') {
> +		&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
> +	} elsif($cgiparams{'ACTION'} eq 'upgrade') {
> +		&General::system_background("/usr/local/bin/pakfire", "upgrade", "-y", "--no-colors");
> +	} elsif($cgiparams{'ACTION'} eq $Lang::tr{'save'}) {
> +		$pakfiresettings{"TREE"} = $cgiparams{"TREE"};
> +
> +		# Check for valid input
> +		if ($pakfiresettings{"TREE"} !~ m/^(stable|testing|unstable)$/) {
> +			$errormessage .= $Lang::tr{'pakfire invalid tree'};
> +		}
> +
> +		unless ($errormessage) {
> +			&General::writehash("${General::swroot}/pakfire/settings", \%pakfiresettings);
> +
> +			# Update lists
> +			&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
> +		}
> +	}
> +}
> +
>  ### Start pakfire page ###
>  &Header::showhttpheaders();
>  
> @@ -185,9 +215,6 @@ END
>  # Process Pakfire commands
>  if (($cgiparams{'ACTION'} eq 'install') && (! &_is_pakfire_busy())) {
>  	my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
> -	if ("$cgiparams{'FORCE'}" eq "on") {
> -		&General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive", "--no-colors", @pkgs);
> -	} else {
>  		&Header::openbox("100%", "center", $Lang::tr{'request'});
>  		my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
>  		print <<END;
> @@ -222,12 +249,9 @@ END
>  		&Header::closebigbox();
>  		&Header::closepage();
>  		exit;
> -	}
> +
>  } elsif (($cgiparams{'ACTION'} eq 'remove') && (! &_is_pakfire_busy())) {
>  	my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
> -	if ("$cgiparams{'FORCE'}" eq "on") {
> -		&General::system_background("/usr/local/bin/pakfire", "remove", "--non-interactive", "--no-colors", @pkgs);
> -	} else {
>  		&Header::openbox("100%", "center", $Lang::tr{'request'});
>  		my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
>  		print <<END;
> @@ -262,30 +286,9 @@ END
>  		&Header::closebigbox();
>  		&Header::closepage();
>  		exit;
> -	}
> -
> -} elsif (($cgiparams{'ACTION'} eq 'update') && (! &_is_pakfire_busy())) {
> -	&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
> -} elsif (($cgiparams{'ACTION'} eq 'upgrade') && (! &_is_pakfire_busy())) {
> -	&General::system_background("/usr/local/bin/pakfire", "upgrade", "-y", "--no-colors");
> -} elsif ($cgiparams{'ACTION'} eq "$Lang::tr{'save'}") {
> -	$pakfiresettings{"TREE"} = $cgiparams{"TREE"};
> -
> -	# Check for valid input
> -	if ($pakfiresettings{"TREE"} !~ m/^(stable|testing|unstable)$/) {
> -		$errormessage .= $Lang::tr{'pakfire invalid tree'};
> -	}
> -
> -	unless ($errormessage) {
> -		&General::writehash("${General::swroot}/pakfire/settings", \%pakfiresettings);
>  
> -		# Update lists
> -		&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
> -	}
>  }
>  
> -&General::readhash("${General::swroot}/pakfire/settings", \%pakfiresettings);
> -
>  my %selected=();
>  my %checked=();
>  

Re: [PATCH 2/7] pakfire.cgi: Fix indentation

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 6060 bytes --]

Oh yes, we do have a rather lot of these. Thank you for cleaning them up.

Acked-by: Peter Müller <peter.muelle(a)ipfire.org>

> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
> ---
>  html/cgi-bin/pakfire.cgi | 111 ++++++++++++++++++++-------------------
>  1 file changed, 56 insertions(+), 55 deletions(-)
> 
> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
> index b908ec6a0..535168547 100644
> --- a/html/cgi-bin/pakfire.cgi
> +++ b/html/cgi-bin/pakfire.cgi
> @@ -214,79 +214,80 @@ END
>  
>  # Process Pakfire commands
>  if (($cgiparams{'ACTION'} eq 'install') && (! &_is_pakfire_busy())) {
> +	&Header::openbox("100%", "center", $Lang::tr{'request'});
> +
>  	my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
> -		&Header::openbox("100%", "center", $Lang::tr{'request'});
> -		my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
> -		print <<END;
> -		<table><tr><td colspan='2'>$Lang::tr{'pakfire install package'} @pkgs $Lang::tr{'pakfire possible dependency'}
> +	my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
> +	print <<END;
> +	<table><tr><td colspan='2'>$Lang::tr{'pakfire install package'} @pkgs $Lang::tr{'pakfire possible dependency'}
>  		<pre>
>  END
> -		foreach (@output) {
> -		  $_ =~ s/\^[\[[0-1]\;[0-9]+m//g;
> -			print "$_\n";
> -		}
> -		print <<END;
> +	foreach (@output) {
> +		$_ =~ s/\^[\[[0-1]\;[0-9]+m//g;
> +		print "$_\n";
> +	}
> +	print <<END;
>  		</pre></td></tr>
>  		<tr><td colspan='2'>$Lang::tr{'pakfire accept all'}</td></tr>
>  		<tr><td colspan='2'>&nbsp;</td></tr>
>  		<tr><td align='right'><form method='post' action='$ENV{'SCRIPT_NAME'}'>
> -							<input type='hidden' name='INSPAKS' value='$cgiparams{'INSPAKS'}' />
> -							<input type='hidden' name='FORCE' value='on' />
> -							<input type='hidden' name='ACTION' value='install' />
> -							<input type='image' alt='$Lang::tr{'install'}' title='$Lang::tr{'install'}' src='/images/go-next.png' />
> -						</form>
> -				</td>
> -				<td align='left'>
> -						<form method='post' action='$ENV{'SCRIPT_NAME'}'>
> -							<input type='hidden' name='ACTION' value='' />
> -							<input type='image' alt='$Lang::tr{'abort'}' title='$Lang::tr{'abort'}' src='/images/dialog-error.png' />
> -						</form>
> -				</td>
> -			</tr>
> -		</table>
> +					<input type='hidden' name='INSPAKS' value='$cgiparams{'INSPAKS'}' />
> +					<input type='hidden' name='FORCE' value='on' />
> +					<input type='hidden' name='ACTION' value='install' />
> +					<input type='image' alt='$Lang::tr{'install'}' title='$Lang::tr{'install'}' src='/images/go-next.png' />
> +				</form>
> +			</td>
> +			<td align='left'>
> +				<form method='post' action='$ENV{'SCRIPT_NAME'}'>
> +					<input type='hidden' name='ACTION' value='' />
> +					<input type='image' alt='$Lang::tr{'abort'}' title='$Lang::tr{'abort'}' src='/images/dialog-error.png' />
> +				</form>
> +			</td>
> +		</tr>
> +	</table>
>  END
> -		&Header::closebox();
> -		&Header::closebigbox();
> -		&Header::closepage();
> -		exit;
> +	&Header::closebox();
> +	&Header::closebigbox();
> +	&Header::closepage();
> +	exit;
>  
>  } elsif (($cgiparams{'ACTION'} eq 'remove') && (! &_is_pakfire_busy())) {
> +	&Header::openbox("100%", "center", $Lang::tr{'request'});
> +
>  	my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
> -		&Header::openbox("100%", "center", $Lang::tr{'request'});
> -		my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
> -		print <<END;
> -		<table><tr><td colspan='2'>$Lang::tr{'pakfire uninstall package'} @pkgs $Lang::tr{'pakfire possible dependency'}
> +	my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
> +	print <<END;
> +	<table><tr><td colspan='2'>$Lang::tr{'pakfire uninstall package'} @pkgs $Lang::tr{'pakfire possible dependency'}
>  		<pre>
>  END
> -		foreach (@output) {
> -		  $_ =~ s/\^[\[[0-1]\;[0-9]+m//g;
> -			print "$_\n";
> -		}
> -		print <<END;
> +	foreach (@output) {
> +		$_ =~ s/\^[\[[0-1]\;[0-9]+m//g;
> +		print "$_\n";
> +	}
> +	print <<END;
>  		</pre></td></tr>
>  		<tr><td colspan='2'>$Lang::tr{'pakfire uninstall all'}</td></tr>
>  		<tr><td colspan='2'>&nbsp;</td></tr>
>  		<tr><td align='right'><form method='post' action='$ENV{'SCRIPT_NAME'}'>
> -							<input type='hidden' name='DELPAKS' value='$cgiparams{'DELPAKS'}' />
> -							<input type='hidden' name='FORCE' value='on' />
> -							<input type='hidden' name='ACTION' value='remove' />
> -							<input type='image' alt='$Lang::tr{'uninstall'}' title='$Lang::tr{'uninstall'}' src='/images/go-next.png' />
> -						</form>
> -				</td>
> -				<td align='left'>
> -						<form method='post' action='$ENV{'SCRIPT_NAME'}'>
> -							<input type='hidden' name='ACTION' value='' />
> -							<input type='image' alt='$Lang::tr{'abort'}' title='$Lang::tr{'abort'}' src='/images/dialog-error.png' />
> -						</form>
> -				</td>
> -			</tr>
> -		</table>
> +					<input type='hidden' name='DELPAKS' value='$cgiparams{'DELPAKS'}' />
> +					<input type='hidden' name='FORCE' value='on' />
> +					<input type='hidden' name='ACTION' value='remove' />
> +					<input type='image' alt='$Lang::tr{'uninstall'}' title='$Lang::tr{'uninstall'}' src='/images/go-next.png' />
> +				</form>
> +			</td>
> +			<td align='left'>
> +				<form method='post' action='$ENV{'SCRIPT_NAME'}'>
> +					<input type='hidden' name='ACTION' value='' />
> +					<input type='image' alt='$Lang::tr{'abort'}' title='$Lang::tr{'abort'}' src='/images/dialog-error.png' />
> +				</form>
> +			</td>
> +		</tr>
> +	</table>
>  END
> -		&Header::closebox();
> -		&Header::closebigbox();
> -		&Header::closepage();
> -		exit;
> -
> +	&Header::closebox();
> +	&Header::closebigbox();
> +	&Header::closepage();
> +	exit;
>  }
>  
>  my %selected=();

Re: [PATCH 3/7] pakfire.cgi: Show error and log messages earlier

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 3718 bytes --]

Acked-by: Peter Müller <peter.muelle(a)ipfire.org>

> The main page cannot be used while an installation is running.
> Therefore it makes more sense to generate the log output first.
> 
> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
> ---
>  html/cgi-bin/pakfire.cgi | 79 ++++++++++++++++++++--------------------
>  1 file changed, 40 insertions(+), 39 deletions(-)
> 
> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
> index 535168547..daa82e34c 100644
> --- a/html/cgi-bin/pakfire.cgi
> +++ b/html/cgi-bin/pakfire.cgi
> @@ -212,7 +212,45 @@ END
>  &Header::openpage($Lang::tr{'pakfire configuration'}, 1, $extraHead);
>  &Header::openbigbox('100%', 'left', '', $errormessage);
>  
> -# Process Pakfire commands
> +# Show error message
> +if ($errormessage) {
> +	&Header::openbox('100%', 'left', $Lang::tr{'error messages'});
> +	print "<font class='base'>$errormessage&nbsp;</font>\n";
> +	&Header::closebox();
> +}
> +
> +# Show log output while Pakfire is running
> +if(&_is_pakfire_busy()) {
> +	&Header::openbox("100%", "center", "Pakfire");
> +
> +	print <<END
> +<section id="pflog-header">
> +	<div><img src="/images/indicator.gif" alt="$Lang::tr{'active'}" title="$Lang::tr{'pagerefresh'}"></div>
> +	<div>
> +		<span id="pflog-status">$Lang::tr{'pakfire working'}</span><br>
> +		<span id="pflog-time"></span><br>
> +		<span id="pflog-action"></span>
> +	</div>
> +	<div><a href="$ENV{'SCRIPT_NAME'}"><img src="/images/view-refresh.png" alt="$Lang::tr{'refresh'}" title="$Lang::tr{'refresh'}"></a></div>
> +</section>
> +
> +<!-- Pakfire log messages -->
> +<pre id="pflog-messages"></pre>
> +<script>
> +	// Start automatic log refresh
> +	pakfire.running = true;
> +</script>
> +
> +END
> +;
> +
> +	&Header::closebox();
> +	&Header::closebigbox();
> +	&Header::closepage();
> +	exit;
> +}
> +
> +# Show Pakfire install/remove dependencies and confirm form
>  if (($cgiparams{'ACTION'} eq 'install') && (! &_is_pakfire_busy())) {
>  	&Header::openbox("100%", "center", $Lang::tr{'request'});
>  
> @@ -290,6 +328,7 @@ END
>  	exit;
>  }
>  
> +# Show Pakfire main page
>  my %selected=();
>  my %checked=();
>  
> @@ -299,44 +338,6 @@ $selected{"TREE"}{"testing"} = "";
>  $selected{"TREE"}{"unstable"} = "";
>  $selected{"TREE"}{$pakfiresettings{"TREE"}} = "selected";
>  
> -# DPC move error message to top so it is seen!
> -if ($errormessage) {
> -	&Header::openbox('100%', 'left', $Lang::tr{'error messages'});
> -	print "<font class='base'>$errormessage&nbsp;</font>\n";
> -	&Header::closebox();
> -}
> -
> -# Show log output while Pakfire is running
> -if(&_is_pakfire_busy()) {
> -	&Header::openbox("100%", "center", "Pakfire");
> -
> -	print <<END
> -<section id="pflog-header">
> -	<div><img src="/images/indicator.gif" alt="$Lang::tr{'active'}" title="$Lang::tr{'pagerefresh'}"></div>
> -	<div>
> -		<span id="pflog-status">$Lang::tr{'pakfire working'}</span><br>
> -		<span id="pflog-time"></span><br>
> -		<span id="pflog-action"></span>
> -	</div>
> -	<div><a href="$ENV{'SCRIPT_NAME'}"><img src="/images/view-refresh.png" alt="$Lang::tr{'refresh'}" title="$Lang::tr{'refresh'}"></a></div>
> -</section>
> -
> -<!-- Pakfire log messages -->
> -<pre id="pflog-messages"></pre>
> -<script>
> -	// Start automatic log refresh
> -	pakfire.running = true;
> -</script>
> -
> -END
> -;
> -
> -	&Header::closebox();
> -	&Header::closebigbox();
> -	&Header::closepage();
> -	exit;
> -}
> -
>  my $core_release = `cat /opt/pakfire/db/core/mine 2>/dev/null`;
>  chomp($core_release);
>  my $core_update_age = &General::age("/opt/pakfire/db/core/mine");

Re: [PATCH 4/7] pakfire.cgi: Notify user if Pakfire is already performing a task

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 2524 bytes --]

Acked-by: Peter Müller <peter.muelle(a)ipfire.org>

> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
> ---
>  html/cgi-bin/pakfire.cgi | 6 ++++--
>  langs/de/cgi-bin/de.pl   | 1 +
>  langs/en/cgi-bin/en.pl   | 1 +
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
> index daa82e34c..ec3ee2cc6 100644
> --- a/html/cgi-bin/pakfire.cgi
> +++ b/html/cgi-bin/pakfire.cgi
> @@ -96,8 +96,10 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') {
>  }
>  
>  ### Process Pakfire install/update commands ###
> -if(($cgiparams{'ACTION'} ne '') && (! &_is_pakfire_busy())) {
> -	if(($cgiparams{'ACTION'} eq 'install') && ($cgiparams{'FORCE'} eq 'on')) {
> +if($cgiparams{'ACTION'} ne '') {
> +	if(&_is_pakfire_busy()) {
> +		$errormessage = $Lang::tr{'pakfire already busy'};
> +	} elsif(($cgiparams{'ACTION'} eq 'install') && ($cgiparams{'FORCE'} eq 'on')) {
>  		my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
>  		&General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive", "--no-colors", @pkgs);
>  	} elsif(($cgiparams{'ACTION'} eq 'remove') && ($cgiparams{'FORCE'} eq 'on')) {
> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
> index a841b39f9..7a39e233b 100644
> --- a/langs/de/cgi-bin/de.pl
> +++ b/langs/de/cgi-bin/de.pl
> @@ -1990,6 +1990,7 @@
>  'pagerefresh' => 'Seite wird aktualisiert. Bitte warten.',
>  'pakfire accept all' => 'Möchten Sie der Installation aller Pakete zustimmen?',
>  'pakfire ago' => 'her.',
> +'pakfire already busy' => 'Pakfire führt bereits eine Aufgabe aus. Bitte versuchen Sie es später erneut.',
>  'pakfire available addons' => 'Verfügbare Addons:',
>  'pakfire configuration' => 'Pakfire Konfiguration',
>  'pakfire core update auto' => 'Core- und Addon-Updates automatisch installieren:',
> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
> index e30ee4f69..f90e3103b 100644
> --- a/langs/en/cgi-bin/en.pl
> +++ b/langs/en/cgi-bin/en.pl
> @@ -2041,6 +2041,7 @@
>  'pagerefresh' => 'Page is beeing refreshed, please wait.',
>  'pakfire accept all' => 'Do you want to install all packages?',
>  'pakfire ago' => 'ago.',
> +'pakfire already busy' => 'Pakfire is already performing a task. Please try again later.',
>  'pakfire available addons' => 'Available Addons:',
>  'pakfire configuration' => 'Pakfire Configuration',
>  'pakfire core update auto' => 'Install core and addon updates automatically:',

Re: [PATCH 5/7] pakfire.cgi: Implement Post/Redirect/Get pattern

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 6185 bytes --]

Acked-by: Peter Müller <peter.muelle(a)ipfire.org>

> Refreshing the Pakfire page may cause a command to be
> executed multiple times and induce odd errors.
> 
> This patch implements a HTTP 303 redirect after form processing,
> which causes the browser to discard the POST form data.
> Navigating backward or reloading the page now does not trigger
> multiple executions anymore.
> 
> Fixes: #12781
> 
> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
> ---
>  html/cgi-bin/pakfire.cgi | 56 +++++++++++++++++++++++++++++++++++-----
>  1 file changed, 50 insertions(+), 6 deletions(-)
> 
> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
> index ec3ee2cc6..6fade81bd 100644
> --- a/html/cgi-bin/pakfire.cgi
> +++ b/html/cgi-bin/pakfire.cgi
> @@ -21,6 +21,7 @@
>  
>  use strict;
>  use List::Util qw(any);
> +use URI;
>  
>  # enable only the following on debugging purpose
>  #use warnings;
> @@ -37,12 +38,17 @@ my %color = ();
>  my %pakfiresettings = ();
>  my %mainsettings = ();
>  
> +# The page mode is used to explictly switch between user interface functions:
> +my $PM_DEFAULT = 'default'; # Default user interface with command processing
> +my $PM_LOGREAD = 'logread'; # Log messages viewer (ignores all commands)
> +my $pagemode = $PM_DEFAULT;
> +
>  # Load general settings
>  &General::readhash("${General::swroot}/main/settings", \%mainsettings);
>  &General::readhash("${General::swroot}/pakfire/settings", \%pakfiresettings);
>  &General::readhash("/srv/web/ipfire/html/themes/ipfire/include/colors.txt", \%color);
>  
> -# Get CGI request data
> +# Get CGI POST request data
>  $cgiparams{'ACTION'} = '';
>  $cgiparams{'FORCE'} = '';
>  
> @@ -51,6 +57,17 @@ $cgiparams{'DELPAKS'} = '';
>  
>  &Header::getcgihash(\%cgiparams);
>  
> +# Get CGI GET request data (if available)
> +if($ENV{'QUERY_STRING'}) {
> +	my $uri = URI->new($ENV{'REQUEST_URI'});
> +	my %query = $uri->query_form;
> +
> +	my $mode = lc($query{'mode'} // '');
> +	if(($mode eq $PM_DEFAULT) || ($mode eq $PM_LOGREAD)) {
> +		$pagemode = $mode; # Limit to existing modes
> +	}
> +}
> +
>  ### Process AJAX/JSON request ###
>  if($cgiparams{'ACTION'} eq 'json-getstatus') {
>  	# Send HTTP headers
> @@ -96,19 +113,24 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') {
>  }
>  
>  ### Process Pakfire install/update commands ###
> -if($cgiparams{'ACTION'} ne '') {
> +if(($cgiparams{'ACTION'} ne '') && ($pagemode eq $PM_DEFAULT)) {
>  	if(&_is_pakfire_busy()) {
>  		$errormessage = $Lang::tr{'pakfire already busy'};
> +		$pagemode = $PM_LOGREAD; # Running Pakfire instance found, switch to log viewer mode
>  	} elsif(($cgiparams{'ACTION'} eq 'install') && ($cgiparams{'FORCE'} eq 'on')) {
>  		my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
>  		&General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive", "--no-colors", @pkgs);
> +		&_http_pagemode_redirect($PM_LOGREAD, 1);
>  	} elsif(($cgiparams{'ACTION'} eq 'remove') && ($cgiparams{'FORCE'} eq 'on')) {
>  		my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
>  		&General::system_background("/usr/local/bin/pakfire", "remove", "--non-interactive", "--no-colors", @pkgs);
> +		&_http_pagemode_redirect($PM_LOGREAD, 1);
>  	} elsif($cgiparams{'ACTION'} eq 'update') {
>  		&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
> +		&_http_pagemode_redirect($PM_LOGREAD, 1);
>  	} elsif($cgiparams{'ACTION'} eq 'upgrade') {
>  		&General::system_background("/usr/local/bin/pakfire", "upgrade", "-y", "--no-colors");
> +		&_http_pagemode_redirect($PM_LOGREAD, 1);
>  	} elsif($cgiparams{'ACTION'} eq $Lang::tr{'save'}) {
>  		$pakfiresettings{"TREE"} = $cgiparams{"TREE"};
>  
> @@ -122,6 +144,7 @@ if($cgiparams{'ACTION'} ne '') {
>  
>  			# Update lists
>  			&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
> +			&_http_pagemode_redirect($PM_LOGREAD, 1);
>  		}
>  	}
>  }
> @@ -221,8 +244,8 @@ if ($errormessage) {
>  	&Header::closebox();
>  }
>  
> -# Show log output while Pakfire is running
> -if(&_is_pakfire_busy()) {
> +# Show only log output while Pakfire is running and stop afterwards
> +if(($pagemode eq $PM_LOGREAD) || (&_is_pakfire_busy())) {
>  	&Header::openbox("100%", "center", "Pakfire");
>  
>  	print <<END
> @@ -253,7 +276,8 @@ END
>  }
>  
>  # Show Pakfire install/remove dependencies and confirm form
> -if (($cgiparams{'ACTION'} eq 'install') && (! &_is_pakfire_busy())) {
> +# (_is_pakfire_busy status was checked before and can be omitted)
> +if (($cgiparams{'ACTION'} eq 'install') && ($pagemode eq $PM_DEFAULT)) {
>  	&Header::openbox("100%", "center", $Lang::tr{'request'});
>  
>  	my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
> @@ -291,7 +315,7 @@ END
>  	&Header::closepage();
>  	exit;
>  
> -} elsif (($cgiparams{'ACTION'} eq 'remove') && (! &_is_pakfire_busy())) {
> +} elsif (($cgiparams{'ACTION'} eq 'remove') && ($pagemode eq $PM_DEFAULT)) {
>  	&Header::openbox("100%", "center", $Lang::tr{'request'});
>  
>  	my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
> @@ -476,3 +500,23 @@ sub _start_json_output {
>  	print "Content-Type: application/json\n";
>  	print "\n"; # End of HTTP headers
>  }
> +
> +# Send HTTP 303 redirect headers to change page mode
> +# GET is always used to display the redirected page, which will remove already processed POST form data.
> +# Note: Custom headers must be sent before the HTML output is started by &Header::showhttpheaders().
> +# If switch_mode is set to true, the global page mode variable ("$pagemode") is also updated immediately.
> +sub _http_pagemode_redirect {
> +	my ($mode, $switch_mode) = @_;
> +	$mode //= $PM_DEFAULT;
> +	$switch_mode //= 0;
> +
> +	# Send HTTP redirect with GET parameter
> +	my $location = "https://$ENV{'SERVER_NAME'}:$ENV{'SERVER_PORT'}$ENV{'SCRIPT_NAME'}?mode=${mode}";
> +	print "Status: 303 See Other\n";
> +	print "Location: $location\n";
> +
> +	# Change global page mode
> +	if($switch_mode) {
> +		$pagemode = $mode;
> +	}
> +}

Re: [PATCH 6/7] pakfire.cgi: Discard tac stderr output

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]

Acked-by: Peter Müller <peter.muelle(a)ipfire.org>

> Prevents meaningless "broken pipe" messages in the httpd error log.
> 
> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
> ---
>  html/cgi-bin/pakfire.cgi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
> index 6fade81bd..489b07a6d 100644
> --- a/html/cgi-bin/pakfire.cgi
> +++ b/html/cgi-bin/pakfire.cgi
> @@ -75,7 +75,7 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') {
>  
>  	# Read /var/log/messages backwards until a "Pakfire started" header is found,
>  	# to capture all messages of the last (i.e. current) Pakfire run
> -	my @messages = `tac /var/log/messages | sed -n '/pakfire:/{p;/Pakfire.*started/q}'`;
> +	my @messages = `tac /var/log/messages 2>/dev/null | sed -n '/pakfire:/{p;/Pakfire.*started/q}'`;
>  
>  	# Test if the log contains an error message (fastest implementation, stops at first match)
>  	my $failure = any{ index($_, 'ERROR') != -1 } @messages;

Re: [PATCH 7/7] pakfire.cgi: Cosmetic fixes

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 2523 bytes --]

Acked-by: Peter Müller <peter.muelle(a)ipfire.org>

> Add formatting to improve readability of dependencies list header.
> 
> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
> ---
>  html/cgi-bin/pakfire.cgi | 6 ++++--
>  langs/fr/cgi-bin/fr.pl   | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
> index 489b07a6d..3e8dc5460 100644
> --- a/html/cgi-bin/pakfire.cgi
> +++ b/html/cgi-bin/pakfire.cgi
> @@ -283,7 +283,8 @@ if (($cgiparams{'ACTION'} eq 'install') && ($pagemode eq $PM_DEFAULT)) {
>  	my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
>  	my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
>  	print <<END;
> -	<table><tr><td colspan='2'>$Lang::tr{'pakfire install package'} @pkgs $Lang::tr{'pakfire possible dependency'}
> +	<table style="width: 100%"><tr><td colspan='2'><p>$Lang::tr{'pakfire install package'} <strong>@{pkgs}</strong><br>
> +		$Lang::tr{'pakfire possible dependency'}</p>
>  		<pre>
>  END
>  	foreach (@output) {
> @@ -321,7 +322,8 @@ END
>  	my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
>  	my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
>  	print <<END;
> -	<table><tr><td colspan='2'>$Lang::tr{'pakfire uninstall package'} @pkgs $Lang::tr{'pakfire possible dependency'}
> +	<table style="width: 100%"><tr><td colspan='2'><p>$Lang::tr{'pakfire uninstall package'} <strong>@{pkgs}</strong><br>
> +		$Lang::tr{'pakfire possible dependency'}</p>
>  		<pre>
>  END
>  	foreach (@output) {
> diff --git a/langs/fr/cgi-bin/fr.pl b/langs/fr/cgi-bin/fr.pl
> index bd17df4ee..6dbeebc16 100644
> --- a/langs/fr/cgi-bin/fr.pl
> +++ b/langs/fr/cgi-bin/fr.pl
> @@ -2055,7 +2055,7 @@
>  'pakfire last package update' => 'Dernière mise à jour de la liste des paquets : ',
>  'pakfire last serverlist update' => 'Dernière mise à jour de la liste des serveurs : ',
>  'pakfire last update' => 'Dernière mise à jour : ',
> -'pakfire possible dependency' => '<br><br>Il y a peut-être des dépendances, voici la liste des paquets qu\'il faut (dés)installer.<br>',
> +'pakfire possible dependency' => 'Il y a peut-être des dépendances, voici la liste des paquets qu\'il faut (dés)installer.',
>  'pakfire register' => 'S\'inscrire au serveur pakfire :',
>  'pakfire return' => 'Retour à Pakfire',
>  'pakfire system state' => 'Statut système PakFire ',

Re: [PATCH 5/7] pakfire.cgi: Implement Post/Redirect/Get pattern

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 6436 bytes --]

Hello,

> On 8 May 2022, at 13:09, Leo-Andres Hofmann <hofmann(a)leo-andres.de> wrote:
> 
> Refreshing the Pakfire page may cause a command to be
> executed multiple times and induce odd errors.
> 
> This patch implements a HTTP 303 redirect after form processing,
> which causes the browser to discard the POST form data.
> Navigating backward or reloading the page now does not trigger
> multiple executions anymore.
> 
> Fixes: #12781
> 
> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
> ---
> html/cgi-bin/pakfire.cgi | 56 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 50 insertions(+), 6 deletions(-)
> 
> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
> index ec3ee2cc6..6fade81bd 100644
> --- a/html/cgi-bin/pakfire.cgi
> +++ b/html/cgi-bin/pakfire.cgi
> @@ -21,6 +21,7 @@
> 
> use strict;
> use List::Util qw(any);
> +use URI;
> 
> # enable only the following on debugging purpose
> #use warnings;
> @@ -37,12 +38,17 @@ my %color = ();
> my %pakfiresettings = ();
> my %mainsettings = ();
> 
> +# The page mode is used to explictly switch between user interface functions:
> +my $PM_DEFAULT = 'default'; # Default user interface with command processing
> +my $PM_LOGREAD = 'logread'; # Log messages viewer (ignores all commands)
> +my $pagemode = $PM_DEFAULT;
> +
> # Load general settings
> &General::readhash("${General::swroot}/main/settings", \%mainsettings);
> &General::readhash("${General::swroot}/pakfire/settings", \%pakfiresettings);
> &General::readhash("/srv/web/ipfire/html/themes/ipfire/include/colors.txt", \%color);
> 
> -# Get CGI request data
> +# Get CGI POST request data
> $cgiparams{'ACTION'} = '';
> $cgiparams{'FORCE'} = '';
> 
> @@ -51,6 +57,17 @@ $cgiparams{'DELPAKS'} = '';
> 
> &Header::getcgihash(\%cgiparams);
> 
> +# Get CGI GET request data (if available)
> +if($ENV{'QUERY_STRING'}) {
> +	my $uri = URI->new($ENV{'REQUEST_URI'});
> +	my %query = $uri->query_form;
> +
> +	my $mode = lc($query{'mode'} // '');
> +	if(($mode eq $PM_DEFAULT) || ($mode eq $PM_LOGREAD)) {
> +		$pagemode = $mode; # Limit to existing modes
> +	}
> +}
> +
> ### Process AJAX/JSON request ###
> if($cgiparams{'ACTION'} eq 'json-getstatus') {
> 	# Send HTTP headers
> @@ -96,19 +113,24 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') {
> }
> 
> ### Process Pakfire install/update commands ###
> -if($cgiparams{'ACTION'} ne '') {
> +if(($cgiparams{'ACTION'} ne '') && ($pagemode eq $PM_DEFAULT)) {
> 	if(&_is_pakfire_busy()) {
> 		$errormessage = $Lang::tr{'pakfire already busy'};
> +		$pagemode = $PM_LOGREAD; # Running Pakfire instance found, switch to log viewer mode
> 	} elsif(($cgiparams{'ACTION'} eq 'install') && ($cgiparams{'FORCE'} eq 'on')) {
> 		my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
> 		&General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive", "--no-colors", @pkgs);
> +		&_http_pagemode_redirect($PM_LOGREAD, 1);
> 	} elsif(($cgiparams{'ACTION'} eq 'remove') && ($cgiparams{'FORCE'} eq 'on')) {
> 		my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
> 		&General::system_background("/usr/local/bin/pakfire", "remove", "--non-interactive", "--no-colors", @pkgs);
> +		&_http_pagemode_redirect($PM_LOGREAD, 1);
> 	} elsif($cgiparams{'ACTION'} eq 'update') {
> 		&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
> +		&_http_pagemode_redirect($PM_LOGREAD, 1);
> 	} elsif($cgiparams{'ACTION'} eq 'upgrade') {
> 		&General::system_background("/usr/local/bin/pakfire", "upgrade", "-y", "--no-colors");
> +		&_http_pagemode_redirect($PM_LOGREAD, 1);
> 	} elsif($cgiparams{'ACTION'} eq $Lang::tr{'save'}) {
> 		$pakfiresettings{"TREE"} = $cgiparams{"TREE"};
> 
> @@ -122,6 +144,7 @@ if($cgiparams{'ACTION'} ne '') {
> 
> 			# Update lists
> 			&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
> +			&_http_pagemode_redirect($PM_LOGREAD, 1);
> 		}
> 	}
> }
> @@ -221,8 +244,8 @@ if ($errormessage) {
> 	&Header::closebox();
> }
> 
> -# Show log output while Pakfire is running
> -if(&_is_pakfire_busy()) {
> +# Show only log output while Pakfire is running and stop afterwards
> +if(($pagemode eq $PM_LOGREAD) || (&_is_pakfire_busy())) {
> 	&Header::openbox("100%", "center", "Pakfire");
> 
> 	print <<END
> @@ -253,7 +276,8 @@ END
> }
> 
> # Show Pakfire install/remove dependencies and confirm form
> -if (($cgiparams{'ACTION'} eq 'install') && (! &_is_pakfire_busy())) {
> +# (_is_pakfire_busy status was checked before and can be omitted)
> +if (($cgiparams{'ACTION'} eq 'install') && ($pagemode eq $PM_DEFAULT)) {
> 	&Header::openbox("100%", "center", $Lang::tr{'request'});
> 
> 	my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
> @@ -291,7 +315,7 @@ END
> 	&Header::closepage();
> 	exit;
> 
> -} elsif (($cgiparams{'ACTION'} eq 'remove') && (! &_is_pakfire_busy())) {
> +} elsif (($cgiparams{'ACTION'} eq 'remove') && ($pagemode eq $PM_DEFAULT)) {
> 	&Header::openbox("100%", "center", $Lang::tr{'request'});
> 
> 	my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
> @@ -476,3 +500,23 @@ sub _start_json_output {
> 	print "Content-Type: application/json\n";
> 	print "\n"; # End of HTTP headers
> }
> +
> +# Send HTTP 303 redirect headers to change page mode
> +# GET is always used to display the redirected page, which will remove already processed POST form data.
> +# Note: Custom headers must be sent before the HTML output is started by &Header::showhttpheaders().
> +# If switch_mode is set to true, the global page mode variable ("$pagemode") is also updated immediately.
> +sub _http_pagemode_redirect {
> +	my ($mode, $switch_mode) = @_;
> +	$mode //= $PM_DEFAULT;
> +	$switch_mode //= 0;
> +
> +	# Send HTTP redirect with GET parameter
> +	my $location = "https://$ENV{'SERVER_NAME'}:$ENV{'SERVER_PORT'}$ENV{'SCRIPT_NAME'}?mode=${mode}";
> +	print "Status: 303 See Other\n";
> +	print "Location: $location\n";

I believe that technically you would want another newline at the end of the header.

Would you also not want to call “exit(0)” here to finish processing the script? What else is there to do after you have redirected the user?

-Michael

> +
> +	# Change global page mode
> +	if($switch_mode) {
> +		$pagemode = $mode;
> +	}
> +}
> -- 
> 2.27.0.windows.1
> 

Re: [PATCH 5/7] pakfire.cgi: Implement Post/Redirect/Get pattern

[hofmann]

[-- Attachment #1: Type: text/plain, Size: 7550 bytes --]

Hi,
Please excuse my delayed reply, I wanted to test core 168 first.

12. Mai 2022 11:30, "Michael Tremer" <michael.tremer(a)ipfire.org> schrieb:

> Hello,
> 
>> On 8 May 2022, at 13:09, Leo-Andres Hofmann <hofmann(a)leo-andres.de> wrote:
>> 
>> Refreshing the Pakfire page may cause a command to be
>> executed multiple times and induce odd errors.
>> 
>> This patch implements a HTTP 303 redirect after form processing,
>> which causes the browser to discard the POST form data.
>> Navigating backward or reloading the page now does not trigger
>> multiple executions anymore.
>> 
>> Fixes: #12781
>> 
>> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
>> ---
>> html/cgi-bin/pakfire.cgi | 56 +++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 50 insertions(+), 6 deletions(-)
>> 
>> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
>> index ec3ee2cc6..6fade81bd 100644
>> --- a/html/cgi-bin/pakfire.cgi
>> +++ b/html/cgi-bin/pakfire.cgi
>> @@ -21,6 +21,7 @@
>> 
>> use strict;
>> use List::Util qw(any);
>> +use URI;
>> 
>> # enable only the following on debugging purpose
>> #use warnings;
>> @@ -37,12 +38,17 @@ my %color = ();
>> my %pakfiresettings = ();
>> my %mainsettings = ();
>> 
>> +# The page mode is used to explictly switch between user interface functions:
>> +my $PM_DEFAULT = 'default'; # Default user interface with command processing
>> +my $PM_LOGREAD = 'logread'; # Log messages viewer (ignores all commands)
>> +my $pagemode = $PM_DEFAULT;
>> +
>> # Load general settings
>> &General::readhash("${General::swroot}/main/settings", \%mainsettings);
>> &General::readhash("${General::swroot}/pakfire/settings", \%pakfiresettings);
>> &General::readhash("/srv/web/ipfire/html/themes/ipfire/include/colors.txt", \%color);
>> 
>> -# Get CGI request data
>> +# Get CGI POST request data
>> $cgiparams{'ACTION'} = '';
>> $cgiparams{'FORCE'} = '';
>> 
>> @@ -51,6 +57,17 @@ $cgiparams{'DELPAKS'} = '';
>> 
>> &Header::getcgihash(\%cgiparams);
>> 
>> +# Get CGI GET request data (if available)
>> +if($ENV{'QUERY_STRING'}) {
>> + my $uri = URI->new($ENV{'REQUEST_URI'});
>> + my %query = $uri->query_form;
>> +
>> + my $mode = lc($query{'mode'} // '');
>> + if(($mode eq $PM_DEFAULT) || ($mode eq $PM_LOGREAD)) {
>> + $pagemode = $mode; # Limit to existing modes
>> + }
>> +}
>> +
>> ### Process AJAX/JSON request ###
>> if($cgiparams{'ACTION'} eq 'json-getstatus') {
>> # Send HTTP headers
>> @@ -96,19 +113,24 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') {
>> }
>> 
>> ### Process Pakfire install/update commands ###
>> -if($cgiparams{'ACTION'} ne '') {
>> +if(($cgiparams{'ACTION'} ne '') && ($pagemode eq $PM_DEFAULT)) {
>> if(&_is_pakfire_busy()) {
>> $errormessage = $Lang::tr{'pakfire already busy'};
>> + $pagemode = $PM_LOGREAD; # Running Pakfire instance found, switch to log viewer mode
>> } elsif(($cgiparams{'ACTION'} eq 'install') && ($cgiparams{'FORCE'} eq 'on')) {
>> my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
>> &General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive",
>> "--no-colors", @pkgs);
>> + &_http_pagemode_redirect($PM_LOGREAD, 1);
>> } elsif(($cgiparams{'ACTION'} eq 'remove') && ($cgiparams{'FORCE'} eq 'on')) {
>> my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
>> &General::system_background("/usr/local/bin/pakfire", "remove", "--non-interactive", "--no-colors",
>> @pkgs);
>> + &_http_pagemode_redirect($PM_LOGREAD, 1);
>> } elsif($cgiparams{'ACTION'} eq 'update') {
>> &General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
>> + &_http_pagemode_redirect($PM_LOGREAD, 1);
>> } elsif($cgiparams{'ACTION'} eq 'upgrade') {
>> &General::system_background("/usr/local/bin/pakfire", "upgrade", "-y", "--no-colors");
>> + &_http_pagemode_redirect($PM_LOGREAD, 1);
>> } elsif($cgiparams{'ACTION'} eq $Lang::tr{'save'}) {
>> $pakfiresettings{"TREE"} = $cgiparams{"TREE"};
>> 
>> @@ -122,6 +144,7 @@ if($cgiparams{'ACTION'} ne '') {
>> 
>> # Update lists
>> &General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
>> + &_http_pagemode_redirect($PM_LOGREAD, 1);
>> }
>> }
>> }
>> @@ -221,8 +244,8 @@ if ($errormessage) {
>> &Header::closebox();
>> }
>> 
>> -# Show log output while Pakfire is running
>> -if(&_is_pakfire_busy()) {
>> +# Show only log output while Pakfire is running and stop afterwards
>> +if(($pagemode eq $PM_LOGREAD) || (&_is_pakfire_busy())) {
>> &Header::openbox("100%", "center", "Pakfire");
>> 
>> print <<END
>> @@ -253,7 +276,8 @@ END
>> }
>> 
>> # Show Pakfire install/remove dependencies and confirm form
>> -if (($cgiparams{'ACTION'} eq 'install') && (! &_is_pakfire_busy())) {
>> +# (_is_pakfire_busy status was checked before and can be omitted)
>> +if (($cgiparams{'ACTION'} eq 'install') && ($pagemode eq $PM_DEFAULT)) {
>> &Header::openbox("100%", "center", $Lang::tr{'request'});
>> 
>> my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
>> @@ -291,7 +315,7 @@ END
>> &Header::closepage();
>> exit;
>> 
>> -} elsif (($cgiparams{'ACTION'} eq 'remove') && (! &_is_pakfire_busy())) {
>> +} elsif (($cgiparams{'ACTION'} eq 'remove') && ($pagemode eq $PM_DEFAULT)) {
>> &Header::openbox("100%", "center", $Lang::tr{'request'});
>> 
>> my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
>> @@ -476,3 +500,23 @@ sub _start_json_output {
>> print "Content-Type: application/json\n";
>> print "\n"; # End of HTTP headers
>> }
>> +
>> +# Send HTTP 303 redirect headers to change page mode
>> +# GET is always used to display the redirected page, which will remove already processed POST form
>> data.
>> +# Note: Custom headers must be sent before the HTML output is started by
>> &Header::showhttpheaders().
>> +# If switch_mode is set to true, the global page mode variable ("$pagemode") is also updated
>> immediately.
>> +sub _http_pagemode_redirect {
>> + my ($mode, $switch_mode) = @_;
>> + $mode //= $PM_DEFAULT;
>> + $switch_mode //= 0;
>> +
>> + # Send HTTP redirect with GET parameter
>> + my $location = "https://$ENV{'SERVER_NAME'}:$ENV{'SERVER_PORT'}$ENV{'SCRIPT_NAME'}?mode=${mode}";
>> + print "Status: 303 See Other\n";
>> + print "Location: $location\n";
> 
> I believe that technically you would want another newline at the end of the header.

Yes the second newline would terminate the header. I want Header::showhttpheaders() to be able to
print it's headers later on, so I don't close the header yet.

> Would you also not want to call “exit(0)” here to finish processing the script? What else is there
> to do after you have redirected the user?

I found that sometimes Perl did not close the connection for a long time, probably until the forked
Pakfire process terminated.
If this happened, the browser waited and did not immediately follow the redirect. However, it was able to start rendering the page received so far.

That's why I decided to send a redirect header and additionally generate the log viewer before
exiting. This way the user hopefully never gets to see a blank page.

With core 168 installed on my test system I noticed that it happens much less often now.
Personally I found this very difficult to reproduce and would like to leave my solution it as it is.

Best regards
Leo

> -Michael
> 
>> +
>> + # Change global page mode
>> + if($switch_mode) {
>> + $pagemode = $mode;
>> + }
>> +}
>> --
>> 2.27.0.windows.1