Re: svn commit: r1570434 - /subversion/trunk/subversion/svnserve/cyrus_auth.c

2014-02-21 Thread Stefan Sperling
On Fri, Feb 21, 2014 at 03:06:39AM -, bre...@apache.org wrote:
> Author: breser
> Date: Fri Feb 21 03:06:39 2014
> New Revision: 1570434
> 
> URL: http://svn.apache.org/r1570434
> Log:
> Fix potential integer overflow in Cyrus SASL support for svnserve.
> 
> * subversion/svnserve/cyrus_auth.c
>   (try_auth): error if we'll overflow the clientinlen argument to sasl
> functions, typecast to silence the compiler warning.
> 
> Modified:
> subversion/trunk/subversion/svnserve/cyrus_auth.c

> -  result = sasl_server_step(sasl_ctx, in->data, in->len, &out, &outlen);
> +
> +  if (in->len > UINT_MAX)
> +return svn_error_createf(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
> + _("Step response is too long"));
> +
> +  result = sasl_server_step(sasl_ctx, in->data, (unsigned) in->len,
> +&out, &outlen);

Nice fix. Can we please write 'unsigned int'? I know there is a
default size, which is probably int, but I already have to remember
so many other idiosyncrasies about C...


Re: Bug in ra_serf with client certificates

2014-02-21 Thread Thomas Åkesson

On 28 jan 2014, at 14:37, Lieven Govaerts  wrote:

> On Tue, Jan 28, 2014 at 1:53 PM, Branko Čibej  wrote:
> 
>> [Tue Jan 28 13:32:47 2014] [info] SSL Library Error: 336105671
>> error:140890C7:SSL routines:SSL3_GET_CLIENT_CERTIFICATE:peer did not return
>> a certificate No CAs known to server for verification?
>> 
>> 
>> The bug, as I see it, is that in this case, the command-line client doesn't
>> ask for different credentials. Shouldn't we be transforming (or wrapping)
>> SERF_ERROR_AUTHN_FAILED to SVN_ERR_RA_NOT_AUTHORIZED?
> 
> The command line client doesn't ask for a client certificate, it
> should be defined correctly in the servers file using:
> ssl-client-cert-file
> ssl-client-cert-password

Sorry, I am late to this party. Just got confused by this statement that 
command line client does not ask.

svn info https://secure.example.com
Autentiseringsregion (realm): https://secure.example.com:443
Filnamn för klientcertifikat: 

This happened to become Swedish but the last line asks for a filename of client 
cert. This was 1.7.7 that I had on an old test machine.

Attempting this on 1.8 gives an SSL error as this thread has already stated.


Thanks,
Thomas Å.

Re: Apache Subversion 1.8.8 released

2014-02-21 Thread Nico Kadel-Garcia
Great!

I've updated and run some casual tests on my github published toolkits
for building this on RHEL 6. They're
at:https://github.com/nkadel/subversion-1.8.x-srpm.

The dependencies for "java-devel-openjdk" in RHEL compilation can
cause some confusion, due to the multiple i386, x86_64, version 1.6.0
and 1.7.0 parallel versions of java-openjdk-1.6.0-devel and
java-openjdk-1.7.0-devel.

Is there any particular preference now for Java 7 versus Java 6, Or
should we be just fine compiling this relevant Java components with
any version of Java. I realize that Java 6 is being heavily deprecated
by Oracle.

  Nico Kadel-Garcia



On Thu, Feb 20, 2014 at 11:56 AM, Ben Reser  wrote:
> I'm happy to announce the release of Apache Subversion 1.8.8.
>
> Please note that Subversion 1.8.8 is the next release after Subversion 1.8.5.
> The 1.8.6 and 1.8.7 releases were not published publicly, due to issues found
> during testing.
>
> This release addresses one security issues:
> CVE-2014-0032: mod_dav_svn DoS vulnerability with SVNListParentPath.
>
> Please choose the mirror closest to you by visiting:
>
> http://subversion.apache.org/download/#recommended-release
>
> The SHA1 checksums are:
>
> 8e9f10b7a9704c90e17cfe76fd56e3fe74c01a7a subversion-1.8.8.tar.bz2
> 37790421139d8ce6643a5e690f2cb718ee818cea subversion-1.8.8.zip
> 0317474e42ba9fdd122030e40b862617ae97a5d0 subversion-1.8.8.tar.gz
>
> PGP Signatures are available at:
>
> http://www.apache.org/dist/subversion/subversion-1.8.8.tar.bz2.asc
> http://www.apache.org/dist/subversion/subversion-1.8.8.tar.gz.asc
> http://www.apache.org/dist/subversion/subversion-1.8.8.zip.asc
>
> For this release, the following people have provided PGP signatures:
>
>Ben Reser [4096R/16A0DE01] with fingerprint:
> 19BB CAEF 7B19 B280 A0E2  175E 62D4 8FAD 16A0 DE01
>Bert Huijben [4096R/CCC8E1DF] with fingerprint:
> 3D1D C66D 6D2E 0B90 3952  8138 C4A6 C625 CCC8 E1DF
>Branko Čibej [2048R/C8628501] with fingerprint:
> 8769 28CD 4954 EA74 87B6  B96C 29B8 92D0 C862 8501
>Branko Čibej [4096R/A347943F] with fingerprint:
> BA3C 15B1 337C F0FB 222B  D41A 1BCA 6586 A347 943F
>Ivan Zhakov [4096R/F6AD8147] with fingerprint:
> 4829 8F0F E47F 4B8A 43FD  6525 919F 6F61 F6AD 8147
>Johan Corveleyn [4096R/010C8AAD] with fingerprint:
> 8AA2 C10E EAAD 44F9 6972  7AEA B59C E6D6 010C 8AAD
>Philip Martin [2048R/ED1A599C] with fingerprint:
> A844 790F B574 3606 EE95  9207 76D7 88E1 ED1A 599C
>Stefan Fuhrmann [4096R/57921ACC] with fingerprint:
> 056F 8016 D9B8 7B1B DE41  7467 99EC 741B 5792 1ACC
>
> Release notes for the 1.8.x release series may be found at:
>
> http://subversion.apache.org/docs/release-notes/1.8.html
>
> You can find the list of changes between 1.8.8 and earlier versions at:
>
> http://svn.apache.org/repos/asf/subversion/tags/1.8.8/CHANGES
>
> Questions, comments, and bug reports to us...@subversion.apache.org.
>
> Thanks,
> - The Subversion Team


AW: Bug in ra_serf with client certificates

2014-02-21 Thread Markus Schaber
Hi, all,

Von: Thomas Åkesson [mailto:tho...@akesson.cc]
> 
> On 28 jan 2014, at 14:37, Lieven Govaerts  wrote:
> 
> > On Tue, Jan 28, 2014 at 1:53 PM, Branko Čibej  wrote:
> >
> >> [Tue Jan 28 13:32:47 2014] [info] SSL Library Error: 336105671
> >> error:140890C7:SSL routines:SSL3_GET_CLIENT_CERTIFICATE:peer did not
> >> return a certificate No CAs known to server for verification?
> >>
> >>
> >> The bug, as I see it, is that in this case, the command-line client
> >> doesn't ask for different credentials. Shouldn't we be transforming
> >> (or wrapping) SERF_ERROR_AUTHN_FAILED to SVN_ERR_RA_NOT_AUTHORIZED?
> >
> > The command line client doesn't ask for a client certificate, it
> > should be defined correctly in the servers file using:
> > ssl-client-cert-file
> > ssl-client-cert-password
> 
> Sorry, I am late to this party. Just got confused by this statement that
> command line client does not ask.
> 
> svn info https://secure.example.com
> Autentiseringsregion (realm): https://secure.example.com:443 Filnamn för
> klientcertifikat:
> 
> This happened to become Swedish but the last line asks for a filename of
> client cert. This was 1.7.7 that I had on an old test machine.
> 
> Attempting this on 1.8 gives an SSL error as this thread has already stated.

I remember testing this about one year ago. 1.7 did prompt, but not save the 
certificate file (neither path nor content) in the auth store, so it would
prompt again and again on each connection attempt. Thus, setting it in the
configuration was the only sensible way to use client certificates.


Best regards

Markus Schaber

CODESYS® a trademark of 3S-Smart Software Solutions GmbH

Inspiring Automation Solutions

3S-Smart Software Solutions GmbH
Dipl.-Inf. Markus Schaber | Product Development Core Technology
Memminger Str. 151 | 87439 Kempten | Germany
Tel. +49-831-54031-979 | Fax +49-831-54031-50

E-Mail: m.scha...@codesys.com | Web: http://www.codesys.com | CODESYS store: 
http://store.codesys.com
CODESYS forum: http://forum.codesys.com

Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade 
register: Kempten HRB 6186 | Tax ID No.: DE 167014915



RE: Bug in ra_serf with client certificates

2014-02-21 Thread Bert Huijben


> -Original Message-
> From: Thomas Åkesson [mailto:tho...@akesson.cc]
> Sent: vrijdag 21 februari 2014 11:32
> To: Subversion Development
> Cc: Branko Čibej; Lieven Govaerts
> Subject: Re: Bug in ra_serf with client certificates
> 
> 
> On 28 jan 2014, at 14:37, Lieven Govaerts  wrote:
> 
> > On Tue, Jan 28, 2014 at 1:53 PM, Branko Čibej 
> wrote:
> >
> >> [Tue Jan 28 13:32:47 2014] [info] SSL Library Error: 336105671
> >> error:140890C7:SSL routines:SSL3_GET_CLIENT_CERTIFICATE:peer did not
> return
> >> a certificate No CAs known to server for verification?
> >>
> >>
> >> The bug, as I see it, is that in this case, the command-line client doesn't
> >> ask for different credentials. Shouldn't we be transforming (or wrapping)
> >> SERF_ERROR_AUTHN_FAILED to SVN_ERR_RA_NOT_AUTHORIZED?
> >
> > The command line client doesn't ask for a client certificate, it
> > should be defined correctly in the servers file using:
> > ssl-client-cert-file
> > ssl-client-cert-password
> 
> Sorry, I am late to this party. Just got confused by this statement that
> command line client does not ask.
> 
> svn info https://secure.example.com
> Autentiseringsregion (realm): https://secure.example.com:443
> Filnamn för klientcertifikat:
> 
> This happened to become Swedish but the last line asks for a filename of
> client cert. This was 1.7.7 that I had on an old test machine.
> 
> Attempting this on 1.8 gives an SSL error as this thread has already stated.

There was a behavior change in 1.8, where the default was changed to *not ask* 
until it is enabled in the config.

See 
http://subversion.apache.org/docs/release-notes/1.8.html#client-cert-prompt-suppression

I think the reasoning was that there are servers that allow a client 
certificate, but don't require one. In case you would have to use such a server 
but don't have a certificate you would get the question over and over again.

Bert


> 
> 
> Thanks,
> Thomas Å.



AW: Bug in ra_serf with client certificates

2014-02-21 Thread Markus Schaber
Hi,

Von: Bert Huijben [mailto:b...@qqmail.nl]
> From: Thomas Åkesson [mailto:tho...@akesson.cc]
> > >> [Tue Jan 28 13:32:47 2014] [info] SSL Library Error: 336105671
> > >> error:140890C7:SSL routines:SSL3_GET_CLIENT_CERTIFICATE:peer did
> > >> not return
> > >> a certificate No CAs known to server for verification?
> > >>
> > >>
> > >> The bug, as I see it, is that in this case, the command-line client
> > >> doesn't ask for different credentials. Shouldn't we be transforming
> > >> (or wrapping) SERF_ERROR_AUTHN_FAILED to SVN_ERR_RA_NOT_AUTHORIZED?
> > >
> > > The command line client doesn't ask for a client certificate, it
> > > should be defined correctly in the servers file using:
> > > ssl-client-cert-file
> > > ssl-client-cert-password
> >
> > Sorry, I am late to this party. Just got confused by this statement
> > that command line client does not ask.
> >
> > svn info https://secure.example.com
> > Autentiseringsregion (realm): https://secure.example.com:443 Filnamn
> > för klientcertifikat:
> >
> > This happened to become Swedish but the last line asks for a filename
> > of client cert. This was 1.7.7 that I had on an old test machine.
> >
> > Attempting this on 1.8 gives an SSL error as this thread has already stated.
> 
> There was a behavior change in 1.8, where the default was changed to *not
> ask* until it is enabled in the config.
> 
> See http://subversion.apache.org/docs/release-notes/1.8.html#client-cert-
> prompt-suppression
> 
> I think the reasoning was that there are servers that allow a client
> certificate, but don't require one. In case you would have to use such a
> server but don't have a certificate you would get the question over and over
> again.

A better fix for this would be to save in the auth cache that the login without
certificate was successful, so the user won't be asked again until the next 
failure.



Best regards

Markus Schaber

CODESYS® a trademark of 3S-Smart Software Solutions GmbH

Inspiring Automation Solutions

3S-Smart Software Solutions GmbH
Dipl.-Inf. Markus Schaber | Product Development Core Technology
Memminger Str. 151 | 87439 Kempten | Germany
Tel. +49-831-54031-979 | Fax +49-831-54031-50

E-Mail: m.scha...@codesys.com | Web: http://www.codesys.com | CODESYS store: 
http://store.codesys.com
CODESYS forum: http://forum.codesys.com

Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade 
register: Kempten HRB 6186 | Tax ID No.: DE 167014915



Re: Bug in ra_serf with client certificates

2014-02-21 Thread Thomas Åkesson

On 2014-02-21, at 12:58, Bert Huijben wrote:

> 
> 
>> -Original Message-
>> From: Thomas Åkesson [mailto:tho...@akesson.cc]
>> Sent: vrijdag 21 februari 2014 11:32
>> To: Subversion Development
>> Cc: Branko Čibej; Lieven Govaerts
>> Subject: Re: Bug in ra_serf with client certificates
>> 
>> 
>> On 28 jan 2014, at 14:37, Lieven Govaerts  wrote:
>> 
>>> On Tue, Jan 28, 2014 at 1:53 PM, Branko Čibej 
>> wrote:
>>> 
 [Tue Jan 28 13:32:47 2014] [info] SSL Library Error: 336105671
 error:140890C7:SSL routines:SSL3_GET_CLIENT_CERTIFICATE:peer did not
>> return
 a certificate No CAs known to server for verification?
 
 
 The bug, as I see it, is that in this case, the command-line client doesn't
 ask for different credentials. Shouldn't we be transforming (or wrapping)
 SERF_ERROR_AUTHN_FAILED to SVN_ERR_RA_NOT_AUTHORIZED?
>>> 
>>> The command line client doesn't ask for a client certificate, it
>>> should be defined correctly in the servers file using:
>>> ssl-client-cert-file
>>> ssl-client-cert-password
>> 
>> Sorry, I am late to this party. Just got confused by this statement that
>> command line client does not ask.
>> 
>> svn info https://secure.example.com
>> Autentiseringsregion (realm): https://secure.example.com:443
>> Filnamn för klientcertifikat:
>> 
>> This happened to become Swedish but the last line asks for a filename of
>> client cert. This was 1.7.7 that I had on an old test machine.
>> 
>> Attempting this on 1.8 gives an SSL error as this thread has already stated.
> 
> There was a behavior change in 1.8, where the default was changed to *not 
> ask* until it is enabled in the config.
> 
> See 
> http://subversion.apache.org/docs/release-notes/1.8.html#client-cert-prompt-suppression
> 
> I think the reasoning was that there are servers that allow a client 
> certificate, but don't require one. In case you would have to use such a 
> server but don't have a certificate you would get the question over and over 
> again.

Ok, that clarifies Lieven's statement. It applies "since 1.8". 

I don't care whether the client prompts for filename or not, but I find it 
important to provide a good error message (preferably with recommended action) 
and APIs that help Tortoise et al.

Also like, Markus' idea about recording (no-)need for certificate in auth 
cache. 

Cheers,
Thomas Å.

Re: RFE: API for an efficient retrieval of server-side mergeinfo data

2014-02-21 Thread Doug Robinson
Julian:

Given the required RA protocol changes, when could this change be shipped?
 What version of SVN?

Thank you.

Doug


On Wed, Feb 19, 2014 at 10:06 AM, Julian Foad wrote:

> Marc Strapetz wrote:
> > Julian Foad wrote:
> >> It looks like we have an agreement in principle. Would you like to file
> an
> >> enhancement issue?
> >
> > Great. I've filed an issue now:
> >
> > http://subversion.tigris.org/issues/show_bug.cgi?id=4469
> >
> > Would you please review the various attributes (Subcomponent, ...)?
>
> That's great, thanks. I added a reference to this email thread, added
> myself to the CC list, and tweaked the type from 'feature' to 'enhancement'
> (just my personal interpretation) and schedule from '---' to 'unscheduled'
> (which just indicates I've thought about it and am stating that it's not
> currently tied to any particular release, it doesn't mean it has to stay
> that way).
>
> I talked with Brane about this and we discussed how it might make more
> sense to do a higher level API. Instead of asking "what is the absolute
> difference in the mergeinfo representations?" it could ask "What merges and
> other interesting events have occurred in the lifetime of this path?".
> There are a couple of reasons.
>
> The API as sketched so far is pretty straightforward, but even so the
> effort needed to implement it is not trivial. It requires RA protocol
> changes as well as all the layers of API change. The mergeinfo
> representation is subject to change. It feels like a backward step to
> invest effort in adding more support that is tied specifically to the
> current format.
>
> SmartSVN and other front ends like to be able to draw a merge graph. Even
> the 'svn mergeinfo' command-line command now draws a little ASCII-art graph
> showing limited information about the most recent merge. At present they
> all have to interpret mergeinfo themselves, at a pretty low level, and the
> interpretation is subtle and poorly understood. (I don't understand the
> edge cases related to adds and deletes properly, and I've been working with
> it for years.)
>
> So it seems like a good idea to encapsulate the interpretation of
> mergeinfo a bit more, and expose data in a form that is geared specifically
> towards explaining the history in the way that users can understand it.
> Maybe think of it as an extended 'log' operation, adding a small number of
> new notification types such as:
>
>   * there is a full merge into here, bringing in all the new changes
>   from PATH up to REV;
>   * there is a partial merge to here, bringing in some changes
>   from PATH between REV1 and REV2;
>
> What do you think of that sort of interface? Does your code already
> calculate something like that?
>
> - Julian
>
>


-- 
Douglas B. Robinson | *Senior Product Manager*

WANdisco // *Non-Stop Data*

t. 925-396-1125
e. doug.robin...@wandisco.com

-- 
Listed on the London Stock Exchange: 
WAND

THIS MESSAGE AND ANY ATTACHMENTS ARE CONFIDENTIAL, PROPRIETARY, AND MAY BE 
PRIVILEGED.  If this message was misdirected, WANdisco, Inc. and its 
subsidiaries, ("WANdisco") does not waive any confidentiality or privilege. 
 If you are not the intended recipient, please notify us immediately and 
destroy the message without disclosing its contents to anyone.  Any 
distribution, use or copying of this e-mail or the information it contains 
by other than an intended recipient is unauthorized.  The views and 
opinions expressed in this e-mail message are the author's own and may not 
reflect the views and opinions of WANdisco, unless the author is authorized 
by WANdisco to express such views or opinions on its behalf.  All email 
sent to or from this address is subject to electronic storage and review by 
WANdisco.  Although WANdisco operates anti-virus programs, it does not 
accept responsibility for any damage whatsoever caused by viruses being 
passed.



Re: RFE: API for an efficient retrieval of server-side mergeinfo data

2014-02-21 Thread Branko Čibej
On 21.02.2014 15:50, Doug Robinson wrote:
> Julian:
>
> Given the required RA protocol changes, when could this change be
> shipped?  What version of SVN?

We treat a protocol extension the same way as an API extension: new
protocol-level features can only appear in minor version releases (e.g.,
1.9.0 or 1.10.0), and they must be implemented in such a way that they
do not affect older clients.

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. br...@wandisco.com


Re: RFE: API for an efficient retrieval of server-side mergeinfo data

2014-02-21 Thread Julian Foad
Doug Robinson wrote:

> Julian:
> 
> Given the required RA protocol changes, when could this change be
> shipped?  What version of SVN?


Hi Doug. A change like that could be shipped in a 1.x.0 version.

- Julian


> Julian Foad wrote:
>> Marc Strapetz wrote:
>>> Julian Foad wrote:
 It looks like we have an agreement in principle. Would you like to file an
 enhancement issue?
>>>
>>> Great. I've filed an issue now:
>>>
>>> http://subversion.tigris.org/issues/show_bug.cgi?id=4469

[...]

>> I talked with Brane about this and we discussed how it might make more
>> sense to do a higher level API. [...]


svn_rangelist_dup -> ptr_array_dup?

2014-02-21 Thread Julian Foad
Stefan,

In svn.apache.org/r1395434 and svn.apache.org/r1483310, you optimised 
svn_rangelist_dup().

An svn_rangelist_t is an APR array of pointers to small simple objects.

I guessed we would have other places where we want to duplicate an  
array of pointers to simple objects, but I guessed wrong and 
can't find any. Nevertheless, how about we factor out the generic 
array-duplication code like this, in order to maintain a clear separation 
between rangelist code and generic array code? I think that makes it easier for 
a reader to see that there's nothing special happening there that concerns 
rangelists specifically.


I have at the back of my mind that this function could very well be proposed 
for inclusion in APR, but I am not presently thinking of doing so.

Of course, parameterizing the 'object size' like this may make it slightly 
slower. I don't imagine it would be significant but haven't measured it. Does 
that bother you in this case?

[[[
Index: subversion/libsvn_subr/mergeinfo.c
===
--- subversion/libsvn_subr/mergeinfo.c    (revision 1570152)
+++ subversion/libsvn_subr/mergeinfo.c    (working copy)
@@ -2237,35 +2237,48 @@ svn_mergeinfo__add_suffix_to_mergeinfo(s
 rangelist);
 }

   return SVN_NO_ERROR;
 }

-svn_rangelist_t *
-svn_rangelist_dup(const svn_rangelist_t *rangelist, apr_pool_t *pool)
+/* Deep-copy an array of pointers to simple objects.
+ *
+ * Return a duplicate in POOL of the array ARRAY of pointers to objects
+ * of size OBJECT_SIZE bytes. Duplicate each object bytewise.
+ */
+static apr_array_header_t *
+ptr_array_dup(const apr_array_header_t *array,
+  size_t object_size,
+  apr_pool_t *pool)
 {
-  svn_rangelist_t *new_rl = apr_array_make(pool, rangelist->nelts,
-   sizeof(svn_merge_range_t *));
+  apr_array_header_t *new_array = apr_array_make(pool, array->nelts,
+ sizeof(void *));

   /* allocate target range buffer with a single operation */
-  svn_merge_range_t *copy = apr_palloc(pool, sizeof(*copy) * rangelist->nelts);
+  char *copy = apr_palloc(pool, object_size * array->nelts);

   /* for efficiency, directly address source and target reference buffers */
-  svn_merge_range_t **source = (svn_merge_range_t **)(rangelist->elts);
-  svn_merge_range_t **target = (svn_merge_range_t **)(new_rl->elts);
+  void **source = (void **)(array->elts);
+  void **target = (void **)(new_array->elts);
   int i;

   /* copy ranges iteratively and link them into the target range list */
-  for (i = 0; i < rangelist->nelts; i++)
+  for (i = 0; i < array->nelts; i++)
 {
-  copy[i] = *source[i];
-  target[i] = ©[i];
+  target[i] = ©[i * object_size];
+  memcpy(target[i], source[i], object_size);
 }
-  new_rl->nelts = rangelist->nelts;
+  new_array->nelts = array->nelts;
+
+  return new_array;
+}

-  return new_rl;
+svn_rangelist_t *
+svn_rangelist_dup(const svn_rangelist_t *rangelist, apr_pool_t *pool)
+{
+  return ptr_array_dup(rangelist, sizeof(svn_merge_range_t), pool);
 }

 svn_merge_range_t *
 svn_merge_range_dup(const svn_merge_range_t *range, apr_pool_t *pool)
 {
   svn_merge_range_t *new_range = apr_palloc(pool, sizeof(*new_range));
]]]

- Julian



Re: svn_ra_get_file_revs2 vs. blame vs. FS API

2014-02-21 Thread Julian Foad
Stefan Fuhrmann wrote:
> Julian Foad wrote:
>> Stefan Fuhrmann wrote:
>>> r1568600 uncovered an inconsistency in our API usage / interpretation
>>> making blame -g tests fail for FSX.
>>>
>>> The starting point is svn_fs_contents_changed and svn_fs_props_changed.
>>> FSFS and probably BDB implement those as "has there been an intermittent
>>> change?" E.g. if the result of some merge yields equal content on both 
>>> sides,
>>> they will still be considered "changed". That is a violation of the API 
>>> definition
>>> basically asking "is the delta empty?". Even the FSX code is not fully 
>>> consistent
>>> with that, yet.
>> 
>> BDB and FSFS and FSX all implement the comparison in exactly the same way:
>> "does it have the same rep-key?".
> 
> Well, not exactly. FSFS adds a "unifier" that tells different
> instances of a shared representation apart (but uses the
> same unifier when the rep does not change between
> successive noderevs).

Isn't libsvn_fs_base/fs.h:node_revision_t.data_key_uniquifier exactly the same 
thing in the BDB code? See the comparison function, 
svn_fs_base__things_different().

>> This does not mean "is the content identical?", but it does not necessarily
>> mean "has there been an intervening change to the content?" either.
>
> Well for FSFS, it does the latter.

It's not clear to me that it does exactly the latter, that it says "changed" if 
and only if there has been an intervening change. In my next paragraph I 
speculated about a situation where that might not be the case. It seems to me 
that there may be cases where the rep-key can change even when the content does 
not change. Perhaps that is not in fact possible, but I can't see any clear 
reason why not.

>> It only means "was the same representation instance chosen to represent this
>> content?". For example, I think it might be possible for this to return
>> "different" even if there has been no intervening change to the content, if
>> the rep cache was disabled and re-enabled during a series of commits, 
>> although
>> that depends on even more implementation details.
>> 
>> One thing we can say for certain is that, despite the doc string, these APIs
>> have always been implemented to return "the same" in some cases when the
>> content is known to be the same, and return "different" in cases where the
>> content might be different but might not be.
>> 
>> Some of the callers use the call to optimize whether to fetch and compare the
>> content. That's fine of course.
>> 
>> Some other callers expose this behaviour to higher level APIs, where the 
>> caller
>> might be expecting a definite answer to "Does the content differ?", and 
>> that's
>> where the problems begin.
>>
>> Both semantics are valid, of course, so we might end up wanting to provide 
>> both.
>
> I agree. We might simply a allow_false_positives flag in
> svn_fs_*_changed2. [...]

It's not just a question of whether the function may report some non-changes 
along with the definite changes. The current blame code relies on a *specific* 
pattern of positive outputs which have a specific meaning which happens to be 
different from what the doc string says. In that sense, the outputs are not 
"false" at all. It would be fair to say that it relies on a completely 
different API whose output is a superset of all content changes but does not 
contain any "random" false positives.


>>> svn_ra_get_file_revs2 uses svn_fs_contents_changed to decide whether
>>> a delta should be sent to the given handler. The false positives returned by
>>> FSFS and BDB are benign since we simply end up sending an empty delta
>>> (a mere waste of CPU cycles).
>> 
>> And note that the doc string for svn_ra_get_file_revs2() explicitly says
>> these false positives can be returned.
> 
> The point is that the blame implementation relies on those
> false positives. I'd prefer to make false positives a rare instance.

Right.

>>> However, our blame implementation relies on those false positives because
>>> it tracks contents from revisions to be merged later separately from 
>>> "mainline"
>>> changes. When the merge finally happens but results in the same contents
>>> as the previous branch revision, the delta handler must be called to pull in
>>> the blame info from the branch. The latter no longer happens with the strict
>>> interpretation of svn_fs_contents_changed.
>> 
>> Without following the details, I think what you mean about merges is that the
>> blame algorithm is trying to report an event whenever a merge happened, even
>> if that merge did not result in a content change. The gist of it seems to be
>> that the blame implementation assumes:
>>
>>  * that a new content rep-key is assigned whenever a merge is committed, even
>>  if the content is unchanged, but not at other times when the content is
>>  unchanged; and
> 
> Yep, that is the implementation detail leak that I'm concerned about.
> 
>>  * that svn_ra_get_file_revs2() will notify if and only if a new rep-key is
>>  ass

Re: svn commit: r1570434 - /subversion/trunk/subversion/svnserve/cyrus_auth.c

2014-02-21 Thread Ben Reser
On 2/21/14, 1:43 AM, Stefan Sperling wrote:
> Nice fix. Can we please write 'unsigned int'? I know there is a
> default size, which is probably int, but I already have to remember
> so many other idiosyncrasies about C...

Done in r1570648. I used just unsigned since I just used what Cyrus SASL had in
their API.



Re: 1.7.16 up for testing/signing

2014-02-21 Thread Johan Corveleyn
On Wed, Feb 19, 2014 at 2:02 AM, Ben Reser  wrote:
> The 1.7.16 release artifacts are now available for testing/signing.
> Please get the tarballs from
>   https://dist.apache.org/repos/dist/dev/subversion
> and add your signatures there.  I plan to try and release on February
> 26th so please try and get your votes/signatures in place by February 24th.

Summary
---
+1 to release

Platform

Windows XP (32 bit) SP3
Microsoft Visual Studio 2010 SP1

Verified

Signature and sha1 for subversion-1.7.16.zip

Contents of subversion-1.7.16.zip are identical to tags/1.7.16, and to
branches/1.7.x@1569520 (except for expected differences in svn_version.h).

Tested
--
[ Release build ] x [ fsfs ] x [ file | svn | neon | serf ]

Results
---
All tests pass.

For serf I disabled my local AVG Surf-Shield to make it work
(issue #4175 - ra_serf crashes on Windows with AVG 2012 Surf-Shield
 http://subversion.tigris.org/issues/show_bug.cgi?id=4175)

Dependencies

Httpd 2.4.4
Apr 1.4.6
Apr-Util 1.5.2
Apr-Iconv 1.2.1
Neon 0.29.6
OpenSSL 1.0.1e
Serf 1.2.1
SQLite 3.7.17
ZLib 1.2.8 (without assembly optimizations)

Signature
-

subversion-1.7.16.zip:
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (MingW32)

iQIcBAABCgAGBQJTBnCpAAoJELWc5tYBDIqt00IP/jzWmMdOa5VUw6PeO168S3sS
DIFRrE0C2iADADOS7qrYJMoAKrNYv6hKsIrvkMFTIUaPrE7JFMRx5Lc9XR0zup33
nyPp4mRGrwjFXTrP+AiGlisPxle5Q7E5X8DsnL08ST6i3zqW1iADwkJEiQ9FDxN/
LGek4aO4NJA95086zlJ5TVv4P+4mZUBDjErRR7hAY0sl7AIW/GMbr+h9SYm08ZA0
GCqXkGbMpAhRnCUk7PaKBHcRMjRTFyH2NVRBhVQKZewwzO7VsbjosVDxUg4ZXeIk
rfHXbeOVn8tVGE7s2dGEskYhrTRzL30rJqQCCfUUsj5XtLBVGzu3S1pEbBY+3uox
b7hXZ4fq5bDBRRh1vGpSWD2FaY+/UrtQVFjb/boYxyPzpz46JiJj8omhYySnDhow
hlwocq6FdPeTtf2UQCd/Yu5YsumITzummPueTHZFC+7fubbXnatB/XHEyb2nsvql
XmuixwyE79eb27i1ZZho3rV4yKfBOvXaJaCgpzq+bv8iryAqDpCdxEcBFK0u9ZUA
I+YYWUeIEaA4HH2d2KOINPpnDQgN2zCj/95vWn8axH6eKMpfZkx/YMLeJnbaLzQv
h1gp7V+weRQdMkRPA1HsXfG+imZZh4NV5aTm1IgXrAETRlpmSm/n2xcCxK7Ld/Do
ezKnWFhqLnbwGnIK0X1Y
=GmBv
-END PGP SIGNATURE-


-- 
Johan