Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fixes redirects #7136

Merged
merged 2 commits into from
Jul 26, 2022
Merged

fix: fixes redirects #7136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10d2338
fix: fixes redirects
stephanegigandet Jul 26, 2022
bfdc913
suggestions from code review
stephanegigandet Jul 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions cgi/css.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,4 @@
. ".css?v="
. $file_timestamps{'css/dist/app-' . lang('text_direction') . '.css'};

my $r = Apache2::RequestUtil->request();
$r->headers_out->set(Location => $redirect);
$r->status(302);
return 302;
redirect(302, $redirect);
8 changes: 0 additions & 8 deletions cgi/display.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,4 @@
display_tag($request_ref);
}

if (defined $request_ref->{redirect}) {
my $r = shift;

$r->headers_out->set(Location => $request_ref->{redirect});
$r->status(301);
return 301;
}

exit 0;
68 changes: 44 additions & 24 deletions lib/ProductOpener/Display.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ BEGIN
use vars qw(@ISA @EXPORT_OK %EXPORT_TAGS);
@EXPORT_OK = qw(
&startup
&init
&init_request
&analyze_request
&redirect

&display_date
&display_date_tag
Expand Down Expand Up @@ -435,6 +436,33 @@ sub process_template($template_filename, $template_data_ref, $result_content_ref
}


=head2 redirect($status_code, $redirect_url)
Copy link
Member

Choose a reason for hiding this comment

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

to @yuktea : typically a function that, when we will split is part of "http utils" more than display.

Copy link
Member

Choose a reason for hiding this comment

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

didn't you forget to add it to export ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, I'll add it. I just noticed I was also exporting init which has been renamed to init_request


This function instructs mod_perl to print redirect HTTP header (Location) and to terminate the request immediately.
The mod_perl process is not terminated and will continue to serve future requests.

=head3 Arguments

=head4 Status code $status_code

e.g. 302 for a temporary redirect

=head4 Redirect url $redirect_url


=cut

sub redirect($status_code, $redirect_url) {

my $r = Apache2::RequestUtil->request();

$r->headers_out->set(Location => $redirect_url);
$r->status($status_code);
# note: under mod_perl, exit() will end the request without terminating the Apache mod_perl process
Copy link
Member

Choose a reason for hiding this comment

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

thanks for the comment :-)

exit();
}


=head2 init_request ()

C<init_request()> is called at the start of each new request (web page or API).
Expand Down Expand Up @@ -472,17 +500,13 @@ sub init_request() {
$bodyabout = '';
$admin = 0;

my $r = shift;
my $r = Apache2::RequestUtil->request();
Copy link
Member

Choose a reason for hiding this comment

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

you can remove the code below:

	if (not defined $r) {
		$r = Apache2::RequestUtil->request();
	}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks


$cc = 'world';
$lc = 'en';
@lcs = ();
$country = 'en:world';

if (not defined $r) {
$r = Apache2::RequestUtil->request();
}

$r->headers_out->set(Server => "Product Opener");
$r->headers_out->set("X-Frame-Options" => "DENY");
$r->headers_out->set("X-Content-Type-Options" => "nosniff");
Expand Down Expand Up @@ -553,11 +577,9 @@ sub init_request() {
}
elsif ($ENV{QUERY_STRING} !~ /(cgi|api)\//) {
# redirect
my $redirect = get_world_subdomain() . "/" . $ENV{QUERY_STRING};
$log->info("request could not be matched to a known format, redirecting", { subdomain => $subdomain, lc => $lc, cc => $cc, country => $country, redirect => $redirect }) if $log->is_info();
$r->headers_out->set(Location => $redirect);
$r->status(302);
return 302;
my $redirect_url = get_world_subdomain() . $ENV{QUERY_STRING};
$log->info("request could not be matched to a known format, redirecting", { subdomain => $subdomain, lc => $lc, cc => $cc, country => $country, redirect => $redirect_url }) if $log->is_info();
redirect(302, $redirect_url);
}

$lc =~ s/_.*//; # PT_PT doest not work yet: categories
Expand All @@ -573,11 +595,9 @@ sub init_request() {
if ((defined $lc) and (defined $cc) and (defined $country_languages{$cc}[0]) and ($country_languages{$cc}[0] eq $lc) and ($subdomain ne $cc) and ($subdomain !~ /^(ssl-)?api/) and ($r->method() eq 'GET') and ($ENV{QUERY_STRING} !~ /(cgi|api)\//)) {
# redirect
my $ccdom = format_subdomain($cc);
my $redirect = "$ccdom/" . $ENV{QUERY_STRING};
$log->info("lc is equal to first lc of the country, redirecting to countries main domain", { subdomain => $subdomain, lc => $lc, cc => $cc, country => $country, redirect => $redirect }) if $log->is_info();
$r->headers_out->set(Location => $redirect);
$r->status(302);
return 302;
my $redirect_url = $ccdom . $ENV{QUERY_STRING};
$log->info("lc is equal to first lc of the country, redirecting to countries main domain", { subdomain => $subdomain, lc => $lc, cc => $cc, country => $country, redirect => $redirect_url }) if $log->is_info();
redirect(302, $redirect_url);
}


Expand Down Expand Up @@ -917,7 +937,7 @@ sub analyze_request($request_ref) {
elsif ((defined $options{redirect_texts}) and (defined $options{redirect_texts}{$lang . "/" . $components[0]})) {
$request_ref->{redirect} = $formatted_subdomain . "/" . $options{redirect_texts}{$lang . "/" . $components[0]};
$log->info("renamed text, redirecting", { textid => $components[0], redirect => $request_ref->{redirect} }) if $log->is_info();
return 301;
redirect(302, $request_ref->{redirect});
alexgarel marked this conversation as resolved.
Show resolved Hide resolved
}

# First check if the request is for a text
Expand Down Expand Up @@ -976,7 +996,7 @@ sub analyze_request($request_ref) {
elsif ((scalar(@components) == 2) and ($components[0] eq '.well-known') and ($components[1] eq 'change-password')) {
$request_ref->{redirect} = $formatted_subdomain . '/cgi/change_password.pl';
$log->info('well-known password change page - redirecting', { redirect => $request_ref->{redirect} }) if $log->is_info();
return 307;
redirect(307, $request_ref->{redirect});
}

elsif ($#components == -1) {
Expand Down Expand Up @@ -2948,7 +2968,7 @@ sub display_points($request_ref) {
if ((defined $tagid) and ($newtagid ne $tagid) ) {
$request_ref->{redirect} = $formatted_subdomain . $request_ref->{current_link};
$log->info("newtagid does not equal the original tagid, redirecting", { newtagid => $newtagid, redirect => $request_ref->{redirect} }) if $log->is_info();
return 301;
redirect(302, $request_ref->{redirect});
}


Expand Down Expand Up @@ -3184,7 +3204,7 @@ sub display_tag($request_ref) {
$request_ref->{redirect} .= '.xml' if param("xml");
$request_ref->{redirect} .= '.jqm' if param("jqm");
$log->info("one or more tagids mismatch, redirecting to correct url", { redirect => $request_ref->{redirect} }) if $log->is_info();
return 302;
redirect(302, $request_ref->{redirect});
}

my $weblinks_html = '';
Expand Down Expand Up @@ -7363,8 +7383,8 @@ CSS
# Old UPC-12 in url? Redirect to EAN-13 url
if ($request_code ne $code) {
$request_ref->{redirect} = $request_ref->{canon_url};
$log->info("301 redirecting user because request_code does not match code", { redirect => $request_ref->{redirect}, lc => $lc, request_code => $code }) if $log->is_info();
return 301;
$log->info("302 redirecting user because request_code does not match code", { redirect => $request_ref->{redirect}, lc => $lc, request_code => $code }) if $log->is_info();
redirect(302, $request_ref->{redirect});
}

# Check that the titleid is the right one
Expand All @@ -7373,8 +7393,8 @@ CSS
(($titleid ne '') and ((not defined $request_ref->{titleid}) or ($request_ref->{titleid} ne $titleid))) or
(($titleid eq '') and ((defined $request_ref->{titleid}) and ($request_ref->{titleid} ne ''))) )) {
$request_ref->{redirect} = $request_ref->{canon_url};
$log->info("301 redirecting user because titleid is incorrect", { redirect => $request_ref->{redirect}, lc => $lc, product_lc => $product_ref->{lc}, titleid => $titleid, request_titleid => $request_ref->{titleid} }) if $log->is_info();
return 301;
$log->info("302 redirecting user because titleid is incorrect", { redirect => $request_ref->{redirect}, lc => $lc, product_lc => $product_ref->{lc}, titleid => $titleid, request_titleid => $request_ref->{titleid} }) if $log->is_info();
redirect(302, $request_ref->{redirect});
}

# Note: the product_url function is automatically added to all templates
Expand Down