[PATCH] 666893 - Unable to make sense of XML-RPC response from server

[Nikola Pajkovsky]

By default, XML data from the network may be no larger than 512K.
We have a bug which is 733402 bytes long. Limit is now 2 * 512K.

Signed-off-by: Nikola Pajkovsky 
---
 lib/Utils/abrt_xmlrpc.cpp |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/lib/Utils/abrt_xmlrpc.cpp b/lib/Utils/abrt_xmlrpc.cpp
index ac2654e..4ae772f 100644
--- a/lib/Utils/abrt_xmlrpc.cpp
+++ b/lib/Utils/abrt_xmlrpc.cpp
@@ -52,6 +52,15 @@ void abrt_xmlrpc_conn::new_xmlrpc_client(const char* url, bool ssl_verify)
      * We do it in abrtd's main */
     /* xmlrpc_client_setup_global_const(&env); */

+    /* Bug related #666893 - Unable to make sense of XML-RPC response from server
+     * By default, XML data from the network may be no larger than 512K.
+     * Some client and server modules may fail to enforce this properly.
+     * #define XMLRPC_XML_SIZE_LIMIT_DEFAULT (512*1024)
+     *
+     * bug is long 733402 bytes for now, hope it will be enought
+     */
+    xmlrpc_limit_set(XMLRPC_XML_SIZE_LIMIT_ID, 2 * XMLRPC_XML_SIZE_LIMIT_DEFAULT);
+
     struct xmlrpc_curl_xportparms curlParms;
     memset(&curlParms, 0, sizeof(curlParms));
     /* curlParms.network_interface = NULL; - done by memset */

[Denys Vlasenko]

On Tue, 2011-01-04 at 18:32 +0100, Nikola Pajkovsky wrote:
> By default, XML data from the network may be no larger than 512K.
> We have a bug which is 733402 bytes long. Limit is now 2 * 512K.
>
> Signed-off-by: Nikola Pajkovsky

Good analysis. I think the fix is good, only comment wording needs
some tweaks:
> diff --git a/lib/Utils/abrt_xmlrpc.cpp b/lib/Utils/abrt_xmlrpc.cpp
> index ac2654e..4ae772f 100644
> --- a/lib/Utils/abrt_xmlrpc.cpp
> +++ b/lib/Utils/abrt_xmlrpc.cpp
> @@ -52,6 +52,15 @@ void abrt_xmlrpc_conn::new_xmlrpc_client(const char* url, bool ssl_verify)
>       * We do it in abrtd's main */
>      /* xmlrpc_client_setup_global_const(&env); */
> /* Bug related #666893 - Unable to make sense of XML-RPC response from server
> * By default, XML data from the network may be no larger than 512K.

I understand the text above (propose small change: refer to the bug by
URL - bugzilla.redhat.com/show_bug.cgi?id=666893 - so that people
do not need to guess *which bugzilla* it refers to), but next part ...
> * Some client and server modules may fail to enforce this properly.

I dont understand it. What "modules"?
> * #define XMLRPC_XML_SIZE_LIMIT_DEFAULT (512*1024)

This in unclear too. I propose changing the above line to
"XMLRPC_XML_SIZE_LIMIT_DEFAULT is #defined to 512k in curl headers".
> *
> * bug is long 733402 bytes for now, hope it will be enought

Broken English. Propose: "Users reported trouble with 733402 byte long
responses, hope raising the limit to 2*512k is enough"
> */
> xmlrpc_limit_set(XMLRPC_XML_SIZE_LIMIT_ID, 2 * XMLRPC_XML_SIZE_LIMIT_DEFAULT);
> struct xmlrpc_curl_xportparms curlParms;
>    memset(&curlParms, 0, sizeof(curlParms));
>    /* curlParms.network_interface = NULL; - done by memset */

[Nikola Pajkovsky]

On Wed, 05 Jan 2011 11:15:23 +0100, Denys Vlasenko  wrote:
> On Tue, 2011-01-04 at 18:32 +0100, Nikola Pajkovsky wrote:
> > By default, XML data from the network may be no larger than 512K.
> > We have a bug which is 733402 bytes long. Limit is now 2 * 512K.
> >
> > Signed-off-by: Nikola Pajkovsky
>
> Good analysis. I think the fix is good, only comment wording needs
> some tweaks:

thanks
> > diff --git a/lib/Utils/abrt_xmlrpc.cpp b/lib/Utils/abrt_xmlrpc.cpp
> > index ac2654e..4ae772f 100644
> > --- a/lib/Utils/abrt_xmlrpc.cpp
> > +++ b/lib/Utils/abrt_xmlrpc.cpp
> > @@ -52,6 +52,15 @@ void abrt_xmlrpc_conn::new_xmlrpc_client(const char* url, bool ssl_verify)
> >       * We do it in abrtd's main */
> >      /* xmlrpc_client_setup_global_const(&env); */
> > /* Bug related #666893 - Unable to make sense of XML-RPC response from server
> > * By default, XML data from the network may be no larger than 512K.
>
> I understand the text above (propose small change: refer to the bug by
> URL - bugzilla.redhat.com/show_bug.cgi?id=666893 - so that people
> do not need to guess *which bugzilla* it refers to), but next part ...

will do
> > * Some client and server modules may fail to enforce this properly.
>
> I dont understand it. What "modules"?

text was taken from xmlrpc-c/base.h
> > * #define XMLRPC_XML_SIZE_LIMIT_DEFAULT (512*1024)
>
> This in unclear too. I propose changing the above line to
> "XMLRPC_XML_SIZE_LIMIT_DEFAULT is #defined to 512k in curl headers".
> > *
> > * bug is long 733402 bytes for now, hope it will be enought
>
> Broken English. Propose: "Users reported trouble with 733402 byte long
> responses, hope raising the limit to 2*512k is enough"

ok
> > */
> > xmlrpc_limit_set(XMLRPC_XML_SIZE_LIMIT_ID, 2 * XMLRPC_XML_SIZE_LIMIT_DEFAULT);
> > struct xmlrpc_curl_xportparms curlParms;
> >    memset(&curlParms, 0, sizeof(curlParms));
> >    /* curlParms.network_interface = NULL; - done by memset */

thanks

[Denys Vlasenko]

On Wed, 2011-01-05 at 11:39 +0100, Nikola Pajkovsky wrote:
> > > --- a/lib/Utils/abrt_xmlrpc.cpp
> > > +++ b/lib/Utils/abrt_xmlrpc.cpp
> > > @@ -52,6 +52,15 @@ void abrt_xmlrpc_conn::new_xmlrpc_client(const char* url, bool ssl_verify)
> > >       * We do it in abrtd's main */
> > >      /* xmlrpc_client_setup_global_const(&env); */
> > > /* Bug related #666893 - Unable to make sense of XML-RPC response from server
> > > * By default, XML data from the network may be no larger than 512K.
> >
> > I understand the text above (propose small change: refer to the bug by
> > URL - bugzilla.redhat.com/show_bug.cgi?id=666893 - so that people
> > do not need to guess *which bugzilla* it refers to), but next part ...
>
> will do
> > > * Some client and server modules may fail to enforce this properly.
> >
> > I dont understand it. What "modules"?
>
> text was taken from xmlrpc-c/base.h

I see.

The comment should be understandable and useful for someone who reads
this code for the first time.

In this case, information "that some modules
_of curl_ do not properly check for the limit" is most likely not
interesting to the reader. I propose deleting it.
> > > * #define XMLRPC_XML_SIZE_LIMIT_DEFAULT (512*1024)
> >
> > This in unclear too. I propose changing the above line to
> > "XMLRPC_XML_SIZE_LIMIT_DEFAULT is #defined to 512k in curl headers".
> > > *
> > > * bug is long 733402 bytes for now, hope it will be enought
> >
> > Broken English. Propose: "Users reported trouble with 733402 byte long
> > responses, hope raising the limit to 2*512k is enough"
>
> ok
> > > */
> > > xmlrpc_limit_set(XMLRPC_XML_SIZE_LIMIT_ID, 2 * XMLRPC_XML_SIZE_LIMIT_DEFAULT);
> > > struct xmlrpc_curl_xportparms curlParms;
> > >    memset(&curlParms, 0, sizeof(curlParms));
> > >    /* curlParms.network_interface = NULL; - done by memset */
>
> thanks

[Nikola Pajkovsky]

On Wed, 05 Jan 2011 11:58:32 +0100, Denys Vlasenko  wrote:
> On Wed, 2011-01-05 at 11:39 +0100, Nikola Pajkovsky wrote:
> > > > --- a/lib/Utils/abrt_xmlrpc.cpp
> > > > +++ b/lib/Utils/abrt_xmlrpc.cpp
> > > > @@ -52,6 +52,15 @@ void abrt_xmlrpc_conn::new_xmlrpc_client(const char* url, bool ssl_verify)
> > > >       * We do it in abrtd's main */
> > > >      /* xmlrpc_client_setup_global_const(&env); */
> > > > /* Bug related #666893 - Unable to make sense of XML-RPC response from server
> > > > * By default, XML data from the network may be no larger than 512K.
> > >
> > > I understand the text above (propose small change: refer to the bug by
> > > URL - bugzilla.redhat.com/show_bug.cgi?id=666893 - so that people
> > > do not need to guess *which bugzilla* it refers to), but next part ...
> >
> > will do
> > > > * Some client and server modules may fail to enforce this properly.
> > >
> > > I dont understand it. What "modules"?
> >
> > text was taken from xmlrpc-c/base.h
>
> I see.
>
> The comment should be understandable and useful for someone who reads
> this code for the first time.
>
> In this case, information "that some modules
> _of curl_ do not properly check for the limit" is most likely not
> interesting to the reader. I propose deleting it.

I did it, before you sent it ;)
> > > > * #define XMLRPC_XML_SIZE_LIMIT_DEFAULT (512*1024)
> > >
> > > This in unclear too. I propose changing the above line to
> > > "XMLRPC_XML_SIZE_LIMIT_DEFAULT is #defined to 512k in curl headers".
> > > > *
> > > > * bug is long 733402 bytes for now, hope it will be enought
> > >
> > > Broken English. Propose: "Users reported trouble with 733402 byte long
> > > responses, hope raising the limit to 2*512k is enough"
> >
> > ok
> > > > */
> > > > xmlrpc_limit_set(XMLRPC_XML_SIZE_LIMIT_ID, 2 * XMLRPC_XML_SIZE_LIMIT_DEFAULT);
> > > > struct xmlrpc_curl_xportparms curlParms;
> > > >    memset(&curlParms, 0, sizeof(curlParms));
> > > >    /* curlParms.network_interface = NULL; - done by memset */
> >
> > thanks