[OPEN-ILS-DEV] PATCH: osrf_system.c
Dan Scott
denials at gmail.com
Mon May 21 22:15:48 EDT 2007
Ahem. With attachment this time...
On 21/05/07, Dan Scott <denials at gmail.com> wrote:
> On 21/05/07, Mike Rylander <mrylander at gmail.com> wrote:
> > On 5/21/07, Scott McKellar <mck9 at swbell.net> wrote:
> > >
> > > --- Mike Rylander <mrylander at gmail.com> wrote:
> > >
> > > <snip -- example of mangled configuration file>
> > >
> > > > Wow ... yeah, we should definitely ignore that -- assuming it parses
> > > > properly. I never would have thought to try that, honestly, and I
> > > > can
> > > > see no use for such orphaned text nodes.
> > > >
> > > > I didn't notice such a construct in any of the example config files,
> > > > so I suppose this was an experiment on your part?
> > >
> > > Yes, it was an experiment. I figured that sooner or later somebody
> > > will do this with a config file. It's valid XML, so it will get past
> > > the XML parser, and we'd better be prepared to handle it. There are
> > > probably other perversities that we haven't dreamed up yet.
> > >
> > > > The key seems to be ILS/OpenSRF/src/utils/xml_utils.c -- specifically
> > > > _xmlToJSON(). It should be simple enough to fix this, really. We
> > > > can
> > > > just check to see if there is more than one child node for a given
> > > > parent, and ignore text nodes in that case. See
> > > >
> > > >
> > > http://open-ils.org/cgi-bin/viewcvs.cgi/ILS/OpenSRF/src/utils/xml_utils.c?r1=1.2&r2=1.3&diff_format=l
> > > >
> > > > for my provisional approach to a fix (ignore the spacing :P), and
> > > > thanks, as usual...
> > >
> > > _xmlToJSON() is part of the problem. I was finding it hard to wrap
> > > my head around how it works, so if you can find a fix before I can,
> > > that's fine.
> > >
> > > The other part of the problem is jsonObjectSetKey(). It turns the
> > > old node into an array node willy-nilly, without regard for what
> > > sort of node it was originally. In this case it had been a string
> > > node. The code winds up trying to treat a char * as if it were a
> > > jsonObjectNode *, and hilarity ensues.
> > >
> >
> > I would certainly go along with putting some type checking in
> > jsonObjectSetKey() and raising an error if the object in question
> > already holds a value.
> >
> > > It may be a mistake to ignore such extraneous text. The user may
> > > have put it there for a reason, trying to specify some option or
> > > something. If we silently ignore the text, the user thinks the
> > > config file is okay, and then can't figure out why the option didn't
> > > take (or doesn't even notice). It may be more helpful to respond
> > > with a fatal error, or at least a warning.
> >
> > I think Dan's idea of a validating schema makes the most sense in this
> > case, since that would allow us to simply update the schema in new
> > versions if we wanted to allow such constructs. That, or something
> > like it (programmatic validation for configs), is something we've
> > wanted to put some time into, but time being what it is ...
> >
> > >
> > > When I get home I'll try out your fix and subject it to some more
> > > torture tests.
> > >
> >
> > Thanks.
> >
> > --miker
> >
> > > Scott McKellar
> > > http://home.swbell.net/mck9/aargh/
> > >
> > >
> >
> >
> > --
> > Mike Rylander
> >
>
> Attached, please find a patch for an XML schema for the srfsh.xml
> config format (srfsh_config.xsd), along with a valid srfsh.xml file
> (srfsh.xml) and an invalid srfsh.xml file (srfsh_bad.xml) for testing
> purposes. I placed them in the Open-ILS/src/extras/config directory
> for now, possibly to be used in a manual check of the .srfsh.xml file
> via something like:
>
> xmllint --schema srfsh_config.xsd --noout .srfsh.xml
>
> The right place to incorporate the use of the schema is probably in
> orsfConfigInit() in osrfConfig.c, but I won't be doing that tonight :)
> Where are those round tuits when you need them?
>
> Hrm, I probably should have added a copyright statement to the schema.
> Ah well; I can always catch it in post-production.
>
> Developer's Certificate of Origin 1.1 By making a contribution to this
> project, I certify that:
>
> (a) The contribution was created in whole or in part by me and I have
> the right to submit it under the open source license indicated in the
> file; or
>
> (b) The contribution is based upon previous work that, to the best of
> my knowledge, is covered under an appropriate open source license and
> I have the right under that license to submit that work with
> modifications, whether created in whole or in part by me, under the
> same open source license (unless I am permitted to submit under a
> different license), as indicated in the file; or
>
> (c) The contribution was provided directly to me by some other person
> who certified (a), (b) or (c) and I have not modified it; and
>
> (d) In the case of each of (a), (b), or (c), I understand and agree
> that this project and the contribution are public and that a record of
> the contribution (including all personal information I submit with it,
> including my sign-off) is maintained indefinitely and may be
> redistributed consistent with this project or the open source license
> indicated in the file.
>
>
>
> --
> Dan Scott
> Laurentian University
>
--
Dan Scott
Laurentian University
-------------- next part --------------
diff -crN ../ILS/Open-ILS/src/extras/config/srfsh.xml Open-ILS/src/extras/config/srfsh.xml
*** ../ILS/Open-ILS/src/extras/config/srfsh.xml Wed Dec 31 19:00:00 1969
--- Open-ILS/src/extras/config/srfsh.xml Mon May 21 21:06:13 2007
***************
*** 0 ****
--- 1,12 ----
+ <!-- This file follows the standard C stack bootstrap config file layout -->
+ <srfsh>
+ <router_name>router</router_name>
+ <domains>
+ <domain>localhost</domain>
+ </domains>
+ <username>osrf</username>
+ <passwd>osrf</passwd>
+ <port>5222</port>
+ <logfile>/openils/var/log/srfsh.log</logfile>
+ <loglevel>4</loglevel>
+ </srfsh>
diff -crN ../ILS/Open-ILS/src/extras/config/srfsh_bad.xml Open-ILS/src/extras/config/srfsh_bad.xml
*** ../ILS/Open-ILS/src/extras/config/srfsh_bad.xml Wed Dec 31 19:00:00 1969
--- Open-ILS/src/extras/config/srfsh_bad.xml Mon May 21 21:06:13 2007
***************
*** 0 ****
--- 1,17 ----
+ <!-- This file deliberately breaks the bootstrap config file layout -->
+ <srfsh>
+ <router_name>router</router_name>
+ <domains>
+ <domain>localhost</domain>
+ </domains>
+ <!-- valid port range is between 1 and 65535 -->
+ <port>75222</port>
+ <!-- Need all of the following elements -->
+ <!--
+ <username>osrf</username>
+ <passwd>osrf</passwd>
+ <logfile>/openils/var/log/srfsh.log</logfile>
+ -->
+ <!-- valid loglevels are 1-4 -->
+ <loglevel>5</loglevel>
+ </srfsh>
diff -crN ../ILS/Open-ILS/src/extras/config/srfsh_config.xsd Open-ILS/src/extras/config/srfsh_config.xsd
*** ../ILS/Open-ILS/src/extras/config/srfsh_config.xsd Wed Dec 31 19:00:00 1969
--- Open-ILS/src/extras/config/srfsh_config.xsd Mon May 21 21:06:13 2007
***************
*** 0 ****
--- 1,55 ----
+ <?xml version="1.0" encoding="UTF-8" ?>
+ <xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema">
+
+ <!-- define types -->
+ <xs:simpleType name="loglevelType">
+ <xs:restriction base="xs:positiveInteger">
+ <xs:maxInclusive value="4"/>
+ </xs:restriction>
+ </xs:simpleType>
+
+ <xs:simpleType name="portType">
+ <xs:restriction base="xs:positiveInteger">
+ <xs:maxInclusive value="65535"/>
+ </xs:restriction>
+ </xs:simpleType>
+
+
+ <!-- define simple elements -->
+ <xs:element name="router_name" type="xs:string"/>
+ <xs:element name="domain" type="xs:string"/>
+ <xs:element name="username" type="xs:string"/>
+ <xs:element name="passwd" type="xs:string"/>
+ <xs:element name="port" type="portType"/>
+ <xs:element name="logfile" type="xs:string"/>
+ <xs:element name="loglevel" type="loglevelType"/>
+
+ <!-- group type -->
+ <xs:group name="srfshElements">
+ <xs:all>
+ <xs:element ref="router_name"/>
+ <xs:element ref="domains"/>
+ <xs:element ref="username"/>
+ <xs:element ref="passwd"/>
+ <xs:element ref="port"/>
+ <xs:element ref="logfile"/>
+ <xs:element ref="loglevel"/>
+ </xs:all>
+ </xs:group>
+
+ <!-- complex elements -->
+ <xs:element name="domains">
+ <xs:complexType>
+ <xs:sequence>
+ <xs:element ref="domain" minOccurs="1" maxOccurs="unbounded"/>
+ </xs:sequence>
+ </xs:complexType>
+ </xs:element>
+
+ <xs:element name="srfsh">
+ <xs:complexType>
+ <xs:group ref="srfshElements"/>
+ </xs:complexType>
+ </xs:element>
+
+ </xs:schema>
More information about the Open-ils-dev
mailing list