fix: revert POST for document search requests

ietf/api/serializer.py

@@ -146,7 +146,6 @@ def end_object(self, obj):
                                 field_value = None
                         else:
                             field_value = field
-                        # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1
                         if isinstance(field_value, QuerySetAny) or isinstance(field_value, list):
                             self._current[name] = dict([ (rel.pk, self.expand_related(rel, name)) for rel in field_value ])
                         else:

ietf/doc/tests.py

@@ -73,163 +73,96 @@
 
 
 class SearchTests(TestCase):
-    def test_search_handles_querystring_parameters(self):
-        """Search parameters via querystring should not actually search"""
-        url = urlreverse("ietf.doc.views_search.search")
-        r = self.client.get(url + "?name=some-document-name&oldDrafts=on")
-        # Check that we got a valid response and that the warning about query string parameters is shown.
-        self.assertContains(
-            r,
-            "Searching via the URL query string is no longer supported.",
-            status_code=200,
-        )
-        # Check that the form was filled in correctly (not an exhaustive check, but different from the
-        # form defaults)
-        pq = PyQuery(r.content)
-        self.assertEqual(
-            pq("form#search_form input#id_name").attr("value"),
-            "some-document-name",
-            "The name field should be set in the SearchForm",
-        )
-        self.assertEqual(
-            pq("form#search_form input#id_olddrafts").attr("checked"),
-            "checked",
-            "The old drafts checkbox should be selected in the SearchForm",
-        )
-        self.assertIsNone(
-            pq("form#search_form input#id_rfcs").attr("checked"),
-            "The RFCs checkbox should not be selected in the SearchForm",
-        )
-        self.assertIsNone(
-            pq("form#search_form input#id_activedrafts").attr("checked"),
-            "The active drafts checkbox should not be selected in the SearchForm",
-        )
-
     def test_search(self):
-        draft = WgDraftFactory(
-            name="draft-ietf-mars-test",
-            group=GroupFactory(acronym="mars", parent=Group.objects.get(acronym="farfut")),
-            authors=[PersonFactory()],
-            ad=PersonFactory(),
-        )
+
+        draft = WgDraftFactory(name='draft-ietf-mars-test',group=GroupFactory(acronym='mars',parent=Group.objects.get(acronym='farfut')),authors=[PersonFactory()],ad=PersonFactory())
         rfc = WgRfcFactory()
         draft.set_state(State.objects.get(used=True, type="draft-iesg", slug="pub-req"))
-        old_draft = IndividualDraftFactory(
-            name="draft-foo-mars-test",
-            authors=[PersonFactory()],
-            title="Optimizing Martian Network Topologies",
-        )
+        old_draft = IndividualDraftFactory(name='draft-foo-mars-test',authors=[PersonFactory()],title="Optimizing Martian Network Topologies")
         old_draft.set_state(State.objects.get(used=True, type="draft", slug="expired"))
-    
-        url = urlreverse("ietf.doc.views_search.search")
-    
+
+        base_url = urlreverse('ietf.doc.views_search.search')
+
         # only show form, no search yet
-        r = self.client.get(url)
+        r = self.client.get(base_url)
         self.assertEqual(r.status_code, 200)
-    
+
         # no match
-        r = self.client.post(url, {"activedrafts": "on", "name": "thisisnotadocumentname"})
+        r = self.client.get(base_url + "?activedrafts=on&name=thisisnotadocumentname")
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, "No documents match")
-    
-        r = self.client.post(url, {"rfcs": "on", "name": "xyzzy"})
+
+        r = self.client.get(base_url + "?rfcs=on&name=xyzzy")
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, "No documents match")
-    
-        r = self.client.post(url, {"olddrafts": "on", "name": "bar"})
+
+        r = self.client.get(base_url + "?olddrafts=on&name=bar")
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, "No documents match")
-    
-        r = self.client.post(url, {"olddrafts": "on", "name": "foo"})
+
+        r = self.client.get(base_url + "?olddrafts=on&name=foo")
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, "draft-foo-mars-test")
-    
-        r = self.client.post(url, {"olddrafts": "on", "name": "FoO"})  # mixed case
+
+        r = self.client.get(base_url + "?olddrafts=on&name=FoO")  # mixed case
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, "draft-foo-mars-test")
-    
+
         # find by RFC
-        r = self.client.post(url, {"rfcs": "on", "name": rfc.name})
+        r = self.client.get(base_url + "?rfcs=on&name=%s" % rfc.name)
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, rfc.title)
-    
+
         # find by active/inactive
-    
+
         draft.set_state(State.objects.get(type="draft", slug="active"))
-        r = self.client.post(url, {"activedrafts": "on", "name": draft.name})
+        r = self.client.get(base_url + "?activedrafts=on&name=%s" % draft.name)
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, draft.title)
-    
+
         draft.set_state(State.objects.get(type="draft", slug="expired"))
-        r = self.client.post(url, {"olddrafts": "on", "name": draft.name})
+        r = self.client.get(base_url + "?olddrafts=on&name=%s" % draft.name)
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, draft.title)
-
+        
         draft.set_state(State.objects.get(type="draft", slug="active"))
-    
+
         # find by title
-        r = self.client.post(url, {"activedrafts": "on", "name": draft.title.split()[0]})
+        r = self.client.get(base_url + "?activedrafts=on&name=%s" % draft.title.split()[0])
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, draft.title)
-    
+
         # find by author
-        r = self.client.post(
-            url,
-            {
-                "activedrafts": "on",
-                "by": "author",
-                "author": draft.documentauthor_set.first().person.name_parts()[1],
-            },
-        )
+        r = self.client.get(base_url + "?activedrafts=on&by=author&author=%s" % draft.documentauthor_set.first().person.name_parts()[1])
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, draft.title)
-    
+
         # find by group
-        r = self.client.post(
-            url,
-            {"activedrafts": "on", "by": "group", "group": draft.group.acronym},
-        )
+        r = self.client.get(base_url + "?activedrafts=on&by=group&group=%s" % draft.group.acronym)
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, draft.title)
-
-        r = self.client.post(
-            url,
-            {"activedrafts": "on", "by": "group", "group": draft.group.acronym.swapcase()},
-        )
+
+        r = self.client.get(base_url + "?activedrafts=on&by=group&group=%s" % draft.group.acronym.swapcase())
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, draft.title)
-    
+
         # find by area
-        r = self.client.post(
-            url,
-            {"activedrafts": "on", "by": "area", "area": draft.group.parent_id},
-        )
+        r = self.client.get(base_url + "?activedrafts=on&by=area&area=%s" % draft.group.parent_id)
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, draft.title)
-    
+
         # find by area
-        r = self.client.post(
-            url,
-            {"activedrafts": "on", "by": "area", "area": draft.group.parent_id},
-        )
+        r = self.client.get(base_url + "?activedrafts=on&by=area&area=%s" % draft.group.parent_id)
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, draft.title)
-    
+
         # find by AD
-        r = self.client.post(url, {"activedrafts": "on", "by": "ad", "ad": draft.ad_id})
+        r = self.client.get(base_url + "?activedrafts=on&by=ad&ad=%s" % draft.ad_id)
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, draft.title)
-    
+
         # find by IESG state
-        r = self.client.post(
-            url,
-            {
-                "activedrafts": "on",
-                "by": "state",
-                "state": draft.get_state("draft-iesg").pk,
-                "substate": "",
-            },
-        )
+        r = self.client.get(base_url + "?activedrafts=on&by=state&state=%s&substate=" % draft.get_state("draft-iesg").pk)
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, draft.title)
 
@@ -238,15 +171,15 @@ def test_search_became_rfc(self):
         rfc = WgRfcFactory()
         draft.set_state(State.objects.get(type="draft", slug="rfc"))
         draft.relateddocument_set.create(relationship_id="became_rfc", target=rfc)
-        url = urlreverse("ietf.doc.views_search.search")
+        base_url = urlreverse('ietf.doc.views_search.search')
 
         # find by RFC
-        r = self.client.post(url, {"rfcs": "on", "name": rfc.name})
+        r = self.client.get(base_url + f"?rfcs=on&name={rfc.name}")
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, rfc.title)
 
         # find by draft
-        r = self.client.post(url, {"activedrafts": "on", "rfcs": "on", "name": draft.name})
+        r = self.client.get(base_url + f"?activedrafts=on&rfcs=on&name={draft.name}")
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, rfc.title)
 

ietf/doc/views_search.py

@@ -33,31 +33,28 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-
+import hashlib
+import json
 import re
 import datetime
 import copy
-import hashlib
-import json
 import operator
 
 from collections import defaultdict
 from functools import reduce
 
 from django import forms
 from django.conf import settings
-from django.contrib import messages
 from django.core.cache import cache, caches
 from django.urls import reverse as urlreverse
 from django.db.models import Q
-from django.http import Http404, HttpResponseBadRequest, HttpResponse, HttpResponseRedirect
+from django.http import Http404, HttpResponseBadRequest, HttpResponse, HttpResponseRedirect, QueryDict
 from django.shortcuts import render
 from django.utils import timezone
 from django.utils.html import strip_tags
 from django.utils.cache import _generate_cache_key # type: ignore
 from django.utils.text import slugify
-from django.views.decorators.csrf import ensure_csrf_cookie
-from django_stubs_ext import QuerySetAny
+
 
 import debug                            # pyflakes:ignore
 
@@ -149,29 +146,6 @@ def clean(self):
             q['irtfstate'] = None
         return q
 
-    def cache_key_fragment(self):
-        """Hash a bound form to get a value for use in a cache key
-
-        Raises a ValueError if the form is not valid.
-        """
-        def _serialize_value(val):
-            # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1
-            if isinstance(val, QuerySetAny):
-                return [item.pk for item in val]
-            else:
-                return getattr(val, "pk", val)  # use pk if present, else value
-
-        if not self.is_valid():
-            raise ValueError(f"SearchForm invalid: {self.errors}")
-        contents = {
-            field_name: _serialize_value(field_value)
-            for field_name, field_value in self.cleaned_data.items() 
-            if field_name != "sort" and field_value is not None
-        }
-        contents_json = json.dumps(contents, sort_keys=True)
-        return hashlib.sha512(contents_json.encode("utf-8")).hexdigest()
-
-
 def retrieve_search_results(form, all_types=False):
     """Takes a validated SearchForm and return the results."""
 
@@ -284,65 +258,51 @@ def retrieve_search_results(form, all_types=False):
     return docs
 
 
-@ensure_csrf_cookie
 def search(request):
-    """Search for a draft"""
-    # defaults for results / meta
-    results = []
-    meta = {"by": None, "searching": False}
-
-    if request.method == "POST":
-        form = SearchForm(data=request.POST)
-        if form.is_valid():
-            cache_key = f"doc:document:search:{form.cache_key_fragment()}"
-            cached_val = cache.get(cache_key)
-            if cached_val:
-                [results, meta] = cached_val
-            else:
-                results = retrieve_search_results(form)
-                results, meta = prepare_document_table(
-                    request, results, form.cleaned_data
-                )
-                cache.set(
-                    cache_key, [results, meta]
-                )  # for settings.CACHE_MIDDLEWARE_SECONDS
-                log(f"Search results computed for {form.cleaned_data}")
-            meta["searching"] = True
-    else:
-        if request.GET:
-            # backwards compatibility - fill in the form
-            get_params = request.GET.copy()
-            if "activeDrafts" in request.GET:
-                get_params["activedrafts"] = request.GET["activeDrafts"]
-            if "oldDrafts" in request.GET:
-                get_params["olddrafts"] = request.GET["oldDrafts"]
-            if "subState" in request.GET:
-                get_params["substate"] = request.GET["subState"]
-            form = SearchForm(data=get_params)
-            messages.error(
-                request,
-                (
-                    "Searching via the URL query string is no longer supported. "
-                    "The form below has been filled in with the parameters from your request. "
-                    'To execute your search, please click "Search".'
-                ),
-            )
+    def _get_cache_key(params):
+        fields = set(SearchForm.base_fields) - {'sort'}
+        kwargs = dict([(k, v) for (k, v) in list(params.items()) if k in fields])
+        key = "doc:document:search:" + hashlib.sha512(json.dumps(kwargs, sort_keys=True).encode('utf-8')).hexdigest()
+        return key
+
+    if request.GET:
+        # backwards compatibility
+        get_params = request.GET.copy()
+        if 'activeDrafts' in request.GET:
+            get_params['activedrafts'] = request.GET['activeDrafts']
+        if 'oldDrafts' in request.GET:
+            get_params['olddrafts'] = request.GET['oldDrafts']
+        if 'subState' in request.GET:
+            get_params['substate'] = request.GET['subState']
+
+        form = SearchForm(get_params)
+        if not form.is_valid():
+            return HttpResponseBadRequest("form not valid: %s" % form.errors)
+
+        cache_key = _get_cache_key(get_params)
+        cached_val = cache.get(cache_key)
+        if cached_val:
+            [results, meta] = cached_val
         else:
-            form = SearchForm()
+            results = retrieve_search_results(form)
+            results, meta = prepare_document_table(request, results, get_params)
+            cache.set(cache_key, [results, meta]) # for settings.CACHE_MIDDLEWARE_SECONDS
+            log(f"Search results computed for {get_params}")
+        meta['searching'] = True
+    else:
+        form = SearchForm()
+        results = []
+        meta = { 'by': None, 'searching': False }
+        get_params = QueryDict('')
 
-    return render(
-        request,
-        "doc/search/search.html",
-        context={"form": form, "docs": results, "meta": meta},
+    return render(request, 'doc/search/search.html', {
+        'form':form, 'docs':results, 'meta':meta, 'queryargs':get_params.urlencode() },
     )
 
-
-@ensure_csrf_cookie
 def frontpage(request):
     form = SearchForm()
     return render(request, 'doc/frontpage.html', {'form':form})
 
-
 def search_for_name(request, name):
     def find_unique(n):
         exact = Document.objects.filter(name__iexact=n).first()