[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1704374 - in /subversion/trunk/subversion: include/svn_diff.h include/svn_error_codes.h libsvn_diff/binary_diff.c libsvn_diff/diff.h libsvn_diff/parse-diff.c tests/cmdline/patch_tests.py

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Wed, 23 Sep 2015 16:16:04 +0100

Johan Corveleyn wrote:
> Bert Huijben wrote:
>> Johan Corveleyn wrote:
>>> Without completely understanding how the binary diff / patch is now
>>> implemented, I'd say: better to error out in this case. If it helps
>>> detecting corruption of the binary diff (say it was randomly changed
>>> by some wire transmission) ...
>>
>> This is completely against the rest of the patch implementation, and how other diff implementations work.
>>
>> For many reasons (including: legacy, compatibility, etc.) all of them ignore everything they don't understand. The current (1.8/1.9) patch code just ignores these hunks and doesn't error on them.
>>
>> The patch code only errors out on fatal errors.

I don't follow. Johan implicity argues that this sequence of base-85
input characters considered bad data. You (Bert) are saying 'svn
patch' should not error out when it gets bad data it should (and does)
ignore the patch. But it doesn't in this case. Instead, it converts
the input into some (more or less arbitray) output data and applies
it.

We're talking about the behaviour of one case inside the base-85
decoder. The decoder errors out in other cases:

  const char *p = strchr(b85str, c);
  if (!p)
    return svn_error_create(SVN_ERR_DIFF_UNEXPECTED_DATA, NULL,
                            _("Invalid base85 value"));
[...]
    if (base85_len != expected_data)
      return svn_error_create(SVN_ERR_DIFF_UNEXPECTED_DATA, NULL,
                              _("Unexpected base85 line length"));

We're talking about two different levels. Inside this base-85 decoder
we're talking about whether to return an error or some (more or less
arbitrary) output value when we read a 'bad' sequence of input
characters. If you want 'svn patch' to ignore the patch in this case,
then the base-85 decoder MUST detect this case and return an error.
Currently it doesn't.

Or did I misunderstand you?

- Julian
Received on 2015-09-23 17:16:52 CEST

This is an archived mail posted to the Subversion Dev mailing list.