aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/application_controller.rb10
-rw-r--r--app/controllers/request_controller.rb24
-rw-r--r--app/models/foi_attachment.rb1
-rw-r--r--app/models/incoming_message.rb30
-rw-r--r--app/models/request_mailer.rb13
-rw-r--r--app/views/admin_general/debug.rhtml2
-rw-r--r--app/views/user/sign.rhtml13
-rw-r--r--doc/CHANGES.md1
-rw-r--r--spec/controllers/request_controller_spec.rb17
-rw-r--r--spec/controllers/user_controller_spec.rb5
-rw-r--r--spec/integration/errors_spec.rb8
-rw-r--r--vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb25
12 files changed, 104 insertions, 45 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 8fd2da54a..7aa522389 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -11,6 +11,8 @@
require 'open-uri'
class ApplicationController < ActionController::Base
+ class PermissionDenied < StandardError
+ end
# Standard headers, footers and navigation for whole site
layout "default"
include FastGettext::Translation # make functions like _, n_, N_ etc available)
@@ -120,6 +122,8 @@ class ApplicationController < ActionController::Base
case exception
when ActiveRecord::RecordNotFound, ActionController::UnknownAction, ActionController::RoutingError
@status = 404
+ when PermissionDenied
+ @status = 403
else
@status = 500
notify_about_exception exception
@@ -189,7 +193,7 @@ class ApplicationController < ActionController::Base
return File.exists?(key_path)
end
def foi_fragment_cache_read(key_path)
- cached = File.read(key_path)
+ return File.read(key_path)
end
def foi_fragment_cache_write(key_path, content)
FileUtils.mkdir_p(File.dirname(key_path))
@@ -367,8 +371,8 @@ class ApplicationController < ActionController::Base
# XXX this is a result of the OR hack below -- should fix by
# allowing a parameter to perform_search to control the
# default operator!
- query = query.gsub(/(\s-\s|&)/, "")
- query = query.split(/ +(?!-)/)
+ query = query.strip.gsub(/(\s-\s|&)/, "")
+ query = query.split(/ +(?![-+]+)/)
if query.last.nil? || query.last.strip.length < 3
xapian_requests = nil
else
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 6e33fe043..99aa3c7ea 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -37,8 +37,7 @@ class RequestController < ApplicationController
end
if !params[:query].nil?
query = params[:query]
- query = query.split(' ').join(' OR ') # XXX: HACK for OR instead of default AND!
- @xapian_requests = perform_search([PublicBody], query, 'relevant', nil, 5)
+ @xapian_requests = perform_search_typeahead(query, PublicBody)
end
medium_cache
end
@@ -118,11 +117,14 @@ class RequestController < ApplicationController
def details
long_cache
@info_request = InfoRequest.find_by_url_title(params[:url_title])
- if !@info_request.user_can_view?(authenticated_user)
- render :template => 'request/hidden', :status => 410 # gone
- return
+ if @info_request.nil?
+ raise ActiveRecord::RecordNotFound.new("Request not found")
+ else
+ if !@info_request.user_can_view?(authenticated_user)
+ render :template => 'request/hidden', :status => 410 # gone
+ return
+ end
end
-
@columns = ['id', 'event_type', 'created_at', 'described_state', 'last_described_at', 'calculated_state' ]
end
@@ -600,9 +602,13 @@ class RequestController < ApplicationController
before_filter :authenticate_attachment, :only => [ :get_attachment, :get_attachment_as_html ]
def authenticate_attachment
# Test for hidden
- incoming_message = IncomingMessage.find(params[:incoming_message_id])
- if !incoming_message.info_request.user_can_view?(authenticated_user)
- render :template => 'request/hidden', :status => 410 # gone
+ if request.path =~ /\/$/
+ raise PermissionDenied.new("Directory listing not allowed")
+ else
+ incoming_message = IncomingMessage.find(params[:incoming_message_id])
+ if !incoming_message.info_request.user_can_view?(authenticated_user)
+ render :template => 'request/hidden', :status => 410 # gone
+ end
end
end
diff --git a/app/models/foi_attachment.rb b/app/models/foi_attachment.rb
index dc55ff5f6..d12df688a 100644
--- a/app/models/foi_attachment.rb
+++ b/app/models/foi_attachment.rb
@@ -57,6 +57,7 @@ class FoiAttachment < ActiveRecord::Base
end
File.open(self.filepath, "wb") { |file|
file.write d
+ file.fsync
}
update_display_size!
@cached_body = d
diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb
index f0f1680eb..2186d50dc 100644
--- a/app/models/incoming_message.rb
+++ b/app/models/incoming_message.rb
@@ -128,21 +128,23 @@ class IncomingMessage < ActiveRecord::Base
# values in case we want to regenerate them (due to mail
# parsing bugs, etc).
if (!force.nil? || self.last_parsed.nil?)
- self.extract_attachments!
- self.sent_at = self.mail.date || self.created_at
- self.subject = self.mail.subject
- # XXX can probably remove from_name_if_present (which is a
- # monkey patch) by just calling .from_addrs[0].name here
- # instead?
- self.mail_from = self.mail.from_name_if_present
- begin
- self.mail_from_domain = PublicBody.extract_domain_from_email(self.mail.from_addrs[0].spec)
- rescue NoMethodError
- self.mail_from_domain = ""
+ ActiveRecord::Base.transaction do
+ self.extract_attachments!
+ self.sent_at = self.mail.date || self.created_at
+ self.subject = self.mail.subject
+ # XXX can probably remove from_name_if_present (which is a
+ # monkey patch) by just calling .from_addrs[0].name here
+ # instead?
+ self.mail_from = self.mail.from_name_if_present
+ begin
+ self.mail_from_domain = PublicBody.extract_domain_from_email(self.mail.from_addrs[0].spec)
+ rescue NoMethodError
+ self.mail_from_domain = ""
+ end
+ self.valid_to_reply_to = self._calculate_valid_to_reply_to
+ self.last_parsed = Time.now
+ self.save!
end
- self.valid_to_reply_to = self._calculate_valid_to_reply_to
- self.last_parsed = Time.now
- self.save!
end
end
diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb
index 272f2ea83..83cce9045 100644
--- a/app/models/request_mailer.rb
+++ b/app/models/request_mailer.rb
@@ -353,7 +353,18 @@ class RequestMailer < ApplicationMailer
# That that patch has not been applied, despite bribes of beer, is
# typical of the lack of quality of Rails.
- info_requests = InfoRequest.find(:all, :conditions => [ "(select id from info_request_events where event_type = 'comment' and info_request_events.info_request_id = info_requests.id and created_at > ? limit 1) is not null", Time.now() - 1.month ], :include => [ { :info_request_events => :user_info_request_sent_alerts } ], :order => "info_requests.id, info_request_events.created_at" )
+ info_requests = InfoRequest.find(:all,
+ :conditions => [
+ "info_requests.id in (
+ select info_request_id
+ from info_request_events
+ where event_type = 'comment'
+ and created_at > (now() - '1 month'::interval)
+ )"
+ ],
+ :include => [ { :info_request_events => :user_info_request_sent_alerts } ],
+ :order => "info_requests.id, info_request_events.created_at"
+ )
for info_request in info_requests
# Count number of new comments to alert on
diff --git a/app/views/admin_general/debug.rhtml b/app/views/admin_general/debug.rhtml
index 422edea03..b3b06085f 100644
--- a/app/views/admin_general/debug.rhtml
+++ b/app/views/admin_general/debug.rhtml
@@ -16,8 +16,6 @@ TMail::VERSION::STRING <%=h TMail::VERSION::STRING%>
Xapian::version_string <%=h Xapian::version_string%>
<br>
Spec::VERSION::STRING <%=h Spec::VERSION::STRING%>
-<br>
-Spec::Rails::VERSION::STRING <%=h Spec::Rails::VERSION::STRING%>
</p>
<h2>Configuration</h2>
diff --git a/app/views/user/sign.rhtml b/app/views/user/sign.rhtml
index afdb90162..bfd0fa63e 100644
--- a/app/views/user/sign.rhtml
+++ b/app/views/user/sign.rhtml
@@ -1,4 +1,4 @@
-<% if @post_redirect.reason_params[:user_name] %>
+<% if !@post_redirect.nil? && @post_redirect.reason_params[:user_name] %>
<% @title = _("Sign in") %>
<div id="sign_alone">
@@ -19,16 +19,7 @@
<% else %>
<% @title = _('Sign in or make a new account') %>
- <div id="sign_together">
-
- <!--<p id="sign_in_reason">
- <% if @post_redirect.reason_params[:web].empty? %>
- <%= _(' Please sign in or make a new account.') %>
- <% else %>
- <%= @post_redirect.reason_params[:web] %>, <%= _('please sign in or make a new account.') %>
- <% end %>
- </p>-->
-
+ <div id="sign_together">
<div id="left_half">
<h1><%= _('Sign in') %></h1>
<%= render :partial => 'signin', :locals => { :sign_in_as_existing_user => false } %>
diff --git a/doc/CHANGES.md b/doc/CHANGES.md
index 8778aaac2..99aaf7c98 100644
--- a/doc/CHANGES.md
+++ b/doc/CHANGES.md
@@ -20,6 +20,7 @@
* EXCEPTION_NOTIFICATIONS_FROM
* EXCEPTION_NOTIFICATIONS_TO
* The recommended Varnish config has changed, so that we ignore more cookies. You should review your Varnish config with respect to the example at `config/varnish-alaveteli.vcl`.
+* Consider setting elinks global config as described in the "Troubleshooting" section of INSTALL.md
# Version 0.4
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index fc62edc14..96786a0a3 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -420,7 +420,7 @@ describe RequestController, "when searching for an authority" do
get :select_authority, :query => ""
response.should render_template('select_authority')
- assigns[:xapian_requests].results.size == 0
+ assigns[:xapian_requests].should == nil
end
it "should return matching bodies" do
@@ -431,6 +431,18 @@ describe RequestController, "when searching for an authority" do
assigns[:xapian_requests].results.size == 1
assigns[:xapian_requests].results[0][:model].name.should == public_bodies(:geraldine_public_body).name
end
+
+ it "should not give an error when user users unintended search operators" do
+ for phrase in ["Marketing/PR activities - Aldborough E-Act Free Schoo",
+ "Request for communications between DCMS/Ed Vaizey and ICO from Jan 1st 2011 - May ",
+ "Bellevue Road Ryde Isle of Wight PO33 2AR - what is the",
+ "NHS Ayrshire & Arran",
+ " cardiff"]
+ lambda {
+ get :select_authority, :query => phrase
+ }.should_not raise_error(StandardError)
+ end
+ end
end
describe RequestController, "when creating a new request" do
@@ -1496,7 +1508,8 @@ describe RequestController, "when doing type ahead searches" do
it "should not give an error when user users unintended search operators" do
for phrase in ["Marketing/PR activities - Aldborough E-Act Free Schoo",
"Request for communications between DCMS/Ed Vaizey and ICO from Jan 1st 2011 - May ",
- "Bellevue Road Ryde Isle of Wight PO33 2AR - what is the"]
+ "Bellevue Road Ryde Isle of Wight PO33 2AR - what is the",
+ "NHS Ayrshire & Arran"]
lambda {
get :search_typeahead, :q => phrase
}.should_not raise_error(StandardError)
diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb
index 2560b48c7..30ad61706 100644
--- a/spec/controllers/user_controller_spec.rb
+++ b/spec/controllers/user_controller_spec.rb
@@ -111,15 +111,16 @@ describe UserController, "when signing in" do
it "should not log you in if you use an invalid PostRedirect token, and shouldn't give 500 error either" do
ActionController::Routing::Routes.filters.clear
- get :signin, :r => "/list"
- response.should render_template('sign')
post_redirect = "something invalid"
lambda {
post :signin, { :user_signin => { :email => 'bob@localhost', :password => 'jonespassword' },
:token => post_redirect
}
}.should_not raise_error(NoMethodError)
+ post :signin, { :user_signin => { :email => 'bob@localhost', :password => 'jonespassword' },
+ :token => post_redirect }
response.should render_template('sign')
+ assigns[:post_redirect].should == nil
end
# No idea how to test this in the test framework :(
diff --git a/spec/integration/errors_spec.rb b/spec/integration/errors_spec.rb
index bfb7e5fb5..8084bb35a 100644
--- a/spec/integration/errors_spec.rb
+++ b/spec/integration/errors_spec.rb
@@ -45,5 +45,13 @@ describe "When rendering errors" do
get("/request/#{ir.url_title}")
response.code.should == "500"
end
+ it "should render a 403 for attempts at directory listing for attachments" do
+ get("/request/5/response/4/attach/html/3/" )
+ response.code.should == "403"
+ end
+ it "should render a 404 for non-existent 'details' pages for requests" do
+ get("/details/request/wobble" )
+ response.code.should == "404"
+ end
end
diff --git a/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb b/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
index 70605ad04..c086c8125 100644
--- a/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
+++ b/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
@@ -248,6 +248,8 @@ module ActsAsXapian
end
end
+ MSET_MAX_TRIES = 5
+ MSET_MAX_DELAY = 5
# Set self.query before calling this
def initialize_query(options)
#raise options.to_yaml
@@ -278,7 +280,28 @@ module ActsAsXapian
ActsAsXapian.enquire.collapse_key = value
end
- self.matches = ActsAsXapian.enquire.mset(offset, limit, 100)
+ tries = 0
+ delay = 1
+ begin
+ self.matches = ActsAsXapian.enquire.mset(offset, limit, 100)
+ rescue IOError => e
+ if e.message =~ /DatabaseModifiedError: /
+ # This should be a transient error, so back off and try again, up to a point
+ if tries > MAX_TRIES
+ raise "Received DatabaseModifiedError from Xapian even after retrying #{MAX_TRIES} times"
+ else
+ sleep delay
+ end
+ tries += 1
+ delay *= 2
+ delay = MAX_DELAY if delay > MAX_DELAY
+
+ @@db.reopen()
+ retry
+ else
+ raise
+ end
+ end
self.cached_results = nil
}
end
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-27 02:38:51 +0000