Hi Julian, Thank you for your quick response and patch. I hope that this is fixed in the next 1.8.x release and that CollabNet will also release an update to Subversion Edge.
I'm still not so clear how this is exactly a problem with svnrdump 1.8.19 (from Subversion Edge). This is my first time browsing through the svn source code, and I hope you'll indulge me with my questions below. We are using Subversion Edge 5.2.2 (Windows 64-bit) on a Windows Server 2012 (64-bit) OS. I also verified that the httpd.exe, svn.exe, and svnrdump.exe binaries are 64-bit. (I ran the "file xxx" command in Cygwin and also checked their PE signatures as described here: https://superuser.com/questions/358434/how-to-check-if-a-binary-is-32-or-64-bit-on-windows ). You mentioned: > SVN_ERR(svn_stream_printf(eb->stream, pool, SVN_REPOS_DUMPFILE_CONTENT_LENGTH ": %ld\n\n", (unsigned long)info->size + propstring->len)); > info->size is apr_off_t ... probably 64 bits on most systems. > propstring->len is apr_size_t ... probably 64 bits on most systems. > It uses "%lu" for the text content, which thus work OK up to 4 GB, and "%ld" for the overall content length which thus only works up to 2 GB. On a 64-bit system where unsigned long is uint64: If info->size is int64 and propstring->len is uint64, then the expression "(unsigned long)info->size + propstring->len" will produce uint64. This result should be printed with "%lu" (as in your patch). However, printing the result with "%ld" will still print a positive value (signed int64), I believe. If for some reason, info->size is just int32, then it can only store max int32 (2 GB). So the earlier call to apr_file_info_get should already have failed if the file size is greater than 2 GB. err = apr_file_info_get(info, APR_FINFO_SIZE, eb->delta_file); if (err) SVN_ERR(svn_error_wrap_apr(err, NULL)); However, apr_file_info_get did not return an error; so it just somehow returned a negative value in info->size? Regards, Ronald On Wed, Nov 22, 2017 at 6:05 PM, Julian Foad <julianf...@apache.org> wrote: > Julian Foad wrote: > >> (dropping users@) >> >> Julian Foad wrote: >> >>> The attached patch should fix it; not yet tested. >>> >> > Proposed for backport to 1.8.x. > > - Julian > > > I have opened https://issues.apache.org/jira/browse/SVN-4707 >> and attached the patch there. (Patch v2 is same as v1 but with tweaked >> log message.) >> >> We briefly discussed testing. It's a pretty obvious fix, but ideally we >> would write a regression test. We don't want to store 2 GB* of temporary >> data during a test run (that would make testing onerous) so we would need >> to write a test that generates data, streams it to the rdump 'dump' >> function, pipes that into the rdump 'load', and checks that it parses >> without throwing errors, without storing the loaded data. >> >> Anyone interested in writing such a test? >> >> * Actually we should test with over 4 GB because as well as the "%ld" >> bug I found and fixed a "%lu" bug in nearby code at the same time. >> >> - Julian >> >
