From 460ae51fc0e70dd2d7fcef4bdb92308269327f1d Mon Sep 17 00:00:00 2001 From: Dave Lawrence Date: Fri, 9 Aug 2013 13:55:53 -0400 Subject: Bug 903514 - Backport upstream bug 569177 for etag support to bmo/4.2 --- Bugzilla/CGI.pm | 20 ++++++++++ Bugzilla/WebService/Bug.pm | 17 ++++++--- Bugzilla/WebService/Server.pm | 72 ++++++++++++++++++++++++++++++++++- Bugzilla/WebService/Server/JSONRPC.pm | 29 +++++++++++--- Bugzilla/WebService/Server/REST.pm | 4 ++ Bugzilla/WebService/Server/XMLRPC.pm | 27 +++++++++++-- config.cgi | 39 ++++++------------- 7 files changed, 166 insertions(+), 42 deletions(-) diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index 55b6a999c..ce7cdf429 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -235,6 +235,26 @@ sub clean_search_url { } } +sub check_etag { + my ($self, $valid_etag) = @_; + + # ETag support. + my $if_none_match = $self->http('If-None-Match'); + return if !$if_none_match; + + my @if_none = split(/[\s,]+/, $if_none_match); + foreach my $possible_etag (@if_none) { + # remove quotes from begin and end of the string + $possible_etag =~ s/^\"//g; + $possible_etag =~ s/\"$//g; + if ($possible_etag eq $valid_etag or $possible_etag eq '*') { + return 1; + } + } + + return 0; +} + # Overwrite to ensure nph doesn't get set, and unset HEADERS_ONCE sub multipart_init { my $self = shift; diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index a07196209..df9084210 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -350,6 +350,18 @@ sub get { push(@bugs, $self->_bug_to_hash($bug, $params)); } + # Set the ETag before inserting the update tokens + # since the tokens will always be unique even if + # the data has not changed. + $self->bz_etag(\@bugs); + + if (Bugzilla->user->id) { + foreach my $bug (@bugs) { + my $token = issue_hash_token([$bug->{'id'}, $bug->{'last_change_time'}]); + $bug->{'update_token'} = $self->type('string', $token); + } + } + return { bugs => \@bugs, faults => \@faults }; } @@ -1008,11 +1020,6 @@ sub _bug_to_hash { $item{'actual_time'} = $self->type('double', $bug->actual_time); } - if (Bugzilla->user->id) { - my $token = issue_hash_token([$bug->id, $bug->delta_ts]); - $item{'update_token'} = $self->type('string', $token); - } - # The "accessible" bits go here because they have long names and it # makes the code look nicer to separate them out. $item{'is_cc_accessible'} = $self->type('boolean', diff --git a/Bugzilla/WebService/Server.pm b/Bugzilla/WebService/Server.pm index 206f0c657..9727dcbcb 100644 --- a/Bugzilla/WebService/Server.pm +++ b/Bugzilla/WebService/Server.pm @@ -22,6 +22,9 @@ use Bugzilla::Error; use Bugzilla::Util qw(datetime_from); use Scalar::Util qw(blessed); +use Digest::MD5 qw(md5_base64); + +use Storable qw(freeze); sub handle_login { my ($self, $class, $method, $full_method) = @_; @@ -37,7 +40,7 @@ sub handle_login { sub datetime_format_inbound { my ($self, $time) = @_; - + my $converted = datetime_from($time, Bugzilla->local_timezone); if (!defined $converted) { ThrowUserError('illegal_date', { date => $time }); @@ -63,4 +66,71 @@ sub datetime_format_outbound { return $time->iso8601(); } +# ETag support +sub bz_etag { + my ($self, $data) = @_; + my $cache = Bugzilla->request_cache; + if (defined $data) { + # Serialize the data if passed a reference + local $Storable::canonical = 1; + $data = freeze($data) if ref $data; + + # Wide characters cause md5_base64() to die. + utf8::encode($data) if utf8::is_utf8($data); + + # Append content_type to the end of the data + # string as we want the etag to be unique to + # the content_type. We do not need this for + # XMLRPC as text/xml is always returned. + if (blessed($self) && $self->can('content_type')) { + $data .= $self->content_type if $self->content_type; + } + + $cache->{'bz_etag'} = md5_base64($data); + } + return $cache->{'bz_etag'}; +} + 1; + +=head1 NAME + +Bugzilla::WebService::Server - Base server class for the WebService API + +=head1 DESCRIPTION + +Bugzilla::WebService::Server is the base class for the individual WebService API +servers such as XMLRPC, JSONRPC, and REST. You never actually create a +Bugzilla::WebService::Server directly, you only make subclasses of it. + +=head1 FUNCTIONS + +=over + +=item C + +This function is used to store an ETag value that will be used when returning +the data by the different API server modules such as XMLRPC, or REST. The individual +webservice methods can also set the value earlier in the process if needed such as +before a unique update token is added. If a value is not set earlier, an etag will +automatically be created using the returned data except in some cases when an error +has occurred. + +=back + +=head1 SEE ALSO + +L, L, +and L. + +=head1 B + +=over + +=item handle_login + +=item datetime_format_outbound + +=item datetime_format_inbound + +=back diff --git a/Bugzilla/WebService/Server/JSONRPC.pm b/Bugzilla/WebService/Server/JSONRPC.pm index 63e9ca335..4bf3fb191 100644 --- a/Bugzilla/WebService/Server/JSONRPC.pm +++ b/Bugzilla/WebService/Server/JSONRPC.pm @@ -87,12 +87,12 @@ sub response_header { sub response { my ($self, $response) = @_; + my $cgi = $self->cgi; # Implement JSONP. if (my $callback = $self->_bz_callback) { my $content = $response->content; $response->content("$callback($content)"); - } # Use $cgi->header properly instead of just printing text directly. @@ -107,9 +107,18 @@ sub response { push(@header_args, "-$name", $value); } } - my $cgi = $self->cgi; - print $cgi->header(-status => $response->code, @header_args); - print $response->content; + + # ETag support + my $etag = $self->bz_etag; + if ($etag && $cgi->check_etag($etag)) { + push(@header_args, "-ETag", $etag); + print $cgi->header(-status => '304 Not Modified', @header_args); + } + else { + push(@header_args, "-ETag", $etag) if $etag; + print $cgi->header(-status => $response->code, @header_args); + print $response->content; + } } # The JSON-RPC 1.1 GET specification is not so great--you can't specify @@ -269,7 +278,17 @@ sub _handle { my $self = shift; my ($obj) = @_; $self->{_bz_request_id} = $obj->{id}; - return $self->SUPER::_handle(@_); + + my $result = $self->SUPER::_handle(@_); + + # Set the ETag if not already set in the webservice methods. + my $etag = $self->bz_etag; + if (!$etag && ref $result) { + my $data = $self->json->decode($result)->{'result'}; + $self->bz_etag($data); + } + + return $result; } # Make all error messages returned by JSON::RPC go into the 100000 diff --git a/Bugzilla/WebService/Server/REST.pm b/Bugzilla/WebService/Server/REST.pm index 00c71110f..8d3aa481c 100644 --- a/Bugzilla/WebService/Server/REST.pm +++ b/Bugzilla/WebService/Server/REST.pm @@ -125,6 +125,10 @@ sub response { # Access Control $response->header("Access-Control-Allow-Origin", "*"); + # ETag support + my $etag = $self->bz_etag; + $self->bz_etag($result) if !$etag; + # If accessing through web browser, then display in readable format if ($self->content_type eq 'text/html') { $result = $self->json->pretty->canonical->encode($result); diff --git a/Bugzilla/WebService/Server/XMLRPC.pm b/Bugzilla/WebService/Server/XMLRPC.pm index 1c8df1495..8d9108122 100644 --- a/Bugzilla/WebService/Server/XMLRPC.pm +++ b/Bugzilla/WebService/Server/XMLRPC.pm @@ -32,8 +32,8 @@ if ($ENV{MOD_PERL}) { use Bugzilla::WebService::Constants; use Bugzilla::Util; -# Allow WebService methods to call XMLRPC::Lite's type method directly BEGIN { + # Allow WebService methods to call XMLRPC::Lite's type method directly *Bugzilla::WebService::type = sub { my ($self, $type, $value) = @_; if ($type eq 'dateTime') { @@ -50,6 +50,11 @@ BEGIN { } return XMLRPC::Data->type($type)->value($value); }; + + # Add support for ETags into XMLRPC WebServices + *Bugzilla::WebService::bz_etag = sub { + return Bugzilla::WebService::Server->bz_etag($_[1]); + }; } sub initialize { @@ -63,22 +68,38 @@ sub initialize { sub make_response { my $self = shift; + my $cgi = Bugzilla->cgi; $self->SUPER::make_response(@_); # XMLRPC::Transport::HTTP::CGI doesn't know about Bugzilla carrying around # its cookies in Bugzilla::CGI, so we need to copy them over. - foreach my $cookie (@{Bugzilla->cgi->{'Bugzilla_cookie_list'}}) { + foreach my $cookie (@{$cgi->{'Bugzilla_cookie_list'}}) { $self->response->headers->push_header('Set-Cookie', $cookie); } # Copy across security related headers from Bugzilla::CGI - foreach my $header (split(/[\r\n]+/, Bugzilla->cgi->header)) { + foreach my $header (split(/[\r\n]+/, $cgi->header)) { my ($name, $value) = $header =~ /^([^:]+): (.*)/; if (!$self->response->headers->header($name)) { $self->response->headers->header($name => $value); } } + + # ETag support + my $etag = $self->bz_etag; + if (!$etag) { + my $data = $self->response->as_string; + $etag = $self->bz_etag($data); + } + + if ($etag && $cgi->check_etag($etag)) { + $self->response->headers->push_header('ETag', $etag); + $self->response->headers->push_header('status', '304 Not Modified'); + } + elsif ($etag) { + $self->response->headers->push_header('ETag', $etag); + } } sub handle_login { diff --git a/config.cgi b/config.cgi index 963224638..fcc5d82ed 100755 --- a/config.cgi +++ b/config.cgi @@ -44,15 +44,16 @@ use Digest::MD5 qw(md5_base64); my $user = Bugzilla->login(LOGIN_OPTIONAL); my $cgi = Bugzilla->cgi; +# Get data from the shadow DB as they don't change very often. +Bugzilla->switch_to_shadow_db; + # If the 'requirelogin' parameter is on and the user is not # authenticated, return empty fields. if (Bugzilla->params->{'requirelogin'} && !$user->id) { display_data(); + exit; } -# Get data from the shadow DB as they don't change very often. -Bugzilla->switch_to_shadow_db; - # Pass a bunch of Bugzilla configuration to the templates. my $vars = {}; $vars->{'priority'} = get_legal_field_values('priority'); @@ -136,31 +137,13 @@ sub display_data { utf8::encode($digest_data) if utf8::is_utf8($digest_data); my $digest = md5_base64($digest_data); - # ETag support. - my $if_none_match = $cgi->http('If-None-Match') || ""; - my $found304; - my @if_none = split(/[\s,]+/, $if_none_match); - foreach my $if_none (@if_none) { - # remove quotes from begin and end of the string - $if_none =~ s/^\"//g; - $if_none =~ s/\"$//g; - if ($if_none eq $digest or $if_none eq '*') { - # leave the loop after the first match - $found304 = $if_none; - last; - } - } - - if ($found304) { - print $cgi->header(-type => 'text/html', - -ETag => $found304, + if ($cgi->check_etag($digest)) { + print $cgi->header(-ETag => $digest, -status => '304 Not Modified'); + exit; } - else { - # Return HTTP headers. - print $cgi->header (-ETag => $digest, - -type => $format->{'ctype'}); - print $output; - } - exit; + + print $cgi->header (-ETag => $digest, + -type => $format->{'ctype'}); + print $output; } -- cgit v1.2.3-24-g4f1b