Re: svn commit: r1837159 - /subversion/site/publish/docs/release-notes/1.11.html
Daniel Shahaf wrote: >Add the "work in progress" notice (r1780199)? Yes please! (Help appreciated -- I'm around only a few hours in the next 10 days.) - Julian
Re: svn commit: r1837159 - /subversion/site/publish/docs/release-notes/1.11.html
julianf...@apache.org wrote on Tue, 31 Jul 2018 15:31 +: > Author: julianfoad > Date: Tue Jul 31 15:31:37 2018 > New Revision: 1837159 > > URL: http://svn.apache.org/viewvc?rev=1837159=rev > Log: > * publish/docs/release-notes/1.11.html: New file. Add the "work in progress" notice (r1780199)?
Re: Bug report: Regression SVN Client, SSL, Serf 1.3.9-3, SSLVerifyClient require
Daniel Shahaf writes: > Subversion uses Serf, which uses OpenSSL, which talks to an SSL implementation > on the server. The root cause of the error is known to the SSL implementation > on the server (that's why you see it in the error log). It's not obvious that > OpenSSL on the client side even knows what the root cause is. In this case the client knows exactly what is wrong, it's the one closing the connection because: 140258270184704:error:140AB18E:SSL routines:SSL_CTX_use_certificate:ca md too weak:../ssl/ssl_rsa.c:303: Could we get our client to show that error? We would need a new serf API to marshal the error message back to Subversion. -- Philip
Re: Bug report: Regression SVN Client, SSL, Serf 1.3.9-3, SSLVerifyClient require
Folker Schamel wrote on Tue, Jul 31, 2018 at 17:42:10 +0200: > On 2018-07-31 17:04, Philip Martin wrote: > > Folker Schamel writes: > > > For the broken setup, the client reports: > > > svn: E120171: Error running context: An error occurred during SSL > > > communication > > > And the server Apache log reports: > > > ssl_engine_io.c(1308): (70014)End of file found: [client x:x] > > > AH02007: SSL handshake interrupted by system [Hint: Stop button > > > pressed in browser?!] > > Maybe a hint in the svn release notes could be useful, since the svn error > messages are not very useful. Subversion uses Serf, which uses OpenSSL, which talks to an SSL implementation on the server. The root cause of the error is known to the SSL implementation on the server (that's why you see it in the error log). It's not obvious that OpenSSL on the client side even knows what the root cause is. It could be that only the server-side SSL implementation knows the root cause; it could be that the client-side SSL implementation also knows the root cause, but the ball of relaying that information up the stack (openssl->serf->libsvn->stderr) was dropped. The error code in question (E120171, SERF_ERROR_SSL_COMM_FAILED) does appear to be somewhat of a catchall, i.e., a code to which several openssl errors are mapped; but nevertheless, I wouldn't be surprised if the openssl client-side error message were less detailed than the server-side one. For what it's worth, the only part of the quoted error message that Subversion controls is the text "Error running context". The remainder, both the number prefixed andthe error suffixed, is generated by the serf library, based on an error returned by the openssl library. That said, I do agree that "Error running context" isn't the best phrasing. "Context", here, is the name of an internal API. Something like "HTTP request failed" would be better, wouldn't it? > Thanks a lot! > And sorry for bothering on the dev list. I should have posted to user. > Also thanks for your other SNI and DEFLATE tips! Cheers, Daniel
Re: Bug report: Regression SVN Client, SSL, Serf 1.3.9-3, SSLVerifyClient require
Hi Philip, this solved it! Using "openssl s_client" as you described it reported: error setting certificate 140258270184704:error:140AB18E:SSL routines:SSL_CTX_use_certificate:ca md too weak:../ssl/ssl_rsa.c:303: So we created new certificates with sha256 (default in openssl 1.1) instead of md5. This solved the problem. Maybe a hint in the svn release notes could be useful, since the svn error messages are not very useful. Thanks a lot! And sorry for bothering on the dev list. I should have posted to user. Also thanks for your other SNI and DEFLATE tips! Cheers, Folker On 2018-07-31 17:04, Philip Martin wrote: Folker Schamel writes: After upgrading, Subversion SSL connections with "SSLVerifyClient require" seem to be broken. Broken: SVN Client 1.9.5, Serf 1.3.9-3, Server "SSLVerifyClient require" Works: SVN Client 1.9.5, Serf 1.3.9-3, Server "SSLVerifyClient off" Works: SVN Client 1.9.5, Serf 1.3.8-1, Server "SSLVerifyClient require" I can use client certificates on my Debian machine using 1.3.9-3 with "SSLVerifyClient require" so it does work in some configurations. I think that one of the changes between 1.3.8-1 and 1.3.9-3 on Debian is that openssl was upgraded from 1.0 to 1.1. That's a major upgrade and some sort of openssl incompatibility may be the problem. You could use the openssl binary to debug the ssl connection. First you need to convert Subversion's client certificate from pkcs12 to pem: openssl pkcs12 -in path/to/svn/cert.p12 -out cert.pem then you can use: openssl s_client -connect example.com:443 -servername example.com -cert cert.pem If you are using ssl-authority-files in .subversion/servers to verify the server cert you can get s_client to do the same with the additional parameter: openssl s_client ... -CAfile path/to/authority.pem The s_client output may indicate what problem is occurring. For the broken setup, the client reports: svn: E120171: Error running context: An error occurred during SSL communication And the server Apache log reports: ssl_engine_io.c(1308): (70014)End of file found: [client x:x] AH02007: SSL handshake interrupted by system [Hint: Stop button pressed in browser?!] Using the latest TortoiseSVN client reports the same problem, presumably the same cause. TortoiseSVN is probably built with openssl 1.1 as well. [Tue Jul 31 15:30:43.886183 2018] [ssl:debug] [pid x:tid x] ssl_engine_kernel.c(2122): [client x:x] AH02044: No matching SSL virtual host for servername x found (using default/first virtual host) That looks like SNI isn't working but I don't know if that is relevant for your problem. Some sort of vhost/servername problem in the apache config? SetOutputFilter DEFLATE SetInputFilter DEFLATE Header append Vary User-Agent env=!dont-vary DEFLATE combined with mod_dav_svn had problems in the past but I think they are all fixed. Note that when mod_dav_svn and Subversion clients communicate they do so using compressed deltas, so adding DEFLATE doesn't result in very much additional compression. The combination has a more significant compression effect if users are using non-Subversion tools: web browsers, curl, etc. This is probably not relevant to your problem.
Re: Bug report: Regression SVN Client, SSL, Serf 1.3.9-3, SSLVerifyClient require
Folker Schamel writes: > After upgrading, Subversion SSL connections with "SSLVerifyClient > require" seem to be broken. > > Broken: SVN Client 1.9.5, Serf 1.3.9-3, Server "SSLVerifyClient require" > Works: SVN Client 1.9.5, Serf 1.3.9-3, Server "SSLVerifyClient off" > Works: SVN Client 1.9.5, Serf 1.3.8-1, Server "SSLVerifyClient require" I can use client certificates on my Debian machine using 1.3.9-3 with "SSLVerifyClient require" so it does work in some configurations. I think that one of the changes between 1.3.8-1 and 1.3.9-3 on Debian is that openssl was upgraded from 1.0 to 1.1. That's a major upgrade and some sort of openssl incompatibility may be the problem. You could use the openssl binary to debug the ssl connection. First you need to convert Subversion's client certificate from pkcs12 to pem: openssl pkcs12 -in path/to/svn/cert.p12 -out cert.pem then you can use: openssl s_client -connect example.com:443 -servername example.com -cert cert.pem If you are using ssl-authority-files in .subversion/servers to verify the server cert you can get s_client to do the same with the additional parameter: openssl s_client ... -CAfile path/to/authority.pem The s_client output may indicate what problem is occurring. > For the broken setup, the client reports: > svn: E120171: Error running context: An error occurred during SSL > communication > And the server Apache log reports: > ssl_engine_io.c(1308): (70014)End of file found: [client x:x] > AH02007: SSL handshake interrupted by system [Hint: Stop button > pressed in browser?!] > > Using the latest TortoiseSVN client reports the same problem, > presumably the same cause. TortoiseSVN is probably built with openssl 1.1 as well. > [Tue Jul 31 15:30:43.886183 2018] [ssl:debug] [pid x:tid x] > ssl_engine_kernel.c(2122): [client x:x] AH02044: No matching > SSL virtual host for servername x found (using default/first > virtual host) That looks like SNI isn't working but I don't know if that is relevant for your problem. Some sort of vhost/servername problem in the apache config? > > SetOutputFilter DEFLATE > SetInputFilter DEFLATE > Header append Vary User-Agent env=!dont-vary > DEFLATE combined with mod_dav_svn had problems in the past but I think they are all fixed. Note that when mod_dav_svn and Subversion clients communicate they do so using compressed deltas, so adding DEFLATE doesn't result in very much additional compression. The combination has a more significant compression effect if users are using non-Subversion tools: web browsers, curl, etc. This is probably not relevant to your problem. -- Philip
Re: Issue SVN-4767: svnadmin dump shouldn't canonicalize svn:date
Branko Čibej wrote: > You've changed my mind: +1 OK, thanks for the review. Committed, with a test: http://svn.apache.org/r1837151 -- - Julian
Bug report: Regression SVN Client, SSL, Serf 1.3.9-3, SSLVerifyClient require
Hello everyone, After upgrading, Subversion SSL connections with "SSLVerifyClient require" seem to be broken. Broken: SVN Client 1.9.5, Serf 1.3.9-3, Server "SSLVerifyClient require" Works: SVN Client 1.9.5, Serf 1.3.9-3, Server "SSLVerifyClient off" Works: SVN Client 1.9.5, Serf 1.3.8-1, Server "SSLVerifyClient require" For the broken setup, the client reports: svn: E120171: Error running context: An error occurred during SSL communication And the server Apache log reports: ssl_engine_io.c(1308): (70014)End of file found: [client x:x] AH02007: SSL handshake interrupted by system [Hint: Stop button pressed in browser?!] Using the latest TortoiseSVN client reports the same problem, presumably the same cause. Additional details below. Can I help with additional information? Btw, thanks a lot to all Subversion developers and contributors for the awesome work!!! Cheers, Folker * Client-side recipt (latest Debian stretch): root@x:/# apt-get install libserf-1-1=1.3.8-1 . root@x:/# svn --version svn, version 1.9.5 (r1770682) compiled Jun 30 2018, 13:44:22 on x86_64-pc-linux-gnu Copyright (C) 2016 The Apache Software Foundation. This software consists of contributions made by many people; see the NOTICE file for more information. Subversion is open source software, see http://subversion.apache.org/ The following repository access (RA) modules are available: * ra_svn : Module for accessing a repository using the svn network protocol. - with Cyrus SASL authentication - handles 'svn' scheme * ra_local : Module for accessing a repository on local disk. - handles 'file' scheme * ra_serf : Module for accessing a repository via WebDAV protocol using serf. - using serf 1.3.8 (compiled with 1.3.9) - handles 'http' scheme - handles 'https' scheme The following authentication credential caches are available: * Plaintext cache in /root/.subversion * Gnome Keyring * GPG-Agent * KWallet (KDE) root@x:/# svn update Updating '.': At revision 828. root@x:/# apt-get install libserf-1-1=1.3.9-3 . root@x:/# svn --version svn, version 1.9.5 (r1770682) compiled Jun 30 2018, 13:44:22 on x86_64-pc-linux-gnu Copyright (C) 2016 The Apache Software Foundation. This software consists of contributions made by many people; see the NOTICE file for more information. Subversion is open source software, see http://subversion.apache.org/ The following repository access (RA) modules are available: * ra_svn : Module for accessing a repository using the svn network protocol. - with Cyrus SASL authentication - handles 'svn' scheme * ra_local : Module for accessing a repository on local disk. - handles 'file' scheme * ra_serf : Module for accessing a repository via WebDAV protocol using serf. - using serf 1.3.9 (compiled with 1.3.9) - handles 'http' scheme - handles 'https' scheme The following authentication credential caches are available: * Plaintext cache in /root/.subversion * Gnome Keyring * GPG-Agent * KWallet (KDE) root@x:/# svn update Updating '.': svn: E170013: Unable to connect to a repository at URL 'https://x/x/x' svn: E120171: Error running context: An error occurred during SSL communication root@x:/# * Client-side recipt continuation after SSLVerifyClient require -> off root@x:/# svn update Updating '.': At revision 828. root@x:/# * Server-side ssl-error.log: ... [Tue Jul 31 15:30:43.885515 2018] [ssl:info] [pid x:tid x] [client x:x] AH01964: Connection to child 68 established (server localhost:443) [Tue Jul 31 15:30:43.885795 2018] [ssl:trace2] [pid x:tid x] ssl_engine_rand.c(126): Seeding PRNG with 656 bytes of entropy [Tue Jul 31 15:30:43.885983 2018] [ssl:trace3] [pid x:tid x] ssl_engine_kernel.c(1989): [client x:x] OpenSSL: Handshake: start [Tue Jul 31 15:30:43.886064 2018] [ssl:trace3] [pid x:tid x] ssl_engine_kernel.c(1998): [client x:x] OpenSSL: Loop: before/accept initialization [Tue Jul 31 15:30:43.886114 2018] [ssl:trace4] [pid x:tid x] ssl_engine_io.c(2135): [client x:x] OpenSSL: read 5/5 bytes from BIO#7fcef0001580 [mem: 7fcef0006dc3] (BIO dump follows) [Tue Jul 31 15:30:43.886134 2018] [ssl:trace4] [pid x:tid x] ssl_engine_io.c(2135): [client x:x] OpenSSL: read 191/191 bytes from BIO#7fcef0001580 [mem: 7fcef0006dc8] (BIO dump follows) [Tue Jul 31 15:30:43.886183 2018] [ssl:debug] [pid x:tid x] ssl_engine_kernel.c(2122): [client x:x] AH02044: No matching SSL virtual host for servername x found (using default/first virtual host) [Tue Jul 31 15:30:43.886258 2018] [ssl:trace3] [pid x:tid x] ssl_engine_kernel.c(1998): [client x:x] OpenSSL: Loop: unknown state [Tue Jul 31 15:30:43.886294 2018] [ssl:trace3] [pid x:tid x] ssl_engine_kernel.c(1998): [client x:x] OpenSSL: Loop: unknown state [Tue Jul 31 15:30:43.886419 2018]
Re: Issue SVN-4767: svnadmin dump shouldn't canonicalize svn:date
On 31.07.2018 15:14, Julian Foad wrote: > Before r871212 there was no validation of svn:date value entering the repo, I > think, so old-format dates could have been put in. Right. That's the edge-case I was thinking about. > But if anyone put old-format dates into their repo, and even if users do > currently have some tools that expect the conversion to happen, I still don't > see that as a good enough reason why future 'svnadmin dump' should not read > out exactly what's in there. > > It also happens to be a data bug that's extremely easy to fix -- just enable > revprop changes and script the propedits. > >> 2. Our dumpfile format is documented. With your proposed change, there >> is a slight chance that "svnadmin dump" could produce invalid dump >> files. > Invalid how? I don't see the contents of svn:date documented in > 'notes/dump-load-format.txt'. Ha, indeed, that document doesn't say anything concrete about the values of reserved properties. Anyway, I was just trying to pedantically think my way through the possible edge cases. In retrospect, it's a pity we didn't make the 0.14->1.0 dump/load cycle work the way you're proposing now. Also, you make a good point about scripting the fix before the dump; these are revision properties, after all. You've changed my mind: +1 -- Brane
Re: Issue SVN-4767: svnadmin dump shouldn't canonicalize svn:date
Branko Čibej wrote on 2018-07-31: > On 31.07.2018 13:04, Julian Foad wrote: > > https://issues.apache.org/jira/browse/SVN-4767 > > > > Running tests with the '--dump-load-cross-check' option revealed a minor > > discrepancy between the dump files produced by 'svnadmin dump' and > > 'svnrdump dump' in some test cases. > > - 2015-01-01T00:00:00.00Z > > + 2015-01-01T00:00:00.0Z I wasn't clear but the repository contains ".0Z" in the revprop value, explicitly set by a "propset"; the ".00Z" line is produced by "svnadmin dump" after going through from_cstring() and to_cstring(); and the ".0Z" line is produced by "svnrdump dump" which prints the property value sent by the server (after normalizing EOLs). > > "svnadmin dump" "canonicalizes" each svn:date revprop while dumping, in the > > function write_revision_record(). > > > > This seems to have been done in r842390 in order to upgrade from pre-0.14 > > repository format to the new timestamp format introduced in 0.14 -- see > > issue SVN-614 "DAV:creationdate needs to be an ISO8601 date". > > svn_time_from_cstring() reads either new or old format, and then > > svn_time_to_cstring() writes the new format. > > There's another possible interpretation here: that the millisecond field > should be 6 digits wide, but for some reason svn_time_to_cstring doesn't > format it that way when the value is zero. [...] That's not what's happening. > > [...] such a transformation should have been done during "svnadmin load" > > [...] > > Agreed, this fits with what "svnadmin load" already does with the > --normalize-props option. But if the timestamp is *not* in the right > format, "load" should fix it. Maybe "except with > --bypass-prop-validation"? [...] > > In any case I think the logic should be changed a bit: Only reformat the > svn:date property if it is not already in ISO8601 format; otherwise, > leave it alone. I'm not yet sure what "load" should do. > > While "svnadmin dump" makes this transformation, "svnrdump dump" and > > "svndumpfilter" do not. This could lead to unintended differences in dump > > output depending on which tool is used. > > "svnadmin dump" should always define the canonical behaviour. Both > "svnrdump" and "svndumpfilter" are newer; they should behave the way > "svnadmin dump" does, not the other way around. One _could_ argue that > this is an omission in svnrdump. In general I agree that's the right way round, but in this case I think the overriding decision is that this is a bug in "svnadmin dump" that the other utilities happen to have not copied. > > Therefore I will remove this code path. It even has a comment saying "### > > Remove this when it is no longer needed for sure" which referred to being > > needed for pre-0.14 upgrades. We definitely no longer need that. > > +0, and I'll explain why: > > 1. This is not a trivial change and should be documented in 1.11 > release notes (hence, it stays on trunk until then). Makes sense. > It's possible > that some 3rd-party tools rely on "svnadmin dump" producing > timestamps in the ISO format. There are two kinds of change in question: changing old format to new (ISO) format, and canonicalizing details (such as number of decimal places) in an ISO format date. The only case where dump would stop producing ISO format is where the repository contains old format dates. That can't happen "naturally" because a dump-load was required between v0.14 and v1.0 (and the dump was programmed to convert the format). It could only happen if users have loaded those old-format dates in again. Before r871212 there was no validation of svn:date value entering the repo, I think, so old-format dates could have been put in. But if anyone put old-format dates into their repo, and even if users do currently have some tools that expect the conversion to happen, I still don't see that as a good enough reason why future 'svnadmin dump' should not read out exactly what's in there. It also happens to be a data bug that's extremely easy to fix -- just enable revprop changes and script the propedits. > 2. Our dumpfile format is documented. With your proposed change, there > is a slight chance that "svnadmin dump" could produce invalid dump > files. Invalid how? I don't see the contents of svn:date documented in 'notes/dump-load-format.txt'. > This is in effect a corollary of point 1., above. Note that > "svndrump dump" is already "infected" by this potential bug, whereas > "svndumpfilter" is not because it starts with a presumably valid > dumpfile. -- - Julian
Re: Issue SVN-4767: svnadmin dump shouldn't canonicalize svn:date
Branko Čibej wrote on Tue, Jul 31, 2018 at 13:43:57 +0200:
> On 31.07.2018 13:04, Julian Foad wrote:
> > However, this does not only convert old to new format, but could also make
> > textual changes to the string if the revprop value is not already
> > canonical. Dump should carefully output exactly what is in the repository
> > and not gratuitously change it. In retrospect, such a transformation should
> > have been done during "svnadmin load" instead of in "dump".
>
> Agreed, this fits with what "svnadmin load" already does with the
> --normalize-props option. But if the timestamp is *not* in the right
> format, "load" should fix it. Maybe "except with
> --bypass-prop-validation"? Another option that controls "load" behaviour
> WRT dates is --ignore-dates, though I can't remember why we introduced that.
>
Presumably in order not to break the binary search algorithm underlying the
@{DATE} peg revision syntax.
Re: Issue SVN-4767: svnadmin dump shouldn't canonicalize svn:date
On 31.07.2018 13:04, Julian Foad wrote: > https://issues.apache.org/jira/browse/SVN-4767 > > Running tests with the '--dump-load-cross-check' option revealed a minor > discrepancy between the dump files produced by 'svnadmin dump' and 'svnrdump > dump' in some test cases. > > [...] > K > svn:date > V > - 2015-01-01T00:00:00.00Z > + 2015-01-01T00:00:00.0Z > [...] > > "svnadmin dump" "canonicalizes" each svn:date revprop while dumping, in the > function write_revision_record(). > > This seems to have been done in r842390 in order to upgrade from pre-0.14 > repository format to the new timestamp format introduced in 0.14 -- see issue > SVN-614 "DAV:creationdate needs to be an ISO8601 date". > svn_time_from_cstring() reads either new or old format, and then > svn_time_to_cstring() writes the new format. There's another possible interpretation here: that the millisecond field should be 6 digits wide, but for some reason svn_time_to_cstring doesn't format it that way when the value is zero. I don't have the actual standard, so I can't check what it says ... and, of course, in the case of 0 there is no semantic difference. > However, this does not only convert old to new format, but could also make > textual changes to the string if the revprop value is not already canonical. > Dump should carefully output exactly what is in the repository and not > gratuitously change it. In retrospect, such a transformation should have been > done during "svnadmin load" instead of in "dump". Agreed, this fits with what "svnadmin load" already does with the --normalize-props option. But if the timestamp is *not* in the right format, "load" should fix it. Maybe "except with --bypass-prop-validation"? Another option that controls "load" behaviour WRT dates is --ignore-dates, though I can't remember why we introduced that. In any case I think the logic should be changed a bit: Only reformat the svn:date property if it is not already in ISO8601 format; otherwise, leave it alone. > While "svnadmin dump" makes this transformation, "svnrdump dump" and > "svndumpfilter" do not. This could lead to unintended differences in dump > output depending on which tool is used. "svnadmin dump" should always define the canonical behaviour. Both "svnrdump" and "svndumpfilter" are newer; they should behave the way "svnadmin dump" does, not the other way around. One _could_ argue that this is an omission in svnrdump. > Therefore I will remove this code path. It even has a comment saying "### > Remove this when it is no longer needed for sure" which referred to being > needed for pre-0.14 upgrades. We definitely no longer need that. +0, and I'll explain why: 1. This is not a trivial change and should be documented in 1.11 release notes (hence, it stays on trunk until then). It's possible that some 3rd-party tools rely on "svnadmin dump" producing timestamps in the ISO format. 2. Our dumpfile format is documented. With your proposed change, there is a slight chance that "svnadmin dump" could produce invalid dump files. This is in effect a corollary of point 1., above. Note that "svndrump dump" is already "infected" by this potential bug, whereas "svndumpfilter" is not because it starts with a presumably valid dumpfile. -- Brane
Re: Issue SVN-4767: svnadmin dump shouldn't canonicalize svn:date
Julian Foad wrote on Tue, Jul 31, 2018 at 12:04:23 +0100: > https://issues.apache.org/jira/browse/SVN-4767 > > Running tests with the '--dump-load-cross-check' option revealed a minor > discrepancy between the dump files produced by 'svnadmin dump' and 'svnrdump > dump' in some test cases. > > [...] > K > svn:date > V > - 2015-01-01T00:00:00.00Z > + 2015-01-01T00:00:00.0Z > [...] > > "svnadmin dump" "canonicalizes" each svn:date revprop while dumping, in the > function write_revision_record(). > > This seems to have been done in r842390 in order to upgrade from pre-0.14 > repository format to the new timestamp format introduced in 0.14 -- see issue > SVN-614 "DAV:creationdate needs to be an ISO8601 date". > svn_time_from_cstring() reads either new or old format, and then > svn_time_to_cstring() writes the new format. > One clarification: Both "2015-01-01T00:00:00.00Z" and "2015-01-01T00:00:00.0Z" are "new format" timestamps. (I assume they are handled identically, but perhaps some piece of code somewhere malfunctions when the number of decimal places is != 6.) The "old format" which that issue and comment refer to is this one: [[[ /* Our old timestamp strings looked like this: * *"Tue 3 Oct 2000 HH:MM:SS.UUU (day 277, dst 1, gmt_off -18000)" * * The idea is that they are conventionally human-readable for the * first part, and then in parentheses comes everything else required * to completely fill in an apr_time_exp_t: tm_yday, tm_isdst, * and tm_gmtoff. * * This format is still recognized on input, for backward * compatibility, but no longer generated. */ static const char * const old_timestamp_format = "%s %d %s %d %02d:%02d:%02d.%06d (day %03d, dst %d, gmt_off %06d)"; ]]] That string constant is still present in HEAD, but it's been converted to a macro named OLD_TIMESTAMP_FORMAT. > However, this does not only convert old to new format, but could also make > textual changes to the string if the revprop value is not already canonical. > Dump should carefully output exactly what is in the repository and not > gratuitously change it. In retrospect, such a transformation should have been > done during "svnadmin load" instead of in "dump". > > While "svnadmin dump" makes this transformation, "svnrdump dump" and > "svndumpfilter" do not. This could lead to unintended differences in dump > output depending on which tool is used. (I made some progress in unifying the > output logic for those three dump producers a couple of years ago, but I left > this part alone because I did not know what to do with it.) > > The discrepancy I noticed above comes from prop_tests.py 41 and 42 which > explicitly propset svn:date to a value such as '2015-01-01T00:00:00.0Z'. > "svnadmin dump" was munging this non-canonical value on output, which meant > it was not a true dump. (Whether a non-canonical format should have been > allowed into the repository in the first place is a separate issue.) > > Therefore I will remove this code path. It even has a comment saying "### > Remove this when it is no longer needed for sure" which referred to being > needed for pre-0.14 upgrades. We definitely no longer need that. +1 to having dump write the data verbatim. Cheers, Daniel
Issue SVN-4767: svnadmin dump shouldn't canonicalize svn:date
https://issues.apache.org/jira/browse/SVN-4767 Running tests with the '--dump-load-cross-check' option revealed a minor discrepancy between the dump files produced by 'svnadmin dump' and 'svnrdump dump' in some test cases. [...] K svn:date V - 2015-01-01T00:00:00.00Z + 2015-01-01T00:00:00.0Z [...] "svnadmin dump" "canonicalizes" each svn:date revprop while dumping, in the function write_revision_record(). This seems to have been done in r842390 in order to upgrade from pre-0.14 repository format to the new timestamp format introduced in 0.14 -- see issue SVN-614 "DAV:creationdate needs to be an ISO8601 date". svn_time_from_cstring() reads either new or old format, and then svn_time_to_cstring() writes the new format. However, this does not only convert old to new format, but could also make textual changes to the string if the revprop value is not already canonical. Dump should carefully output exactly what is in the repository and not gratuitously change it. In retrospect, such a transformation should have been done during "svnadmin load" instead of in "dump". While "svnadmin dump" makes this transformation, "svnrdump dump" and "svndumpfilter" do not. This could lead to unintended differences in dump output depending on which tool is used. (I made some progress in unifying the output logic for those three dump producers a couple of years ago, but I left this part alone because I did not know what to do with it.) The discrepancy I noticed above comes from prop_tests.py 41 and 42 which explicitly propset svn:date to a value such as '2015-01-01T00:00:00.0Z'. "svnadmin dump" was munging this non-canonical value on output, which meant it was not a true dump. (Whether a non-canonical format should have been allowed into the repository in the first place is a separate issue.) Therefore I will remove this code path. It even has a comment saying "### Remove this when it is no longer needed for sure" which referred to being needed for pre-0.14 upgrades. We definitely no longer need that. -- - Julian
Re: authz changes between 1.9 and 1.10
On 30.07.2018 14:51, Philip Martin wrote: > Daniel Shahaf writes: > >> Branko Čibej wrote on Mon, 30 Jul 2018 03:07 +0200: >>> It's definitely better to have consistent behaviour across all rule >>> types. >> +1 > I like the idea of achieving consistency by making the duplicate entries > into an error: it changes the behaviour of 1.10 but anyone affected gets > an error. It's also a simpler version of my existing patch. > > Consistency is more of a problem for the inheritance case: > > [/path] > userA = rw > [repo:/path] > userB = rw > > because any change will silently change the behaviour of 1.10. The glob > implementation made file order (sequence number in the implementation) > important and my experimental inheritance patch arbitrarily picks the > per-repository sequence number when inheriting but without any real > justification. Shouldn't it be the other way around? At request processing time, only one rule will match. If it's a per-repository rule, it should be possible to check if a generic rule with the same path exists. This part can even be pre-calculated in the parser, so expensive lookup at processing time can be avoided. > The choice has no effect when using the glob > implementation on a 1.9 authz file, but the choice starts to matter when > glob rules are present. > > The release notes for 1.10 didn't specify how glob rules are > prioritised, so anyone using them had to read the design docs or > experiment. How inheritance affects glob rules is the outstanding > behaviour question. Do the glob inherit like non-glob rules? How does > the sequence number of inherited rules get defined? One option is to > make :glob: into an error, and introduce :GLOB: with defined rules for > inheritance. I think it's fair to say that the current behaviour in 1.10.x is a bug, then explain in an announcement what we're doing about it. It was never the intention of the new authz code to *silently* change behaviour from 1.9. -- Brane
