Server: Nicer error reporting when an invalid search expression is used as a restriction

src/calibre/srv/ajax.py

@@ -536,11 +536,11 @@ def search_result(ctx, rd, db, query, num, offset, sort, sort_order, vl=''):
         if sfield not in skeys:
             raise HTTPNotFound('%s is not a valid sort field'%sort)
 
-    ids = ctx.search(rd, db, query, vl=vl)
+    ids, parse_error = ctx.search(rd, db, query, vl=vl, report_restriction_errors=True)
     ids = db.multisort(fields=multisort, ids_to_sort=ids)
     total_num = len(ids)
     ids = ids[offset:offset+num]
-    return {
+    ans = {
         'total_num': total_num, 'sort_order':sort_order,
         'offset':offset, 'num':len(ids), 'sort':sort,
         'base_url':ctx.url_for(search, library_id=db.server_library_id),
@@ -549,6 +549,9 @@ def search_result(ctx, rd, db, query, num, offset, sort, sort_order, vl=''):
         'book_ids':ids,
         'vl': vl,
     }
+    if parse_error is not None:
+        ans['bad_restriction'] = unicode(parse_error)
+    return ans
 
 
 @endpoint('/ajax/search/{library_id=None}', postprocess=json)

src/calibre/srv/handler.py

@@ -14,6 +14,7 @@ from calibre.srv.library_broker import LibraryBroker, path_for_db
 from calibre.srv.routes import Router
 from calibre.srv.users import UserManager
 from calibre.utils.date import utcnow
+from calibre.utils.search_query_parser import ParseException
 
 
 class Context(object):
@@ -82,14 +83,24 @@ class Context(object):
     def has_id(self, request_data, db, book_id):
         restriction = self.restriction_for(request_data, db)
         if restriction:
-            return book_id in db.search('', restriction=restriction)
+            try:
+                return book_id in db.search('', restriction=restriction)
+            except ParseException:
+                return False
         return db.has_id(book_id)
 
-    def allowed_book_ids(self, request_data, db):
+    def get_allowed_book_ids_from_restriction(self, request_data, db):
         restriction = self.restriction_for(request_data, db)
-        if restriction:
-            return frozenset(db.search('', restriction=restriction))
-        return db.all_book_ids()
+        return frozenset(db.search('', restriction=restriction)) if restriction else None
+
+    def allowed_book_ids(self, request_data, db):
+        try:
+            ans = self.get_allowed_book_ids_from_restriction(request_data, db)
+        except ParseException:
+            return frozenset()
+        if ans is None:
+            ans = db.all_book_ids()
+        return ans
 
     def check_for_write_access(self, request_data):
         if not request_data.username:
@@ -99,8 +110,13 @@ class Context(object):
         if self.user_manager.is_readonly(request_data.username):
             raise HTTPForbidden('The user {} does not have permission to make changes'.format(request_data.username))
 
-    def get_effective_book_ids(self, db, request_data, vl):
-        return db.books_in_virtual_library(vl, self.restriction_for(request_data, db))
+    def get_effective_book_ids(self, db, request_data, vl, report_parse_errors=False):
+        try:
+            return db.books_in_virtual_library(vl, self.restriction_for(request_data, db))
+        except ParseException:
+            if report_parse_errors:
+                raise
+            return frozenset()
 
     def get_categories(self, request_data, db, sort='name', first_letter_sort=True, vl=''):
         restrict_to_ids = self.get_effective_book_ids(db, request_data, vl)
@@ -135,8 +151,15 @@ class Context(object):
                 cache[key] = old
             return old[1]
 
-    def search(self, request_data, db, query, vl=''):
-        restrict_to_ids = self.get_effective_book_ids(db, request_data, vl)
+    def search(self, request_data, db, query, vl='', report_restriction_errors=False):
+        try:
+            restrict_to_ids = self.get_effective_book_ids(db, request_data, vl, report_parse_errors=report_restriction_errors)
+        except ParseException:
+            try:
+                self.get_allowed_book_ids_from_restriction(request_data, db)
+            except ParseException as e:
+                return frozenset(), e
+            return frozenset(), None
         query = query or ''
         key = query, restrict_to_ids
         with self.lock:
@@ -149,6 +172,8 @@ class Context(object):
                     cache.popitem(last=False)
             else:
                 cache[key] = old
+            if report_restriction_errors:
+                return old[1], None
             return old[1]
 
 

src/calibre/srv/tests/ajax.py

@@ -91,6 +91,8 @@ class ContentTest(LibraryBaseTest):
             self.assertTrue(db.has_id(3))
             server.handler.ctx.user_manager.add_user('12', 'test', restriction={
                 'library_restrictions':{os.path.basename(db.backend.library_path): 'id:1 or id:2'}})
+            server.handler.ctx.user_manager.add_user('inv', 'test', restriction={
+                'library_restrictions':{os.path.basename(db.backend.library_path): '"1'}})
             conn = server.connect()
 
             def url_for(path, **kw):
@@ -122,6 +124,8 @@ class ContentTest(LibraryBaseTest):
             ae(set(r(url_for('/ajax/search'))['book_ids']), {1,2})
             ae(set(r(url_for('/ajax/search?query=id:2'))['book_ids']), {2})
             ae(set(r(url_for('/ajax/search?vl=1'))['book_ids']), {1})
+            data = make_request(conn, '/ajax/search', username='inv', password='test', prefix='', method='GET')[1]
+            ae(data['bad_restriction'], 'Invalid syntax. Expected a lookup name or a word')
 
             # books.py
             nf(url_for('/book-manifest', book_id=3, fmt='TXT'))

src/pyj/book_list/library_data.pyj

@@ -1,14 +1,15 @@
 # vim:fileencoding=utf-8
 # License: GPL v3 Copyright: 2017, Kovid Goyal <kovid at kovidgoyal.net>
-from __python__ import hash_literals, bound_methods
+from __python__ import bound_methods, hash_literals
 
-from ajax import ajax, absolute_path
+from gettext import gettext as _
+
+from ajax import absolute_path, ajax
+from book_list.globals import get_session_data
 from lru_cache import LRUCache
 from session import get_interface_data
 from utils import parse_url_params
 
-from book_list.globals import get_session_data
-
 load_status = {'loading':True, 'ok':False, 'error_html':None, 'current_fetch': None}
 library_data = {'metadata':{}, 'previous_book_ids': v'[]'}
 
@@ -98,18 +99,24 @@ def current_book_ids():
 
 def on_data_loaded(end_type, xhr, ev):
     load_status.current_fetch = None
+    def bad_load(msg):
+        load_status.ok = False
+        load_status.loading = False
+        load_status.error_html = msg or xhr.error_html
+
     if end_type is 'load':
         data = JSON.parse(xhr.responseText)
-        update_library_data(data)
-        sd = get_session_data()
-        q = loaded_books_query()
-        sd.set_library_option(q.library_id, 'sort', q.sort)
+        if data.bad_restriction:
+            bad_load(_('The library restriction for the current user is invalid: {}').format(data.bad_restriction))
+        else:
+            update_library_data(data)
+            sd = get_session_data()
+            q = loaded_books_query()
+            sd.set_library_option(q.library_id, 'sort', q.sort)
     elif end_type is 'abort':
         pass
     else:
-        load_status.ok = False
-        load_status.loading = False
-        load_status.error_html = xhr.error_html
+        bad_load()
 
 
 def fetch_init_data():