Updated: Comment #5
We should really, really be using /search/node?keys=mytext rather than /search/node/mytext. Trying to put the keywords into the URL directly really complicates the code, doesn't really provide any benefits, and causes lots of side effects (see Related Issues section).
Move the keywords to a ?keys= URL query instead of appending them to the URL.
Write tests for the issues listed as duplicates of this one, to make sure they are actually resolved.
Write a patch and update tests as needed.
URLs for searching will change.
None.
Here is a list of bugs that have been reported that are marked as duplicates of this issue. They would be good to use as test cases:
#890058: Searching for ../../admin takes me to the admin page
#1421560: searches beginning with a . lead to apache Forbidden error
#1529778: Search with validation error (e.g. small word error) blocks new search from search block form
#1754060: Queries run twice (once on wrong thing) after submitting search block forms on main search page
#2017095: After searching for a too-short word, original search keyword is retained in subsequent searches
Note that this other issue would also resolve some of the above bugs:
#1789768: search_box_form_submit() improperly calls form_set_error()
Comments
Comment #1
jhodgdon+1.
Comment #2
jhodgdonSee also:
#753996: [META] Re-architect use of menus, tabs, and forms in Search module
Are these two issues duplicates?
Comment #3
dave reidSeems like #753996 is sort of a meta-issue, while smaller tasks like this can be done independently.
Comment #4
jhodgdonWe really need to take care of this issue. I am marking it as a major task, because it is causing several bugs and the only way to fix them is to take the keywords out of their direct place in the URL and make them be a query string. This will greatly simply all sorts of code in the search module too, I believe.
alexpott also just endorsed the idea of doing this. We need to.
I'm going to update the issue summary and include a list of issues this would solve there.
Comment #4.0
jhodgdonmake an issue summary
Comment #4.1
jhodgdonadd another related issue
Comment #4.2
jhodgdonadd a couple more related issues
Comment #5
pwolanin commentedWill try to get to this next week (Friday) when I have time to code
Comment #5.0
pwolanin commentedanother related issue
Comment #6
pwolanin commentedunassigning until I have time in case someone else wants to work on it
Comment #7
jhodgdonI would really like to elevate this issue to "critical", except I'm sure that if I do, it will be shot down by the critical issue counting staff. :)
Still... I think this is about the highest-priority issue in search.module and we really need to get it done.
pwolanin: do you think you might have time in the near future, or should we recruit someone else?
Comment #8
jhodgdonI'm working on #1366020: Overhaul SearchQuery; make search redirects use GET query params for keywords and realizing I cannot fix that issue without also resolving this one. So for the moment I am postponing this issue until that other one is done or abandoned.
Comment #9
jhodgdonThis was taken care of on #1366020: Overhaul SearchQuery; make search redirects use GET query params for keywords