Per http://postgresql.org/docs/9.1/static/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS,
the maximum identifier length works out to being 63 bytes (+1 for the
null terminator), so to avoid potential memory pressure by a 10GB string
somehow being passed in as the savepoint name, malloc no more than 64
bytes and copy no more than 63 bytes from the incoming name to the
escaped name.
Signed-off-by: Dan Scott <email address hidden>
Signed-off-by: Galen Charlton <email address hidden>
When invoking open-ils.{cstore,pcrud,rstore}.savepoint.*, the
caller supplies a name for the savepoint. However, the savepoint
names could be constructed so that the caller could execute
arbitrary SQL. This patch sanitizes the name so that it contains
only alphanumeric and underscore characters.
Signed-off-by: Galen Charlton <email address hidden>
Signed-off-by: Dan Scott <email address hidden>
Edit the verify_copy_for_hold helper function to actually work. There
were cases where it would permit a copy when it shouldn't and also
disallow a copy when it should have allowed it. It now makes better
use of the oargs override argument.
If oargs has an events member, the method now removes the matching from
the list of failure events returned by the permit_copy_hold check. It
was previously bugged, stopping on the first matching event, clearing
the list of failed and events, and returning a value to indicate that
the copy is permitted for the hold, even when it may not be.
If oargs has the "all" member and there are failure events remaining,
the function loops through those events checking if the requestor has
the override permission for the event. If the requestor does have the
override permission, then the event is added to oargs->{events} to be
saved for future checks on future copies. It the requestor does not
have the override permission, then the event is pushed onto a new array
of failed events and also onto oargs->{failed}. The latter is kept
to avoid looking up events repeatedly on future copy checks.
The oargs->{failed} member is added as a shortcut to avoid repeatedly
looking up override permissions when the requestor does not have them.
It is actually checked, when present, before the user permission is
checked with a costly database lookup.
Since verify_copy_for_hold has no other way to determine that an override
was requested, it checks for the events and/or all members of oargs being
set and having a value that would evaluate to true. If oargs is undefined,
a hashref with no members, or lacks a "true" events or all member, then
verify_copy_for_hold functions as though no overrides are requested.
Additionally, all functions that call verify_copy_for_hold, either directly
or indirectly, have had their intro logic modified to only set oargs when
an override is requested and oargs is not already set. We make the assump-
tion that if oargs is set, it contains the events member. (Perhaps that is
a poor assumption, but all of the code so far looks like it will work.)
Fix test_and_create_hold_batch so that it passes the oargs parameter into
open-ils.circ.title_hold.is_possible in the way that check_title_hold (the
implementation function) actually expects it to be passed. This also means
that we need to delete oargs from the params hashref before creating the
hold objects, as that would blow up if it were present.
Signed-off-by: Jason Stephenson <email address hidden>
Signed-off-by: Bill Erickson <email address hidden>
