From f9cd15c79202a50d7c0a3f9aa8de45c2c23cdb49 Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Sat, 24 Oct 2009 05:22:45 +0000 Subject: Bug 523495: Re-work attachment.cgi and the general attachment_base-checking code to prevent an infinite redirect loop when ssl_redirect is on and Bugzilla has an attachment_base set. Patch by Max Kanat-Alexander r=LpSolit, a=LpSolit --- Bugzilla.pm | 5 ++++- Bugzilla/CGI.pm | 36 +++++++++++++++++++++++++++--------- attachment.cgi | 23 ++++++++++++----------- 3 files changed, 43 insertions(+), 21 deletions(-) diff --git a/Bugzilla.pm b/Bugzilla.pm index 62b1af659..67ec611a9 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -113,7 +113,10 @@ sub init_page { }; } - do_ssl_redirect_if_required(); + # Because of attachment_base, attachment.cgi handles this itself. + if (basename($0) ne 'attachment.cgi') { + do_ssl_redirect_if_required(); + } # If Bugzilla is shut down, do not allow anything to run, just display a # message to the user about the downtime and log out. Scripts listed in diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index c30e13618..8c68f996c 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -28,6 +28,8 @@ use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Util; +use File::Basename; + BEGIN { if (ON_WINDOWS) { # Help CGI find the correct temp directory as the default list @@ -71,15 +73,9 @@ sub new { $self->charset(Bugzilla->params->{'utf8'} ? 'UTF-8' : ''); # Redirect to urlbase/sslbase if we are not viewing an attachment. - if (use_attachbase() && i_am_cgi()) { - my $cgi_file = $self->url('-path_info' => 0, '-query' => 0, '-relative' => 1); - $cgi_file =~ s/\?$//; - my $urlbase = Bugzilla->params->{'urlbase'}; - my $sslbase = Bugzilla->params->{'sslbase'}; - my $path_regexp = $sslbase ? qr/^(\Q$urlbase\E|\Q$sslbase\E)/ : qr/^\Q$urlbase\E/; - if ($cgi_file ne 'attachment.cgi' && $self->self_url !~ /$path_regexp/) { - $self->redirect_to_urlbase; - } + my $script = basename($0); + if ($self->url_is_attachment_base and $script ne 'attachment.cgi') { + $self->redirect_to_urlbase(); } # Check for errors @@ -398,6 +394,28 @@ sub redirect_to_urlbase { exit; } +sub url_is_attachment_base { + my ($self, $id) = @_; + return 0 if !use_attachbase() or !i_am_cgi(); + my $attach_base = Bugzilla->params->{'attachment_base'}; + # If we're passed an id, we only want one specific attachment base + # for a particular bug. If we're not passed an ID, we just want to + # know if our current URL matches the attachment_base *pattern*. + my $regex; + if ($id) { + $attach_base =~ s/\%bugid\%/$id/; + $regex = quotemeta($attach_base); + } + else { + # In this circumstance we run quotemeta first because we need to + # insert an active regex meta-character afterward. + $regex = quotemeta($attach_base); + $regex =~ s/\\\%bugid\\\%/\\d+/; + } + $regex = "^$regex"; + return ($self->self_url =~ $regex) ? 1 : 0; +} + 1; __END__ diff --git a/attachment.cgi b/attachment.cgi index be82294d7..a10d9f970 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -77,10 +77,8 @@ my $action = $cgi->param('action') || 'view'; # You must use the appropriate urlbase/sslbase param when doing anything # but viewing an attachment. if ($action ne 'view') { - my $urlbase = Bugzilla->params->{'urlbase'}; - my $sslbase = Bugzilla->params->{'sslbase'}; - my $path_regexp = $sslbase ? qr/^(\Q$urlbase\E|\Q$sslbase\E)/ : qr/^\Q$urlbase\E/; - if (use_attachbase() && $cgi->self_url !~ /$path_regexp/) { + do_ssl_redirect_if_required(); + if ($cgi->url_is_attachment_base) { $cgi->redirect_to_urlbase; } Bugzilla->login(); @@ -243,10 +241,6 @@ sub view { if (use_attachbase()) { $attachment = validateID(undef, 1); - # Replace %bugid% by the ID of the bug the attachment belongs to, if present. - my $attachbase = Bugzilla->params->{'attachment_base'}; - my $bug_id = $attachment->bug_id; - $attachbase =~ s/%bugid%/$bug_id/; my $path = 'attachment.cgi?id=' . $attachment->id; # The user is allowed to override the content type of the attachment. if (defined $cgi->param('content_type')) { @@ -254,10 +248,16 @@ sub view { } # Make sure the attachment is served from the correct server. - if ($cgi->self_url !~ /^\Q$attachbase\E/) { - # We couldn't call Bugzilla->login earlier as we first had to make sure - # we were not going to request credentials on the alternate host. + my $bug_id = $attachment->bug_id; + if (!$cgi->url_is_attachment_base($bug_id)) { + # We couldn't call Bugzilla->login earlier as we first had to + # make sure we were not going to request credentials on the + # alternate host. Bugzilla->login(); + my $attachbase = Bugzilla->params->{'attachment_base'}; + # Replace %bugid% by the ID of the bug the attachment + # belongs to, if present. + $attachbase =~ s/\%bugid\%/$bug_id/; if (attachmentIsPublic($attachment)) { # No need for a token; redirect to attachment base. print $cgi->redirect(-location => $attachbase . $path); @@ -291,6 +291,7 @@ sub view { } } } else { + do_ssl_redirect_if_required(); # No alternate host is used. Request credentials if required. Bugzilla->login(); $attachment = validateID(); -- cgit v1.2.3-24-g4f1b