dbs [Thu, 18 Oct 2007 01:11:10 +0000 (01:11 +0000)]
Install the JavaScript libraries into an OpenSRF library,
in prepration for OpenSRF applications being able to use
one known good JavaScript library for OpenSRF communication.
(I'm looking at you, EG, and your two copies of JSON.js)
miker [Mon, 1 Oct 2007 02:47:44 +0000 (02:47 +0000)]
Patch that:
1) Creates safe_calloc, suggested by Scott McKellar which includes
memset, as calloc does. safe_malloc will eventually lose its call
to memset.
2) Creates a macro in utils.h called osrf_clearbuf which
* under CLFAGS=-DNDEBUG fills the buffer with !s and a trailing nul
* otherwise (currently) uses memset to fill with nuls
The secondary behavior should be changed to a no-op after no more
problems arise under NDEBUG mode. I reversed the suggested semantics
because I'm not ready to completely break trunk. To break everything
(AKA find where we should be providing a terminal nul) just compile
like this:
$ CLFAGS=-DNDEBUG make clean all
3) replaces all memsets (excepting the ones in safe_?alloc) that act
on char bufs with said macro, so they can be spotted and improved to
deal with nul terminators where needed. I didn't touch any memsets on
struct pointers.
4) made jid_get_*() from src/libopensrf/transport_message.c safe for
use with NDEBUG mode. They were depending on the target buffer that
the caller passes in to be nul-filled (they use strncpy/memcpy which
don't guarantee terminal nul) -- now they provide their own terminal
nul.
Broad patch from Dan Scott to move towards better memory management:
* bzero->memset (with sizeof) - except when followed immediately by
snprintf(), in which case the call was deleted completely
* sprintf->snprintf (with sizeof) - for the C99-guaranteed
null-terminated string and avoidance of overwrites
* fgets (with sizeof) - because in at least one case "len - 1" was
being used for the length, even though fgets is defined to retrieve 1
byte less than the requested length - so the code was effectively
retrieving 2 bytes less than the allocated buffer
* in 2 places in srfsh.c, increased buffer size by 1 to allow for
null terminator
* various typo fixes
Continued const-correctness improvement from Scott McKellar:
1. I changed the signature of jsonObjectDecodeClass so that it accepts
a non-const pointer parameter. Since it doesn't change the contents
of the jsonObject, there's no need to confine it to non-const
jsonObjects.
2. I replaced jsonObjectGetKey() with jsonObjectGetKeyConst().
3. In one spot I captured the return value of jsonObjectGetIndex()
with a const pointer instead of a non-const pointer.
Patch from Scott McKellar to improve const-correctness in srfsh:
This patch sprinkles a few const qualifiers over srfsh.c, and also
fixes a couple of things I stumbled across along the way.
The main purpose is to treat jsonObjects in a more const-correct
manner. I introduced a new jsonObjectGetKeyConst function, and I
prepared for the day when jsonObjectGetString() returns a const
pointer instead of a non-const pointer.
This patch relies on a previous patch that defines the new
jsonObjectGetKeyConst function.
1. In handle_login() I changed hash to a const pointer, so that we
can assign the return value of jsonObjectGetString() to it.
2. Later in the same function, I rearranged the code a bit to ensure
that we always free a prior value of login_session before giving it
a new value. The original code potentially leaks memory.
3. In the same area I changed authtoken to a const pointer.
4. In send_request() I eliminated an unnecessary test for the
non-nullness of omsg->_result_content, because we had just tested
it a few lines ago.
5. Also in send_request(): I introduced a layer of strdup() when
we get content from jsonObjectGetString(). This change concerns
more than just const-correctness.
The problem with the original code is that we get content sometimes
from jsonFormatString() and sometimes from jsonObjectGetString().
The former returns a malloc'ed buffer, which we need to free. The
latter returns a pointer to a buffer owned by a jsonObject, and we
should not free it. As it happens, we do free it. If we got the
string from jsonObjectGetString(), then we leave the jsonObject with
an invalid pointer inside it. If we free the jsonObject later, we'll
try to free that same buffer a second time. Oops.
Another issue is that jsonObjectGetString() can return NULL, which
we should handle more carefully.
These problems occur in two different places in the same function.
I fixed them both the same way, with some differences in the details.
Patch from Scott McKellar which introduces a const-accepting and -returning
version of jsonObjectGetKey. This is the first step in a plan to push
const-correctness on the OpenSRF stack when dealing with complex objects. The
hope is that this will increase the utility of compile-time checks in new
client code.
erickson [Sun, 19 Aug 2007 01:16:02 +0000 (01:16 +0000)]
added support for multi-threaded client interactions. much like the java lib, each thread is allowed 1 jabber connection, as opposed to 1 process-wide jabber connection. also did some re-tabbing to force 4-space tabs (not raw tabs) - more of those to come
erickson [Fri, 17 Aug 2007 21:57:16 +0000 (21:57 +0000)]
added multi-threaded client support to the opensrf network/xmpp layer
this is all managed below the covers so that clients can continue to safely
use bootstrapClient and will all "just work"
we now allow 1 xmpp connection per thread, as opposed to 1 per process.
making sure error logs are logged to stderr even if syslogging is not an option -- eventually, it should fall back to stderr logging like the C code does
python only evaluates default function param values once, so mutable types will always be pointers to the original default val. change the default osrfNetworkObject init function to None and setting default manually
1. In handle_print(), I added the ability to display the value of
raw_print. If we can display pretty_print, we should be able to
display raw_print.
2. In a couple of places we display the value of login_session.
However if we aren't logged in, we end up passing NULL to printf(),
thereby invoking undefined behavior. Apparently glibc responds by
printing "(null)". I contrived explicitly to print "(none)" rather
than rely on glibc.
3. Since login_session is assigned from strdup() if not NULL, I added
a free() for it in a couple of places: one at the end of main(), and
one just before the assignment from strdup().
Patch from Scott McKellar to fill buffer overflow holes:
The first overflow can happen with an excessively long username.
The second overflow is more doubtful, because the inputs come from
two other functions. It's not obvious whether an overflow is possible
or not. It may be that those functions will never return strings long
enough to overflow. However it is easier to assume that they might,
and avoid the overflow for sure, than to determine whether an overflow
is possible in the first place.
In each case I declared a variable-length character array with a
calculated length.
Attached is a minor patch to OpenSRF/src/java/Makefile for creating a
opensrf.jar. I was thinking an opensrf.jar would make it easy to
deploy OpenSRF client in a Woodchip/OpenTaps instance. It's not yet
clear to me what the build environment is (or will be) for
Woodchip--but .jar files are used to across the board to manage
external dependencies in Javaland.
- - -
I've also given the depencies their own vars for easier handling and
made the dependency fetching smarter to prevent re-fetching existing deps
This moves OpenSRF to the new JSON wire protocol,
including:
a new C parser
a new osrfList based string_array module
a compatibility layer for mimicking libobjson and legacy JSON I/O in the HTTP gateway
a small JSON test program for profiling, etc.
Patch from Scott McKellar to repair srfsh buffer overflow:
The potential overflow occurs in handle_introspection(), which in the
existing code uses sprintf() to insert some user-supplied input into a
fixed length buffer, without checking the length of the user-supplied
input.
There's always more than one way to fix this sort of thing. For
example I could have formatted into a growing_buffer. Instead, I
chose to calculate the necessary buffer size and then declare a
variable-length buffer. That way I avoid two mallocs and two frees.
I tested this change by inserting some temporary printfs to capture
the commands being sent to parse_request(). The outputs before and
after the change were identical.
There's at least one more such potential overflow that I haven't
addressed yet. It's near the top of handle_login(), where we
use sprintf() to insert a username into a fixed length buffer.
There may be others that I haven't noticed yet.
Patch from Scott McKellar to clean up srfsh's user interface:
1. srfsh exits the main loop in two circumstances. The first is when
it sees the "quit" or "exit" command. The second is when it sees
end-of-file, either because it reads the end of a script or because
the user enters the associated keystrokes (typically Control-D).
In the case of end-of-file, srfsh exits without issuing another
newline. As a result, the next shell prompt shows up concatenated
to the srfsh prompt. This result is harmless but annoying.
I tweaked the code to detect this situation and issue an extra
newline, so that the next shell prompt show up on a line by itself.
2. I strip off leading whitespace before processing the input
command.
As a result, srfsh will now recognize "quit", "exit", or a leading
exclamation point, even when preceded by whitespace. Other commands
are not affected because strtok() already strips off leading
blanks. Tab characters don't matter because readline() removes them.
3. I also strip off trailing whitespace. This measure affects only
the "quit" and "exit" commands.
4. I ignore input lines whose first non-whitespace character is an
octothorpe ('#', also known as hash, pound, or tic-tac-toe). In
other words, scripts can now have comments.
Patch from Scott McKellar, with modifications, to remove unused code and provide some memory protection:
This patch contains one minor change and one less minor change.
--------
The less minor change is the elimination of some old code that once
opened a pipe to send commands to bash. According to Bill Erickson
in a private email, that code is a leftover remnant from an
experiment and may be removed.
Before a patch was applied several days ago, things worked like this:
IF you compiled srfsh with a certain macro #defined, AND you entered
a command that was invalid to srfsh, THEN srfsh would pipe the
command to bash for execution.
It is doubtful that anyone but Bill ever used this variation, but if
anyone wants to keep it around, then this is his or her chance to
protest. Before protesting, however, please note that the shell
escape mechanism now provides shell-like command processing. I.e.
if you start the srfsh command line with an exclamation point, srfsh
passes the rest of the command to the default shell (normally
/bin/sh) for execution. The command so passed can use environmental
variables, pipes, IO redirection, wild card expansion, and most of
the sorts of things you're used to from the shell command line.
Note that not all shells behave the same. If you're used to using
tcsh as your shell, for example, you may find that sh won't work
quite the same way. For now, at least, that's just too bad, but
that's the way shell escapes typically work.
It is probably possible to invoke the user's default shell as
identified by the environmental variable $SHELL. However that
nicety would not be trivial to code.
----------
The minor change is that, after building an array of pointers
pointing to tokens from the input command, I set the next
pointer to NULL so that it mark the end of the token list.
(miker: I changed to calloc/free instead of an array, but left the final
NULL in place.)
Patch from Scott McKellar to fix potential local buffer
overflow attack against srfsh:
This patch fixes a potential buffer overflow in the parse_error
function. The existing code concatenates the strtoked tokens into a
fixed-length buffer, with no check for overflow. It isn't hard to
build an srfsh command that overflows the buffer, with baleful
results.
While it's not likely that anyone would do so by accident from the
command line, an srfsh script might well do so, especially if the
script were generated from another program.
More important, someone sufficiently clever might be able to use
such an overflow to work mischief.
My version of parse_error() uses a growing_buffer to accumulate
the tokens.
Patch from Scott McKellar to remove srfsh.h and push all relevant
delarations into the implementation file, with some additional cleanup
to remove "declared by not used" warnings:
1. We have no further use for the header file. Accordingly I
include a patch to the Makefile, removing a dependency on the header.
2. I declared as static everything that was formerly in the header,
plus a few variables that were already defined in srfsh.c.
3. I commented out the shell_reader variable, since it is no longer
used.
4. I commented out two of the lines printed by the help command.
These lines refer to the "time" command, which srfsh no longer
supports.
5. I commented out the prototype for the handle_time function, since
the function itself is already commented out.
Patch from Scott McKellar for declaration cleanup and improved error handling:
1. I added the const qualifier to several function parameters.
2. I replaced inet_addr() with inet_aton().
According to the man page, inet_addr() is obsolete. It reports an
error by returning -1, which however is a valid IP address
(255.255.255.255). inet_aton reports errors in a more robust way.
3. I check the return value from inet_aton(). If the input address is
invalid, I log a message and return -1.
Without such a check, an invalid listener address leads to a server
that is completely unresponsive. The user has no way to know what's
wrong unless he spots the mistake himself in the relevant
configuration file.
Patch from Dan Scott to move JSON to OpenSRF::Utils::JSON:
I noticed back when I was first installing OpenSRF that it includes a
module, JSON.pm, that exists at the root level of the package
directories. This would be fine, except it conflicts with a CPAN
module that is also named JSON, which confuses the CPAN installer when
you check for upgrades and conceivably could lead to a broken system.
I suggested to Mike that it would probably make sense to move the
OpenSRF version of the module into the OpenSRF/Utils/ package
namespace, and he agreed. Of course, there are ramifications
throughout the code, so I've tried to be extra-careful in catching and
correcting all of the places where the use of this module surfaces in
both OpenSRF and Evergreen.
Patch from Dan Scott to finish up the removal of bootstrap.conf:
Attached are patches that complete the work that I should have done in
my first patch. I also modify the Makefile in OpenSRF to prevent
bootstrap.conf.example from being copied from the OpenSRF/examples
directory into /openils/conf/.
miker [Sat, 30 Jun 2007 03:14:36 +0000 (03:14 +0000)]
Patch from Scott McKellar implementing cleaner daemonization; moved daemonizing code above worker forking in system boostrapping:
1. As long as I was in the neighborhood, I replaced the error messages
going to stderr with messages going to the logging machinery.
2. I altered the comment just above daemonize(), because the original
was inaccurate. This function does not change the process title.
3. Pedantic point: I captured the return value of fork() with a pid_t
instead of an int.
4. After the fork, the parent process terminates with _exit()
instead of exit(). That way it doesn't flush any buffers or close
any files. It also doesn't call any functions registered with
atexit(). In fact it doesn't do much of anything except get out of
the way and let the child process take its place in every way.
5. The child process switches to the root directory, calls setsid()
to create a new session, and freopens the three standard streams
to /dev/null.
I could have changed directories before the fork, or at any of
several other places. I don't think it makes any difference.
I could also have done the freopens before the fork, but if the
fork failed, I might not have a way to report it (if the logging is
going to stderr for some reason).
Note that I'm not flushing any buffers, apart from the three standard
streams. There's no need to. The child process inherits the file
descriptors intact and can do with them whatever it likes. The
parent may have died, but the estate doesn't have to go through
probate.
I didn't add any error checking for chdir(), setsid(), or freopen().
The first two are unlikely to fail, as is the freopen of stdin. The
freopens of stderr and stdout could fail, if for example they are
redirected to a full disk. I don't know how paranoid we need to be.
miker [Fri, 29 Jun 2007 03:55:37 +0000 (03:55 +0000)]
Patch from Dan Scott to move perl OpenSRF core bootstrapping settings into
opensrf_core.xml. This removes the dependency on the INI style bootstrap.conf
file:
Building on Nathan Eady's suggestion / intention from December
(http://list.georgialibraries.org/pipermail/open-ils-dev/2006-December/000177.html),
here is a patch that enables OpenSRF to avoid duplicating settings in
opensrf_core.xml and bootstrap.conf by having both Perl and C apps
read from opensrf_core.xml
The major limitation is that I've hardcoded /config/opensrf to appear as
the 'bootstrap' section of config for compatibility with the expectations of
the rest of OpenSRF. A broader patch would convert all of those other calls
from config->bootstrap->blah to config->opensrf->blah and make the
siblings of /config/opensrf visible in the config as well, as a more
generally useful approach.
Applied with minor changes to avoid API regressions and remove unneeded code.
miker [Fri, 29 Jun 2007 02:24:47 +0000 (02:24 +0000)]
arg ... Patch from Scott McKellar that:
1. When we log messages to a log file, we open and close the file
for each message. If the open fails, we write a message to stderr
about the failure to open, but we discard the original message that
we were asked to issue.
With this patch, after issuing the message about the failure to
open, we write the original message to stderr.
I believe that this change plugs the last message leak. Now if we're
asked to issue a message, by golly we're going to issue it, one way
or another.
Of course the user may still not see the message, either through
inattention or, for example, because stderr is redirected to
/dev/null. In that case the user is just a poo-poo head, and I have
no sympathy for him.
2. In an earlier post I proposed to change osrfLogSetType() so that
it would return the previous log type. That way one could
temporarily change the log type and then restore it later, without
knowing in advance what to restore it to.
Upon further reflection I decided that the only plausible use for
this trick would be to reroute messages to standard error, and I
might as well provide a mechanism limited to this purpose.
Accordingly I created two new functions and prototyped them in log.h:
osrfLogToStderr() reroutes messages to stderr, and saves the previous
log type internally.
osrfRestoreLogType() restores the log type previously saved, if any.
This interface provides fewer ways for the calling code to mess up
than what I had originally contemplated. First, the calling code
doesn't have to keep track of the previous log type. Second, if the
messages are already rerouted, osrfLogToStderr() will do nothing.
One needn't worry about nested reroutings, or maintaining a stack of
log types, or anything of the sort.
If we ever need anything fancier we can build it, but I don't think
we will.
3. Wherever a function takes no parameters, I coded "void" as the
formal parameter list. Otherwise the compiler doesn't know whether
the function takes parameters or not.
miker [Thu, 28 Jun 2007 03:14:06 +0000 (03:14 +0000)]
Patch from Scott McKellar:
At one time. if we had trouble reading or parsing the configuration
file, we would issue log messages to complain. However those log
messages didn't have anywhere to go because, not having loaded the
configuration file, we didn't know where to write the messages.
The earlier patch wrote the same messages to standard error as well
as to the logging machinery, so that they would be visible.
These extra writes are no longer necessary, With recent changes to
the logging machinery, all log messages will go to standard error
if no other destination has been defined for them.
Accordingly this patch removes two messages, now redundant, that
were being written to standard error.
miker [Tue, 26 Jun 2007 03:04:37 +0000 (03:04 +0000)]
Patch from Scott McKellar, finishing off the "log to stderr when all esle fails" work:
Currently if we have asked to log to a file, but we haven't yet
specified the name of that log file, then any messages issued are
dropped.
With this patch, any messages issued under these conditions will be
rerouted to standard error. The rerouting is only temporary,
applying only to the current message.
miker [Mon, 25 Jun 2007 03:06:56 +0000 (03:06 +0000)]
Patch from Scott McKellar to improve default logging:
1. In log.h I added another macro OSRF_LOG_TYPE_STDERR by which the
application can specifically request logging to standard error.
2. In log.c I initialze _osrfLogType to OSRF_LOG_TYPE_STDERR so that
the application can issue a log message even if it hasn't called
osrfLogInit() yet.
3. I added the const qualifier to the parameters of the functions
_osrfLogToFile() and _osrfLogSetXid().
4. In osrfLogCleanup() I set _osrfLogAppname and _osrfLogFile to
NULL after freeing them. Otherwise the application could cause
needless mischief by calling this function twice.
5. Also in osrfLogCleanup() I set _osrfLogType back to the default
OSRF_LOG_TYPE_STDERR so that the application can still issue a
message after calling osrfLogCleanup().
6. I rewrote osrfLogSetType() to use switch/case instead of an "if".
Now that we have three valid values to check instead of two, a
switch/case is a little tidier. Also the default branch applies
the default log type OSRF_LOG_TYPE_STDERR.
7. _osrfLogDetail() had a local variable named "l" (lower case L).
I renamed it to "label" because "l" looks too much like a digit.
8.Also in _osrfLogDetail(): I added a branch to write a message to
stderr when _osrfLogType is OSRF_LOG_TYPE_STDERR.
9. The existing _osrfLogToFile function sizes and declares a buffer
named "buf", fills it with zeros, and then doesn't do anything with
it. This buffer is probably a relic of some earlier version. I
eliminated it.
10. A few lines thereafter, I eliminated a useless call to bzero().
11. The call to strftime() contained a hard-coded buffer size. I
changed it to use the sizeof operator, so that the buffer size is
defined in only one place.
12. If we can't open the log file, we write a message to stderr.
In this message I changed "file" to "log file" to make the meaning
more clear to a benighted and panicky user.
13. If after writing the message we can't close the file, the
existing code logs an error message by calling osrfLogWarning().
However in this circumstance this latter message will probably meet
the same fate, and no one will ever see it. I changed the code so
as to write this message to stderr.
projects / OpenSRF.git / log