fix: prevent default mod_perl error message to be appended (#7331)

* fix: prevent default mod_perl error message to be appended * add return
openfoodfacts · Sep 19, 2022 · 978fe5d · 978fe5d
1 parent 78edefd
commit 978fe5d
Show file tree

Hide file tree

Showing 16 changed files with 97 additions and 58 deletions.
diff --git a/cgi/display.pl b/cgi/display.pl
@@ -36,7 +36,7 @@
 use Log::Any qw($log);
 
 use Apache2::RequestRec ();
-use Apache2::Const ();
+use Apache2::Const qw(:common);
 
 # The nginx reverse proxy turns /somepath?someparam=somevalue to /cgi/display.pl?/somepath?someparam=somevalue
 # so that all non /cgi/ queries are sent to display.pl and that we can get the path in the query string
@@ -60,14 +60,23 @@
 # analyze request will fill request with action and parameters
 analyze_request($request_ref);
 
-$log->debug("after analyze_request", { blogid => $request_ref->{blogid}, tagid => $request_ref->{tagid}, urlsdate => $request_ref->{urlsdate}, urlid => $request_ref->{urlid}, user => $request_ref->{user}, query => $request_ref->{query} });
+# If we have an error, display the error page and return
+
+if (defined $request_ref->{error_status}) {
+	$log->debug("analyze_request error", { request_ref => $request_ref });
+	display_error($request_ref->{error_message}, $request_ref->{error_status});
+	$log->debug("analyze_request error - return Apache2::Const::OK");
+	return Apache2::Const::OK;
+}
+
+$log->debug("after analyze_request", { tagid => $request_ref->{tagid}, urlsdate => $request_ref->{urlsdate}, urlid => $request_ref->{urlid}, user => $request_ref->{user}, query => $request_ref->{query} });
 
 # Only display texts if products are private and no owner is defined
 if ( ((defined $server_options{private_products}) and ($server_options{private_products}))
 	and ((defined $request_ref->{api}) or (defined $request_ref->{product}) or (defined $request_ref->{groupby_tagtype}) or ((defined $request_ref->{tagtype}) and (defined $request_ref->{tagid})))
 	and (not defined $Owner_id)) {
 
-	display_error(lang("no_owner_defined"), 200);
+	display_error_and_exit(lang("no_owner_defined"), 200);
 }
 
 if ((defined $request_ref->{api}) and (defined $request_ref->{api_method})) {

diff --git a/cgi/export_products.pl b/cgi/export_products.pl
@@ -58,7 +58,7 @@
 my $html = '';
 
 if (not defined $Owner_id) {
-	display_error(lang("no_owner_defined"), 200);
+	display_error_and_exit(lang("no_owner_defined"), 200);
 }
 
 # Require moderator status to launch the export / import process,

diff --git a/cgi/import_file_process.pl b/cgi/import_file_process.pl
@@ -56,7 +56,7 @@
 my $template_data_ref;
 
 if (not defined $Owner_id) {
-	display_error(lang("no_owner_defined"), 200);
+	display_error_and_exit(lang("no_owner_defined"), 200);
 }
 
 my $import_files_ref = retrieve("$data_root/import_files/${Owner_id}/import_files.sto");
@@ -77,7 +77,7 @@
 }
 else {
 	$log->debug("File not found in import_files.sto", { file_id => $file_id }) if $log->is_debug();
-	display_error("File not found.", 404);
+	display_error_and_exit("File not found.", 404);
 }
 
 $log->debug("File found in import_files.sto", { file_id => $file_id,  file => $file, extension => $extension, import_file => $import_files_ref->{$file_id} }) if $log->is_debug();
@@ -92,7 +92,7 @@
 my $results_ref = load_csv_or_excel_file($file);
 
 if ($results_ref->{error}) {
-	display_error($results_ref->{error}, 200);
+	display_error_and_exit($results_ref->{error}, 200);
 }
 
 my $headers_ref = $results_ref->{headers};
@@ -147,7 +147,7 @@
 if ($results_ref->{error}) {
 	$import_files_ref->{$file_id}{imports}{$import_id}{convert_error} = $results_ref->{error};
 	store("$data_root/import_files/${Owner_id}/import_files.sto", $import_files_ref);
-	display_error($results_ref->{error}, 200);
+	display_error_and_exit($results_ref->{error}, 200);
 }
 
 my $args_ref = {

diff --git a/cgi/import_file_select_format.pl b/cgi/import_file_select_format.pl
@@ -60,7 +60,7 @@
 my $template_data_ref = {};
 
 if (not defined $Owner_id) {
-	display_error(lang("no_owner_defined"), 200);
+	display_error_and_exit(lang("no_owner_defined"), 200);
 }
 
 my $import_files_ref = retrieve("$data_root/import_files/${Owner_id}/import_files.sto");
@@ -82,7 +82,7 @@
 }
 else {
 	$log->debug("File not found in import_files.sto", { file_id => $file_id }) if $log->is_debug();
-	display_error("File not found.", 404);
+	display_error_and_exit("File not found.", 404);
 }
 
 $log->debug("File found in import_files.sto", { file_id => $file_id,  file => $file, extension => $extension, import_file => $import_files_ref->{$file_id} }) if $log->is_debug();
@@ -93,7 +93,7 @@
 	my $results_ref = load_csv_or_excel_file($file);
 
 	if ($results_ref->{error}) {
-		display_error($results_ref->{error}, 200);
+		display_error_and_exit($results_ref->{error}, 200);
 	}
 
 	my $headers_ref = $results_ref->{headers};

diff --git a/cgi/import_file_upload.pl b/cgi/import_file_upload.pl
@@ -58,7 +58,7 @@
 local $log->context->{action} = $action;
 
 if (not defined $Owner_id) {
-	display_error(lang("no_owner_defined"), 200);
+	display_error_and_exit(lang("no_owner_defined"), 200);
 }
 
 if ($action eq "process") {

diff --git a/cgi/import_photos_upload.pl b/cgi/import_photos_upload.pl
@@ -59,7 +59,7 @@
 local $log->context->{action} = $action;
 
 if (not defined $Owner_id) {
-	display_error(lang("no_owner_defined"), 200);
+	display_error_and_exit(lang("no_owner_defined"), 200);
 }
 
 

diff --git a/cgi/import_products_categories_from_public_database.pl b/cgi/import_products_categories_from_public_database.pl
@@ -58,7 +58,7 @@
 my $template_data_ref = {};
 
 if (not defined $Owner_id) {
-	display_error(lang("no_owner_defined"), 200);
+	display_error_and_exit(lang("no_owner_defined"), 200);
 }
 
 if ($action eq "display") {

diff --git a/cgi/org.pl b/cgi/org.pl
@@ -70,7 +70,7 @@
 		$template_data_ref->{org_does_not_exist} = 1;
 	}
 	else {
-		display_error($Lang{error_org_does_not_exist}{$lang}, 404);
+		display_error_and_exit($Lang{error_org_does_not_exist}{$lang}, 404);
 	}
 }
 
@@ -80,7 +80,7 @@
 	$log->debug("user does not have permission to edit org",
 		{orgid => $orgid, org_admins => $org_ref->{admins}, User_id => $User_id})
 	  if $log->is_debug();
-	display_error($Lang{error_no_permission}{$lang}, 403);
+	display_error_and_exit($Lang{error_no_permission}{$lang}, 403);
 }
 
 my @errors = ();
@@ -93,7 +93,7 @@
 				$type = 'delete';
 			}
 			else {
-				display_error($Lang{error_no_permission}{$lang}, 403);
+				display_error_and_exit($Lang{error_no_permission}{$lang}, 403);
 			}
 		}
 		else {

diff --git a/cgi/product_image.pl b/cgi/product_image.pl
@@ -52,19 +52,19 @@
 $log->debug("start", {code => $code, id => $id}) if $log->is_debug();
 
 if (not defined $code) {
-	display_error(sprintf(lang("no_product_for_barcode"), $code), 404);
+	display_error_and_exit(sprintf(lang("no_product_for_barcode"), $code), 404);
 }
 
 my $product_id = product_id_for_owner($Owner_id, $code);
 
 my $product_ref = retrieve_product($product_id);
 
 if (not(defined $product_ref)) {
-	display_error(sprintf(lang("no_product_for_barcode"), $code), 404);
+	display_error_and_exit(sprintf(lang("no_product_for_barcode"), $code), 404);
 }
 
 if ((not(defined $product_ref->{images})) or (not(defined $product_ref->{images}{$id}))) {
-	display_error(sprintf(lang("no_product_for_barcode"), $code), 404);
+	display_error_and_exit(sprintf(lang("no_product_for_barcode"), $code), 404);
 }
 
 my $imagetext;

diff --git a/cgi/product_multilingual.pl b/cgi/product_multilingual.pl
@@ -61,7 +61,7 @@
 my $request_ref = ProductOpener::Display::init_request();
 
 if ($User_id eq 'unwanted-user-french') {
-	display_error(
+	display_error_and_exit(
 		"<b>Il y a des problèmes avec les modifications de produits que vous avez effectuées. Ce compte est temporairement bloqué, merci de nous contacter.</b>",
 		403
 	);
@@ -96,7 +96,7 @@
 		$code = process_search_image_form(\$filename);
 	}
 	elsif ($code !~ /^\d{4,24}$/) {
-		display_error($Lang{invalid_barcode}{$lang}, 403);
+		display_error_and_exit($Lang{invalid_barcode}{$lang}, 403);
 	}
 
 	my $r = Apache2::RequestUtil->request();
@@ -187,28 +187,28 @@
 else {
 	# We should have a code
 	if ((not defined $code) or ($code eq '')) {
-		display_error($Lang{missing_barcode}{$lang}, 403);
+		display_error_and_exit($Lang{missing_barcode}{$lang}, 403);
 	}
 	elsif ($code !~ /^\d{4,24}$/) {
-		display_error($Lang{invalid_barcode}{$lang}, 403);
+		display_error_and_exit($Lang{invalid_barcode}{$lang}, 403);
 	}
 	else {
 		if (    ((defined $server_options{private_products}) and ($server_options{private_products}))
 			and (not defined $Owner_id))
 		{
 
-			display_error(lang("no_owner_defined"), 200);
+			display_error_and_exit(lang("no_owner_defined"), 200);
 		}
 		$product_id = product_id_for_owner($Owner_id, $code);
 		$product_ref = retrieve_product_or_deleted_product($product_id, $User{moderator});
 		if (not defined $product_ref) {
-			display_error(sprintf(lang("no_product_for_barcode"), $code), 404);
+			display_error_and_exit(sprintf(lang("no_product_for_barcode"), $code), 404);
 		}
 	}
 }
 
 if (($type eq 'delete') and (not $User{moderator})) {
-	display_error($Lang{error_no_permission}{$lang}, 403);
+	display_error_and_exit($Lang{error_no_permission}{$lang}, 403);
 }
 
 if ($User_id eq 'unwanted-bot-id') {
@@ -256,7 +256,7 @@
 
 	if (not $proceed_with_edit) {
 
-		display_error("Edit against edit rules", 403);
+		display_error_and_exit("Edit against edit rules", 403);
 	}
 
 	$log->debug("phase 1", {code => $code, type => $type}) if $log->is_debug();

diff --git a/cgi/remove_products.pl b/cgi/remove_products.pl
@@ -55,11 +55,11 @@
 $template_data_ref->{action} = $action;
 
 if (not $server_options{producers_platform}) {
-	display_error(lang("function_not_available"), 200);
+	display_error_and_exit(lang("function_not_available"), 200);
 }
 
 if ((not defined $Owner_id) or ($Owner_id !~ /^(user|org)-\S+$/)) {
-	display_error(lang("no_owner_defined"), 200);
+	display_error_and_exit(lang("no_owner_defined"), 200);
 }
 
 

diff --git a/cgi/reset_password.pl b/cgi/reset_password.pl
@@ -61,7 +61,7 @@
 my $html = '';
 
 if (defined $User_id) {
-	display_error($Lang{error_reset_already_connected}{$lang}, undef);
+	display_error_and_exit($Lang{error_reset_already_connected}{$lang}, undef);
 }
 
 if ($action eq 'process') {
@@ -107,7 +107,7 @@
 	}
 	else {
 		$log->debug("invalid address", {type => $type }) if $log->is_debug();
-		display_error(lang("error_invalid_address"), 404);
+		display_error_and_exit(lang("error_invalid_address"), 404);
 	}
 
 
@@ -190,7 +190,7 @@
 			}
 			else {
 				$log->debug("token is invalid", {userid => $userid }) if $log->is_debug();
-				display_error($Lang{error_reset_invalid_token}{$lang}, undef);
+				display_error_and_exit($Lang{error_reset_invalid_token}{$lang}, undef);
 			}
 		}
 	}

diff --git a/cgi/search.pl b/cgi/search.pl
@@ -55,7 +55,7 @@
 
 if (user_agent() =~ /apps-spreadsheets/) {
 
-	display_error(
+	display_error_and_exit(
 		"Automated queries using Google Spreadsheet overload the Open Food Facts server. We cannot support them. You can contact us at contact\@openfoodfacts.org to tell us about your use case, so that we can see if there is another way to support it.",
 		200
 	);

diff --git a/cgi/user.pl b/cgi/user.pl
@@ -87,15 +87,15 @@
 if ($type =~ /^edit/) {
 	$user_ref = retrieve("$data_root/users/$userid.sto");
 	if (not defined $user_ref) {
-		display_error($Lang{error_invalid_user}{$lang}, 404);
+		display_error_and_exit($Lang{error_invalid_user}{$lang}, 404);
 	}
 }
 else {
 	$type = 'add';
 }
 
 if (($type =~ /^edit/) and ($User_id ne $userid) and not $admin) {
-	display_error($Lang{error_no_permission}{$lang}, 403);
+	display_error_and_exit($Lang{error_no_permission}{$lang}, 403);
 }
 
 my $debug = 0;
@@ -109,7 +109,7 @@
 				$type = 'delete';
 			}
 			else {
-				display_error($Lang{error_no_permission}{$lang}, 403);
+				display_error_and_exit($Lang{error_no_permission}{$lang}, 403);
 			}
 		}
 	}