Splash page catalog search doesn't work for multi-word queries

[Jane Sandberg]

Steps to recreate:

1) On a test server running a very recent branch of Evergreen, type a multi-word query into the "Search the Catalog" splash page input. My query was: Ready Player One
2) Note that you are taken to https://[domain]/eg2/en-US/staff/catalog/search?query=ready%2520player%2520one
3) Note that the search query in the search box is ready%20player%20one

Seems like things are getting url encoded one too many times somewhere along the line.

[Bill Erickson (berick)]

I'm unable to reproduce in Chrome or FF. Interestingly, Chrome escapes the spaces (%20) but FF does not. In either case, however, the search box shows "ready player one".

I'm mainly testing with Chrome 84.0.4147.125 on Linux.

[Jane Sandberg (sandbergja)]

Oh, weird! I'm seeing it on angular-acq-test.evergreencatalog.com right now in firefox and chromium-based edge. I also was seeing it on the angular vol-copy branch. Let me spin up a VM with master on it and see if I fare better.

[Bill Erickson (berick)]

Ah, I do see it on angular-acq-test.evergreencatalog.com as well.

query=ready%2520player%2520one

[Bill Erickson (berick)]

I rebuilt node_modules and tested dev vs. prod builds of the AngJS code and still cannot reproduce locally. At a glance, I'm not seeing any differences in app.js or t_splash on angular-acq-test.evergreencatalog.com as compared to my VM.

I wonder if a node rebuild on angular-acq-test.evergreencatalog.com is in order?

[Jane Sandberg (sandbergja)]

I'm seeing the issue on my vm with a fresh install of current master. It's showing up in Firefox and Chromium for me.

[Galen Charlton (gmc)]

I'm doing a fresh npm install on angular-acq-test to see if it makes any difference.

[Galen Charlton (gmc)]

rm -Rf node_modules => npm install => ng build cleared the problem for me on angular-acq-test. That server has been in use for a while, so I wonder if there was some detritus in node_modules causing a hidden problem with the build.

I tried reproducing the problem on a master system and couldn't reproduce the issue, including with tossing an npm update into the mix.

[Galen Charlton (gmc)]

Oh, to clarify one point: the node rebuilds I did was strictly on the Angular side; I didn't touch the AngularJS side at all.

[Christopher Burton (cburton)]

I am having the same issue with a recent upgrade of Evergreen from 3.4.0 - > 3.6.1. Trying to build the node modules is also bringing up a build error for me at $EVERGREEN_ROOT/Open-ILS/src/eg2/

>> ng build --prod
Schema validation failed with the following errors:
  Data path ".builders['app-shell']" should have required property 'class'.

[Christopher Burton (cburton)]

Actually...that error must be in the environment of that server. I ran it on another, it was successful, but still no change. Harry Potter searches like Harry%20Potter from the splash page search

[Christopher Burton (cburton)]

Upon further investigation. This is when searching from the eg/staff interface which then directs you to the eg2 search.

By implementing a redirect to the splash page, I was able to keep the splash page showing up as the eg2 version in which the search works as intended.

[Jason Stephenson (jstephenson)]

In case this helps: %25 is % encoded, so it looks to me like something is "double" encoding %20 to %2520. My guess would be it happens in the redirect to the Angular/eg2 search.

[Terran McCanna (tmccanna)]

Strangely, we're seeing this on some 3.6.1 servers and not others. Worked fine on our test server before our upgrade, but it's not working in production. The EG code is installed from the same source on both servers.

[Jennifer Bruch (jbruchpails)]

PaILS/SPARK is also encountering this on our test server running 3.6.

[Terran McCanna (tmccanna)]

FYI, we implemented Chris Burton's workaround for this, which is to add redirects to the Apache config file to force use of the eg2 splash page:

RewriteRule ^/eg/staff$ https://%{HTTP_HOST}/eg/staff/ [R=301,L]
RewriteRule ^/eg/staff/$ https://%{HTTP_HOST}/eg2/en-US/staff/ [R=301,L]
RewriteRule ^/eg/staff/splash$ https://%{HTTP_HOST}/eg2/staff/splash [R=301,L]

[Millissa (millissam)]

We are seeing this with 3.6.3.

[Lindsay Stratton (lstratton)]

After upgrade to 3.6.3 I am seeing this with multi-word searches -

1) In the Home tab search box entered - importance being ernest
2) Staff Catalog opened to Keyword contains importance%20%being%ernest and no matching items found

Searching a single term returned results as expected.

[Dan Briem (dbriem)]

I couldn't replicate this on a test server until I ran an older version of Apache, Ubuntu 16.04.7, Apache 2.4.18, Evergreen 3.6.3. The Angular locale path redirect request is the original escaped query, but the resulting response location header is double escaped.

It looks like mod_rewrite used to escape special characters in query strings by default, but newer versions don't: https://github.com/apache/httpd/commit/c1922e4d0c6472e3debe38d321697788c15b04d6. I don't know if this accounts for everyone's experience with this issue, but I couldn't replicate it on recent versions of Apache.

A potential solution is to add the NE (noescape) flag (https://httpd.apache.org/docs/current/rewrite/flags.html#flag_ne) to the Angular locale path redirect in eg_vhost.conf.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dbriem/lp1892435_splash_page_search_double_escaped

[Terran McCanna (tmccanna)]

Since it works on the eg2 splash page, is it really worth fixing the eg splash page? Wouldn't it be better to completely replace the eg splash page with the eg2 one? That's essentially what we're doing by applying the apache redirect from comment #15 above as a workaround, and we haven't had any issues. Is there any reason why the old splash page needs to stay?

[Dan Briem (dbriem)]

I was a little hasty, let me take of the pullrequest. The Angular splash page avoids a redirect, so that's another way to handle it. My thought was, since this isn't the only place we redirect from AngularJS/.tt2 to Angular, to just standardize the escaping behavior.

[Bob Wicksall (bwicksall)]

The following is not a great reason to keep 'eg' but is is our reason.

We customize our eg splash page with links to a custom reporter and the PDF output of our Overdue Notices. This is easy with 'eg' because I just modify one template file. I already heavily modify our OPAC so this one file just goes along for the ride.

'eg2' is a different story as it doesn't lend itself to customization unless you are self hosting. The move to 'eg2' is already a problem for us and this bug just makes things worse. Long term the solution lies elsewhere but for us having just upgraded to 3.6.3 this is a problem.

[Terran McCanna (tmccanna)]

We customized our eg2 splash page to be the same as our eg splash page with no problems, but we don't make changes to it very often - usually just at upgrade time.

But if you do need to make frequent changes, I can't think of any reason why you wouldn't be able to do that. The code for that page (\src\eg2\src\app\staff\splash.component.html) is mostly HTML, so it's just as easy to update . The only difference is that your vendor would need to run ng build to get your changes to appear. I expect they could even set up a script to run ng build nightly, or on the same night every week if you do weekly changes.

[Dan Briem (dbriem)]

This double redirect issue also seems to be the cause of bug 1926797. It's another situation where we use encodeURIComponent to escape a request that needs to be redirected to Angular, which is escaped for a second time on certain servers by the 307 Redirect to the Angular locale path, and the decoded result ends up still being escaped.

Even if it's decided to use the new splash page, I think we might have to address the mod_rewrite inconsistency.

[Jason Boyer (jboyer)]

Because this is a simple fix that solves the problem at hand and is harmless in that it matches the way newer versions of Apache already operate I've gone ahead and pushed it to master, rel_3_6, and rel_3_7. Thanks Jane and Dan!