Hi again,
I made several patches which will fix the issue.
svn-info-assert-path.patch.txt and svn-info-fix-revision-kind.patch.txt
files are attached to this email.
Any thoughts?
On Fri, May 2, 2025 at 4:20 PM Timofei Zhakov <t...@chemodax.net> wrote:
> Hi,
>
> I was consuming the Subversion API, especially requesting the working copy
> information, and noticed a little bit of weird behaviour when the WORKING
> revision kind was specified. For some reason, it makes a request to a
> server, even if it shouldn't. However, if svn_opt_revision_unspecified
> revision kind is passed instead, everything works perfectly as a local
> working copy operation.
>
> I dug down into the implementation a little bit, and noticed this block of
> code: [1]. Here, info decides whether to do a lookup into WC DB or make an
> RA request. However, the branch for local info is being invoked only if the
> kind of both revision and peg revision are 'unspecified', but 'working'
> doesn't trigger it. Maybe, the svn_opt_revision_base revision kind should
> be considered as local, but I'm not sure, because there is a reason for
> svn_client_info4 to execute on the target itself but not its working and
> base variants if you get what I mean. But this is a reason against
> supporting info for 'working' revision. Still, this must be either
> disallowed or optimised. What do you think?
>
> Also, I noticed that it doesn't assert the abspath_or_url argument whether
> it's a local abspath or not. This may cause a crash later on.
>
> Am I right with this or is there conceptual proof behind?
>
> [1]
> https://github.com/apache/subversion/blob/54e2d898f180c47050d0fc788f75ab5e2e43f6e1/subversion/libsvn_client/info.c#L353
>
> --
> Timofei Zhakov
>
--
Timofei Zhakov
Ensure a path was given to client's info command when performing
a local operation since url target is invalid in this case.
* subversion/libsvn_client/info.c
(includes): Add svn_path.h
(svn_client_info4): Add assertion
Index: subversion/libsvn_client/info.c
===================================================================
--- subversion/libsvn_client/info.c (revision 1925415)
+++ subversion/libsvn_client/info.c (working copy)
@@ -29,6 +29,7 @@
#include "svn_client.h"
#include "svn_dirent_uri.h"
#include "svn_hash.h"
+#include "svn_path.h"
#include "svn_pools.h"
#include "svn_sorts.h"
@@ -355,6 +356,9 @@
&& (peg_revision == NULL
|| peg_revision->kind == svn_opt_revision_unspecified))
{
+ /* Ensure a path was given since a local operation is being performed */
+ SVN_ERR_ASSERT(! svn_path_is_url(abspath_or_url));
+
/* Do all digging in the working copy. */
wc_info_receiver_baton_t b;
Perform working copy operation in client's info command even if `working`
or `base` revision's or peg_revision's kinds were given.
* subversion/libsvn_client/info.c
(svn_client_info4): Ditto.
Index: subversion/libsvn_client/info.c
===================================================================
--- subversion/libsvn_client/info.c (revision 1925415)
+++ subversion/libsvn_client/info.c (working copy)
@@ -351,9 +352,13 @@
depth = svn_depth_empty;
if ((revision == NULL
- || revision->kind == svn_opt_revision_unspecified)
+ || revision->kind == svn_opt_revision_unspecified
+ || revision->kind == svn_opt_revision_working
+ || revision->kind == svn_opt_revision_base)
&& (peg_revision == NULL
- || peg_revision->kind == svn_opt_revision_unspecified))
+ || peg_revision->kind == svn_opt_revision_unspecified
+ || peg_revision->kind == svn_opt_revision_working
+ || peg_revision->kind == svn_opt_revision_base))
{
/* Do all digging in the working copy. */
wc_info_receiver_baton_t b;
