Incorrect typing when encoding some values as JSON

[Jeff Davis]

EG 3.7 beta
Ubuntu 20.04
Perl 5.30.0

In some cases, Perl values are encoded as JSON with the wrong datatype. This can lead to unexpected behavior.

For example, open-ils.actor.user.hold_requests.count returns a count of total holds, holds ready for pickup, and holds held behind the desk. It is presumed that these counts will be encoded as numbers in the JSON response, but the latter two values are encoded as strings when the count is zero: you get "0" instead of 0. Long story short, this causes the web client to display an alert when retrieving every patron.

That's just one example. We will likely find other instances, since the behavior appears to be due to a recent change in how Perl handles scalar(@array) in some contexts. There has been some discussion in IRC:
http://irc.evergreen-ils.org/evergreen/2021-04-07#i_479558
http://irc.evergreen-ils.org/evergreen/2021-04-08#i_479751

Perl 5.26 and earlier do not appear to be affected, so these issues should not arise on Ubuntu 18.04 and earlier.

We can address the issue in Evergreen by finding affected instances of scalar(@array) and changing them to something like int(scalar(@array)). We may also want to bring the issue to the Perl community, since it's new and unexpected behavior.

[Jeff Davis (jdavis-sitka)]

Working branch user/jeffdavis/lp1923076-numify-scalar has a fix for the hold count example, wrapping scalar() in int() as suggested by Jason Stephenson:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jeffdavis/lp1923076-numify-scalar

Test plan:

1. Install EG on Ubuntu 20.04 or some other OS using Perl 5.28 or later.
2. In the web client, retrieve a patron who has no holds ready and no other alerts. You will get a blank alert (a STOP sign with no alert message), which must be cleared before doing anything else.
3. Apply the patch and restart Evergreen.
4. Clear local storage and retrieve the patron again. This time there should be no alert.

[Jason Stephenson (jstephenson)]

I have verified this in Evergreen 3.5.3 on Debian Buster. It looks like any Evergreen version running on Debian Buster or Ubuntu 20.04, and later, are affected. The relevant change seems to have been made in Perl 5.28.0.

One way to verify this is to start up srfsh on a system running concerto and run the following commands:

login admin demo123

request open-ils.actor open-ils.actor.user.hold_requests.count "AUTHTOKEN" 1, 6

It will produce output like the following:

Received Data: {
  "total":0,
  "behind_desk":"0",
  "ready":"0"
}

Note that the zeroes after "ready" and "behind_desk" are double quoted. Also note that you need to replace AUTHTOKEN in the above with authtoken you received when logging in.

To test the patch, restart start services after installing Evergreen, and repeat the steps in srfsh. This time, the zeroes after "ready" and "behind_desk" should NOT be enclosed in double quotes.

[Mike Rylander (mrylander)]

Attached is a list of uses of scalar() that we should check, some definitely and some maybe. Since array refs don't seem to suffer this issue, I listed them but we may decide not to care.

I've excluded uses that are only used in truthiness tests (if, unless, while, ?:, etc), that are added to strings (log messages, concatenation to a string for return to the caller, etc), that are definitely not used in JSON (building HTML/XML, EDI messages, SQL, etc) or where I know for certain that the value is only used internally as a cache so we don't have to call scalar() over and over.

[Jason Stephenson (jstephenson)]

I have pushed a couple of extra commits to a collab branch: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/dyrcona/lp1923076-numify-scalar

Along the way, I have also been writing tests for these conditions, and I stumbled across more cases where numbers are coming out as strings in OpenSRF responses. I am inclined to blame JSON::XS.

Following up some stackoverflow comments and a comment in IRC from Jeff Godin in IRC, http://irc.evergreen-ils.org/evergreen/2021-04-09#i_479988, I tried Cpanel::JSON::XS instead of JSON::XS. It appears to resolve more of these issues that turn up in other cases, but still doesn't resolve all of the cases where I see numbers as strings.

One example that turns up qutit a few numbers as strings is to do the following in srfsh:

request open-ils.search open-ils.search.biblio.isbn_list [9781435712751]

If you have that book in your collection, then pick another that you don't have.

[Jason Stephenson (jstephenson)]

I have pushed another commit to the collab branch that adds some live tests to test if the issue is resolved with a few of the affected backend calls. Not all of the affected code can be easily tested.

More tests are welcome.

I'm not entirely sure how serious the issues revealed in my previous testing are.

[Jason Stephenson (jstephenson)]

I have added the pullrequest tag for the collab branch that includes my signoff on Jeff Davis's original commit.

We still have options beyond just applying this branch:

1. We could switch from JSON::XS to Cpanel::JSON::XS in OpenSRF. That would actually alleviate the need for this branch. In my testing, Cpanel::JSON::XS does a better job of identifying numbers.

2. We could also go through the code to identify more places where strings need to be cast to numbers in Perl. It looks like arguments are typically passed to OpenSRF as strings in JSON, so these numbers often come out as strings when passed through without change.

3. The client code, particularly in TypeScript/Angular, could be audited for places where strings need to be converted to numbers.

Finally, I don't think this fix is complete, and I suspect that we will find more things like this that need to be fixed as we go on.

[Michele Morgan (mmorgan)]

I'm adding my signoff to this. Resolving the alert that pops up for every patron is important. I've applied the patch to a test system and do not see the erroneous patron alerts. Also ran through basic functions in the staff client and opac and saw no issues.

My signoff branch is here:

user/mmorgan/lp1923076_signoff

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

[Mike Rylander (mrylander)]

Pushed to rel_3_5 through master. Thanks, Jeff, Jason, and Michele!