You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/07/09 13:57:46 UTC
svn commit: r1501207 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
Author: rhuijben
Date: Tue Jul 9 11:57:46 2013
New Revision: 1501207
URL: http://svn.apache.org/r1501207
Log:
Stop ignoring xml parser errors in three places in ra_serf. These parse errors
might be caused by broken network requests, or requests that should have had a
body. In all cases ignoring the parser errors made true errors hard to detect.
* subversion/libsvn_ra_serf/util.c
(inject_to_parser,
svn_ra_serf__process_pending,
svn_ra_serf__handle_xml_parser): Properly handle xml parser errors.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/util.c
Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1501207&r1=1501206&r2=1501207&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Tue Jul 9 11:57:46 2013
@@ -1392,19 +1392,21 @@ inject_to_parser(svn_ra_serf__xml_parser
xml_status = XML_Parse(ctx->xmlp, data, (int) len, 0);
-if (ctx->error && !ctx->ignore_errors)
- return svn_error_trace(ctx->error);
-
- if (xml_status == XML_STATUS_ERROR && !ctx->ignore_errors)
+ if (! ctx->ignore_errors)
{
- if (sl == NULL)
- return svn_error_createf(SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
- _("XML parsing failed"));
-
- return svn_error_createf(SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
- _("XML parsing failed: (%d %s)"),
- sl->code, sl->reason);
- }
+ SVN_ERR(ctx->error);
+
+ if (xml_status != XML_STATUS_OK)
+ {
+ if (sl == NULL)
+ return svn_error_createf(SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
+ _("XML parsing failed"));
+
+ return svn_error_createf(SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
+ _("XML parsing failed: (%d %s)"),
+ sl->code, sl->reason);
+ }
+ }
return SVN_NO_ERROR;
}
@@ -1485,14 +1487,24 @@ svn_ra_serf__process_pending(svn_ra_serf
if (pending_empty &&
parser->pending->network_eof)
{
+ int xml_status;
SVN_ERR_ASSERT(parser->xmlp != NULL);
- /* Tell the parser that no more content will be parsed. Ignore the
- return status. We just don't care. */
- (void) XML_Parse(parser->xmlp, NULL, 0, 1);
+ /* Tell the parser that no more content will be parsed. */
+ xml_status = XML_Parse(parser->xmlp, NULL, 0, 1);
apr_pool_cleanup_run(parser->pool, &parser->xmlp, xml_parser_cleanup);
parser->xmlp = NULL;
+
+ if (! parser->ignore_errors)
+ {
+ SVN_ERR(parser->error);
+
+ if (xml_status != XML_STATUS_OK)
+ {
+ }
+ }
+
add_done_item(parser);
}
@@ -1707,15 +1719,24 @@ svn_ra_serf__handle_xml_parser(serf_requ
in the PENDING structures, then we're completely done. */
if (!HAS_PENDING_DATA(ctx->pending))
{
- XML_Status xml_status;
+ int xml_status;
SVN_ERR_ASSERT(ctx->xmlp != NULL);
- /* Ignore the return status. We just don't care. */
xml_status = XML_Parse(ctx->xmlp, NULL, 0, 1);
apr_pool_cleanup_run(ctx->pool, &ctx->xmlp, xml_parser_cleanup);
- if (ctx->xmlp->
+ if (! ctx->ignore_errors)
+ {
+ SVN_ERR(ctx->error);
+
+ if (xml_status != XML_STATUS_OK)
+ {
+ return svn_error_create(
+ SVN_ERR_XML_MALFORMED, NULL,
+ _("The XML response contains invalid XML"));
+ }
+ }
add_done_item(ctx);
}
RE: svn commit: r1501207 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: vrijdag 12 juli 2013 15:27
> To: dev@subversion.apache.org; Bert Huijben
> Subject: Re: svn commit: r1501207 -
> /subversion/trunk/subversion/libsvn_ra_serf/util.c
>
> On Tue, Jul 9, 2013 at 3:57 PM, <rh...@apache.org> wrote:
> > Author: rhuijben
> > Date: Tue Jul 9 11:57:46 2013
> > New Revision: 1501207
> >
> > URL: http://svn.apache.org/r1501207
> > Log:
> > Stop ignoring xml parser errors in three places in ra_serf. These parse
> errors
> > might be caused by broken network requests, or requests that should have
> had a
> > body. In all cases ignoring the parser errors made true errors hard to
> detect.
> >
> > * subversion/libsvn_ra_serf/util.c
> > (inject_to_parser,
> > svn_ra_serf__process_pending,
> > svn_ra_serf__handle_xml_parser): Properly handle xml parser errors.
> >
> [...]
>
> > @@ -1485,14 +1487,24 @@ svn_ra_serf__process_pending(svn_ra_serf
> > if (pending_empty &&
> > parser->pending->network_eof)
> > {
> > + int xml_status;
> > SVN_ERR_ASSERT(parser->xmlp != NULL);
> >
> > - /* Tell the parser that no more content will be parsed. Ignore the
> > - return status. We just don't care. */
> > - (void) XML_Parse(parser->xmlp, NULL, 0, 1);
> > + /* Tell the parser that no more content will be parsed. */
> > + xml_status = XML_Parse(parser->xmlp, NULL, 0, 1);
> >
> > apr_pool_cleanup_run(parser->pool, &parser->xmlp,
> xml_parser_cleanup);
> > parser->xmlp = NULL;
> > +
> > + if (! parser->ignore_errors)
> > + {
> > + SVN_ERR(parser->error);
> > +
> > + if (xml_status != XML_STATUS_OK)
> > + {
> > + }
> Hi Bert,
>
> What is the purpose of this condition with empty body?
I intended to copy&paste one of the existing error blocks here, but I forgot to do that before committing.
Fixed in r1502777.
Bert
Re: svn commit: r1501207 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Jul 9, 2013 at 3:57 PM, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Tue Jul 9 11:57:46 2013
> New Revision: 1501207
>
> URL: http://svn.apache.org/r1501207
> Log:
> Stop ignoring xml parser errors in three places in ra_serf. These parse errors
> might be caused by broken network requests, or requests that should have had a
> body. In all cases ignoring the parser errors made true errors hard to detect.
>
> * subversion/libsvn_ra_serf/util.c
> (inject_to_parser,
> svn_ra_serf__process_pending,
> svn_ra_serf__handle_xml_parser): Properly handle xml parser errors.
>
[...]
> @@ -1485,14 +1487,24 @@ svn_ra_serf__process_pending(svn_ra_serf
> if (pending_empty &&
> parser->pending->network_eof)
> {
> + int xml_status;
> SVN_ERR_ASSERT(parser->xmlp != NULL);
>
> - /* Tell the parser that no more content will be parsed. Ignore the
> - return status. We just don't care. */
> - (void) XML_Parse(parser->xmlp, NULL, 0, 1);
> + /* Tell the parser that no more content will be parsed. */
> + xml_status = XML_Parse(parser->xmlp, NULL, 0, 1);
>
> apr_pool_cleanup_run(parser->pool, &parser->xmlp, xml_parser_cleanup);
> parser->xmlp = NULL;
> +
> + if (! parser->ignore_errors)
> + {
> + SVN_ERR(parser->error);
> +
> + if (xml_status != XML_STATUS_OK)
> + {
> + }
Hi Bert,
What is the purpose of this condition with empty body?
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com