[PATCH] localrepo: abort when processing incomplete bundle - don't crash

[Matt Mackall]

On Mon, 2011-02-21 at 01:08 +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <mads at kiilerich.com>
> # Date 1298246797 -3600
> # Branch stable
> # Node ID 0c1a5b0caff4d01f74b31b21ff1e49e8cae8cb6b
> # Parent  61a89857688898c298bfa5cc3e3a90197cfcf687
> localrepo: abort when processing incomplete bundle - don't crash
> 
> Mercurial http servers will for some reason sometimes send incomplete bundles.
> hg used to crash, usually in changegroup.py:173 parsechunk with:
>   struct.error: unpack requires a string argument of length 80
> or in revlog.py:1005 _addrevision with:
>   mpatch.mpatchError: patch cannot be decoded
> 
> With this change we intercept these exceptions at a higher level and try to
> give a hint what the problem is:
>   error processing received changelog group

This is ok, but I worry that it's not quite to the point. Everything
points to the problem being "remote side disconnected unexpectedly". So
we should report that explicitly.

I think it would be better to check for EOF.. somewhere. For one, we're
not checking that the read in parsechunk is complete.

Though it seems like we are checking for EOF at unbundle10.chunk..

> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -10,7 +10,7 @@
>  import repo, changegroup, subrepo, discovery, pushkey
>  import changelog, dirstate, filelog, manifest, context, bookmarks
>  import lock, transaction, store, encoding
> -import util, extensions, hook, error
> +import util, extensions, hook, error, mpatch, struct
>  import match as matchmod
>  import merge as mergemod
>  import tags as tagsmod
> @@ -1800,8 +1800,12 @@
>              pr = prog()
>              source.callback = pr
>  
> -            if (cl.addgroup(source, csmap, trp) is None
> -                and not emptyok):
> +            try:
> +                node = cl.addgroup(source, csmap, trp)
> +            except (mpatch.mpatchError, struct.error):
> +                self.ui.traceback()
> +                raise util.Abort(_("error processing received changelog group"))
> +            if (node is None and not emptyok):
>                  raise util.Abort(_("received changelog group is empty"))
>              clend = len(cl)
>              changesets = clend - clstart
> diff --git a/mercurial/mpatch.c b/mercurial/mpatch.c
> --- a/mercurial/mpatch.c
> +++ b/mercurial/mpatch.c
> @@ -464,7 +464,14 @@
>  PyMODINIT_FUNC
>  initmpatch(void)
>  {
> -	Py_InitModule3("mpatch", methods, mpatch_doc);
> +	PyObject *m;
> +
> +	m = Py_InitModule3("mpatch", methods, mpatch_doc);
> +	if (m == NULL)
> +		return;
> +
>  	mpatch_Error = PyErr_NewException("mpatch.mpatchError", NULL, NULL);
> +	Py_INCREF(mpatch_Error);
> +	PyModule_AddObject(m, "mpatchError", mpatch_Error);
>  }
>  #endif
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel


-- 
Mathematics is the supreme nostalgia of our time.