* Handling empty netconf XML messages "]]>]]>" is changed from being accepted to return an error.

* Netconf split lines input (input fragments) fixed
  * Netconf input split on several lines, eg using stdin: "<a>\nfoo</a>]]>]]>" could under some circumstances be split so that only "</a>]]>]]>" be properly processed. This could also happen to a socket receiving a sub-string and then after a delay receive the rest.
  * Fixed by storing residue and add that to the input string if later input is received on the same socket.
* Use [https://github.com/clicon/libevhtp](https://github.com/clicon/libevhtp) instead of [https://github.com/criticalstack/libevhtp](https://github.com/criticalstack/libevhtp) as a source of the evhtp source
This commit is contained in:
Olof hagsand 2021-01-07 16:57:47 +01:00
parent cf63d0f761
commit 8cde7a4ded
11 changed files with 70 additions and 15 deletions

View file

@ -1,6 +1,6 @@
# Clixon Changelog # Clixon Changelog
* [4.10.0](#4100) Expected: February * [4.10.0](#4100) Expected: February 2021
* [4.9.0](#490) 18 December 2020 * [4.9.0](#490) 18 December 2020
* [4.8.0](#480) 18 October 2020 * [4.8.0](#480) 18 October 2020
* [4.7.0](#470) 14 September 2020 * [4.7.0](#470) 14 September 2020
@ -39,6 +39,7 @@ Developers may need to change their code
Users may have to change how they access the system Users may have to change how they access the system
* Handling empty netconf XML messages "]]>]]>" is changed from being accepted to return an error.
* New clixon-lib@2020-12-30.yang revision * New clixon-lib@2020-12-30.yang revision
* Changed: RPC process-control output parameter status to pid * Changed: RPC process-control output parameter status to pid
* New clixon-config@2020-12-30.yang revision * New clixon-config@2020-12-30.yang revision
@ -49,6 +50,7 @@ Users may have to change how they access the system
### Minor changes ### Minor changes
* Use [https://github.com/clicon/libevhtp](https://github.com/clicon/libevhtp) instead of [https://github.com/criticalstack/libevhtp](https://github.com/criticalstack/libevhtp) as a source of the evhtp source
* Added callback to process-control RPC feature in clixon-lib.yang to manage processes * Added callback to process-control RPC feature in clixon-lib.yang to manage processes
* WHen an RPC comes in, be able to look at configuration * WHen an RPC comes in, be able to look at configuration
* Changed behavior of starting restconf internally using `CLICON_BACKEND_RESTCONF_PROCESS` monitoring changes in enable flag, not only the RPC. The semantics is as follows: * Changed behavior of starting restconf internally using `CLICON_BACKEND_RESTCONF_PROCESS` monitoring changes in enable flag, not only the RPC. The semantics is as follows:
@ -61,6 +63,9 @@ Users may have to change how they access the system
### Corrected Bugs ### Corrected Bugs
* Netconf split lines input (input fragments) fixed
* Netconf input split on several lines, eg using stdin: "<a>\nfoo</a>]]>]]>" could under some circumstances be split so that only "</a>]]>]]>" be properly processed. This could also happen to a socket receiving a sub-string and then after a delay receive the rest.
* Fixed by storing residue and add that to the input string if later input is received on the same socket.
* [Presence container configs not displayed in 'show config set' #164 ](https://github.com/clicon/clixon/issues/164) * [Presence container configs not displayed in 'show config set' #164 ](https://github.com/clicon/clixon/issues/164)
* Treat presence container as a leaf: always print a placeholder regardless if it has children or not. An extra check for children could have been made to not print if it has, but this adds an extra minor complexity. * Treat presence container as a leaf: always print a placeholder regardless if it has children or not. An extra check for children could have been made to not print if it has, but this adds an extra minor complexity.

View file

@ -75,6 +75,12 @@
#define NETCONF_LOGFILE "/tmp/clixon_netconf.log" #define NETCONF_LOGFILE "/tmp/clixon_netconf.log"
/* clixon-data value to save buffer between invocations.
* Saving data may be necessary if socket buffer contains partial netconf messages, such as:
* <foo/> ..wait 1min ]]>]]>
*/
#define NETCONF_HASH_BUF "netconf_input_cbuf"
/*! Ignore errors on packet errors: continue */ /*! Ignore errors on packet errors: continue */
static int ignore_packet_errors = 1; static int ignore_packet_errors = 1;
@ -265,6 +271,17 @@ netconf_input_frame(clicon_handle h,
clicon_err(OE_UNIX, errno, "strdup"); clicon_err(OE_UNIX, errno, "strdup");
goto done; goto done;
} }
/* Special case: */
if (strlen(str) == 0){
if ((cbret = cbuf_new()) == NULL){
clicon_err(OE_UNIX, errno, "cbuf_new");
goto done;
}
if (netconf_operation_failed(cbret, "rpc", "Empty XML")< 0)
goto done;
netconf_output_encap(1, cbret, "rpc-error");
goto ok;
}
/* Parse incoming XML message */ /* Parse incoming XML message */
if ((ret = clixon_xml_parse_string(str, YB_RPC, yspec, &xtop, &xret)) < 0){ if ((ret = clixon_xml_parse_string(str, YB_RPC, yspec, &xtop, &xret)) < 0){
if ((cbret = cbuf_new()) == NULL){ if ((cbret = cbuf_new()) == NULL){
@ -331,6 +348,10 @@ netconf_input_frame(clicon_handle h,
* This routine continuously reads until no more data on s. There could * This routine continuously reads until no more data on s. There could
* be risk of starvation, but the netconf client does little else than * be risk of starvation, but the netconf client does little else than
* read data so I do not see a danger of true starvation here. * read data so I do not see a danger of true starvation here.
* @note data is saved in clicon-handle at NETCONF_HASH_BUF since there is a potential issue if data
* is not completely present on the s, ie if eg:
* <a>foo ..pause.. </a>]]>]]>
* then only "</a>" would be delivered to netconf_input_frame().
*/ */
static int static int
netconf_input_cb(int s, netconf_input_cb(int s,
@ -344,10 +365,23 @@ netconf_input_cb(int s,
cbuf *cb=NULL; cbuf *cb=NULL;
int xml_state = 0; int xml_state = 0;
int poll; int poll;
clicon_hash_t *cdat = clicon_data(h); /* Save cbuf between calls if not done */
size_t cdatlen = 0;
void *ptr;
if ((cb = cbuf_new()) == NULL){ if ((ptr = clicon_hash_value(cdat, NETCONF_HASH_BUF, &cdatlen)) != NULL){
clicon_err(OE_XML, errno, "cbuf_new"); if (cdatlen != sizeof(cb)){
return retval; clicon_err(OE_XML, errno, "size mismatch %lu %lu", cdatlen, sizeof(cb));
goto done;
}
cb = *(cbuf**)ptr;
clicon_hash_del(cdat, NETCONF_HASH_BUF);
}
else{
if ((cb = cbuf_new()) == NULL){
clicon_err(OE_XML, errno, "cbuf_new");
goto done;
}
} }
memset(buf, 0, sizeof(buf)); memset(buf, 0, sizeof(buf));
while (1){ while (1){
@ -386,8 +420,15 @@ netconf_input_cb(int s,
/* poll==1 if more, poll==0 if none */ /* poll==1 if more, poll==0 if none */
if ((poll = clixon_event_poll(s)) < 0) if ((poll = clixon_event_poll(s)) < 0)
goto done; goto done;
if (poll == 0) if (poll == 0){
break; /* No data to read */ /* No data to read, save data and continue on next round */
if (cbuf_len(cb) != 0){
if (clicon_hash_add(cdat, NETCONF_HASH_BUF, &cb, sizeof(cb)) == NULL)
return -1;
cb = NULL;
}
break;
}
} /* while */ } /* while */
retval = 0; retval = 0;
done: done:

View file

@ -12,7 +12,7 @@ There are two installation instructions: for libevhtp and nginx.
Download, build and install libevhtp from source. Prereqs: libevent and cmake. Download, build and install libevhtp from source. Prereqs: libevent and cmake.
``` ```
git clone https://github.com/criticalstack/libevhtp.git git clone https://github.com/clicon/libevhtp.git
cd libevhtp/build cd libevhtp/build
cmake -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON .. cmake -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON ..
make make

2
configure vendored
View file

@ -5053,7 +5053,7 @@ if test "x$ac_cv_header_evhtp_evhtp_h" = xyes; then :
_ACEOF _ACEOF
else else
as_fn_error $? "evhtp header missing. See https://github.com/criticalstack/libevhtp" "$LINENO" 5 as_fn_error $? "evhtp header missing. See https://github.com/clicon/libevhtp" "$LINENO" 5
fi fi
done done

View file

@ -211,7 +211,7 @@ if test "x${with_restconf}" == xfcgi; then
# Lives in libfcgi-dev # Lives in libfcgi-dev
AC_CHECK_LIB(fcgi, FCGX_Init,, AC_MSG_ERROR([libfcgi-dev missing])) AC_CHECK_LIB(fcgi, FCGX_Init,, AC_MSG_ERROR([libfcgi-dev missing]))
elif test "x${with_restconf}" == xevhtp; then elif test "x${with_restconf}" == xevhtp; then
AC_CHECK_HEADERS(evhtp/evhtp.h,, AC_MSG_ERROR([evhtp header missing. See https://github.com/criticalstack/libevhtp])) AC_CHECK_HEADERS(evhtp/evhtp.h,, AC_MSG_ERROR([evhtp header missing. See https://github.com/clicon/libevhtp]))
#LIBS += -lpthread -levent -levent_openssl -lssl -lcrypto #LIBS += -lpthread -levent -levent_openssl -lssl -lcrypto
AC_CHECK_LIB(pthread, pthread_create,, AC_MSG_ERROR([libpthread missing])) AC_CHECK_LIB(pthread, pthread_create,, AC_MSG_ERROR([libpthread missing]))
AC_CHECK_LIB(event, event_init,, AC_MSG_ERROR([libevent missing])) AC_CHECK_LIB(event, event_init,, AC_MSG_ERROR([libevent missing]))

View file

@ -47,7 +47,7 @@ RUN apk add --update libevent cmake libevent-dev
# clone libevhtp # clone libevhtp
WORKDIR /clixon WORKDIR /clixon
RUN git clone https://github.com/criticalstack/libevhtp.git RUN git clone https://github.com/clicon/libevhtp.git
WORKDIR /clixon/libevhtp/build WORKDIR /clixon/libevhtp/build
RUN cmake -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON .. RUN cmake -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON ..
RUN make RUN make

View file

@ -47,7 +47,7 @@ RUN apk add --update libevent cmake libevent-dev
# clone libevhtp # clone libevhtp
WORKDIR /clixon WORKDIR /clixon
RUN git clone https://github.com/criticalstack/libevhtp.git RUN git clone https://github.com/clicon/libevhtp.git
WORKDIR /clixon/libevhtp/build WORKDIR /clixon/libevhtp/build
RUN cmake -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON -DBUILD_SHARED_LIBS=OFF .. RUN cmake -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON -DBUILD_SHARED_LIBS=OFF ..
RUN make RUN make

View file

@ -444,6 +444,14 @@ xmltree2cbuf(cbuf *cb,
* @see clixon_xml_parse_string * @see clixon_xml_parse_string
* @see _json_parse * @see _json_parse
* @note special case is empty XML where the parser is not invoked. * @note special case is empty XML where the parser is not invoked.
* It is questionable empty XML is legal. From https://www.w3.org/TR/2008/REC-xml-20081126 Sec 2.1:
* A well-formed document ... contains one or more elements.
* But in clixon one can invoke a parser on a sub-part of a document where it makes sense to accept
* an empty XML. For example where an empty config: <config></config> is parsed.
* In other cases, such as receiving netconf ]]>]]> it should represent a complete document and
* therefore not well-formed.
* Therefore checking for empty XML must be done by a calling function which knows wether the
* the XML represents a full document or not.
*/ */
static int static int
_xml_parse(const char *str, _xml_parse(const char *str,
@ -460,8 +468,9 @@ _xml_parse(const char *str,
int i; int i;
clicon_debug(2, "%s", __FUNCTION__); clicon_debug(2, "%s", __FUNCTION__);
if (strlen(str) == 0) if (strlen(str) == 0){
return 1; /* OK */ return 1; /* OK */
}
if (xt == NULL){ if (xt == NULL){
clicon_err(OE_XML, errno, "Unexpected NULL XML"); clicon_err(OE_XML, errno, "Unexpected NULL XML");
return -1; return -1;

View file

@ -54,7 +54,7 @@ fi
# Framing. with -q to inhibit rcv hello # Framing. with -q to inhibit rcv hello
new "Empty frame" new "Empty frame"
expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' ']]>]]>' expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' '<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><rpc-error><error-type>rpc</error-type><error-tag>operation-failed</error-tag><error-severity>error</error-severity><error-message>Empty XML</error-message></rpc-error></rpc-reply>]]>]]>'
if [ $valgrindtest -eq 0 ]; then # Some leakage in lex / error handling difficult to catch if [ $valgrindtest -eq 0 ]; then # Some leakage in lex / error handling difficult to catch
new "Frame invalid non-xml" new "Frame invalid non-xml"

View file

@ -1,5 +1,5 @@
#!/usr/bin/env bash #!/usr/bin/env bash
# Travis pre-config script. # Travis pre-config script.
# build libevhtp # build libevhtp
git clone https://github.com/criticalstack/libevhtp.git git clone https://github.com/clicon/libevhtp.git
(cd libevhtp/build && cmake -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON .. && make && sudo make install) (cd libevhtp/build && cmake -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON .. && make && sudo make install)

View file

@ -267,7 +267,7 @@ case ${with_restconf} in
fi fi
if $buildevhtp; then if $buildevhtp; then
$sshcmd << 'EOF' $sshcmd << 'EOF'
test -d libevhtp || sudo git clone https://github.com/criticalstack/libevhtp.git test -d libevhtp || sudo git clone https://github.com/clicon/libevhtp.git
cd libevhtp/build; cd libevhtp/build;
CMAKE=$(which cmake) CMAKE=$(which cmake)
sudo $CMAKE -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON -DBUILD_SHARED_LIBS=OFF .. sudo $CMAKE -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON -DBUILD_SHARED_LIBS=OFF ..