You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Johan Corveleyn <jc...@gmail.com> on 2018/12/12 10:30:05 UTC
Re: Subversion Exception!
On Mon, Dec 10, 2018 at 9:47 AM Peta Miller <no...@gmx.de> wrote:
>
> Hello,
>
>
>
> i got following exception at trying to start the tortoisesvn Project Monitor with a doubleclick on the tray icon.
>
> System: WIN 10, Build 15063
>
> ---------------------------
> Subversion Exception!
> ---------------------------
> Subversion encountered a serious problem.
> Please take the time to report this on the Subversion mailing list
> with as much information as possible about what
> you were trying to do.
> But please first search the mailing list archives for the error message
> to avoid reporting the same problem repeatedly.
> You can find the mailing list archives at
> https://subversion.apache.org/mailing-lists.html
>
> Subversion reported the following
> (you can copy the content of this dialog
> to the clipboard using Ctrl-C):
>
> In file
> 'D:\Development\SVN\Releases\TortoiseSVN-1.10.1\ext\subversion\subversion\libsvn_wc\wc_db.c'
> line 10238: assertion failed (svn_dirent_is_absolute(local_abspath))
> ---------------------------
> OK
> ---------------------------
[ Moved this thread to users@subversion.apache.org -- bcc'd dev@ so it
is dropped on further replies ]
Hi Peta,
It is likely that this is a problem specific to TortoiseSVN, and not
to core SVN. TortoiseSVN has its own mailinglists, so you should
report your problem there:
https://tortoisesvn.net/community.html
(Before doing so, you could perhaps upgrade to the latest version,
1.11.0, and see if the problem is already fixed)
--
Johan
Re: Subversion Exception!
Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
> [...] If you have
> found a value for which svn_foo_is_canonical(svn_foo_canonicalize(value))
> returns false, please tell us what that value is.
Daniel, some examples:
path = "foo";
-> assertion 'svn_uri_is_canonical(svn_uri_canonicalize(f, pool), pool)' failed
#define SVN_USE_DOS_PATHS
path = "./v:foo"
-> assertion 'svn_dirent_is_canonical(svn_dirent_canonicalize(f, pool), pool)' failed
--
- Julian
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On 28.12.2018 11:19, Branko Čibej wrote:
> On 28.12.2018 11:05, Bert Huijben wrote:
>> +svn_error_t *
>> +svn_error__malfunction_f(svn_boolean_t can_return,
>> + const char *file, int line,
>> + const char *fmt, ...)
>> +{
>> + apr_pool_t *pool = svn_pool_create(NULL);
>> + va_list ap;
>> + const char *expr;
>> +
>> + va_start(ap, fmt);
>> + expr = apr_pvsprintf(pool, fmt, ap);
>> + va_end(ap);
>> + return malfunction_handler(can_return, file, line, expr);
>> +}
>> +
>>
>> Can we somehow use a smarter pool (perhaps just require passing an existing
>> scratch pool as some similar apr apis do). Some library users do use the
>> can_return feature and/or exception handling and we would introduce a minor
>> memory leak.
> We'd have to somehow tweak the malfunction_handler to create its own
> svn_error_t* from the provided pool.
>
> Another option is to check if can_return is set, and only then, call the
> malfunction_handler first and then change the message stored in the
> returned svn_error_t, using the error pool for formatting.
... but that might cause the expression to not be dispayed to the user ...
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On 28.12.2018 11:05, Bert Huijben wrote:
> +svn_error_t *
> +svn_error__malfunction_f(svn_boolean_t can_return,
> + const char *file, int line,
> + const char *fmt, ...)
> +{
> + apr_pool_t *pool = svn_pool_create(NULL);
> + va_list ap;
> + const char *expr;
> +
> + va_start(ap, fmt);
> + expr = apr_pvsprintf(pool, fmt, ap);
> + va_end(ap);
> + return malfunction_handler(can_return, file, line, expr);
> +}
> +
>
> Can we somehow use a smarter pool (perhaps just require passing an existing
> scratch pool as some similar apr apis do). Some library users do use the
> can_return feature and/or exception handling and we would introduce a minor
> memory leak.
We'd have to somehow tweak the malfunction_handler to create its own
svn_error_t* from the provided pool.
Another option is to check if can_return is set, and only then, call the
malfunction_handler first and then change the message stored in the
returned svn_error_t, using the error pool for formatting.
-- Brane
Re: Subversion Exception!
Posted by Bert Huijben <be...@qqmail.nl>.
+svn_error_t *
+svn_error__malfunction_f(svn_boolean_t can_return,
+ const char *file, int line,
+ const char *fmt, ...)
+{
+ apr_pool_t *pool = svn_pool_create(NULL);
+ va_list ap;
+ const char *expr;
+
+ va_start(ap, fmt);
+ expr = apr_pvsprintf(pool, fmt, ap);
+ va_end(ap);
+ return malfunction_handler(can_return, file, line, expr);
+}
+
Can we somehow use a smarter pool (perhaps just require passing an existing
scratch pool as some similar apr apis do). Some library users do use the
can_return feature and/or exception handling and we would introduce a minor
memory leak.
Further +1
Bert
On Thu, Dec 13, 2018 at 4:05 PM Julian Foad <ju...@apache.org> wrote:
> Johan Corveleyn wrote:
> > Just thinking out loud here, but could we perhaps log the
> > non-canonical value as part of the assertion output?
> > [...]
> > something like
> >
> > line 10238: assertion failed
> > (svn_dirent_is_absolute(local_abspath='C:ImNotCanonical'))
>
> Yes, Johan, we could do that.
>
> The attached patch demonstrates it working.
>
> Test output:
> [[[
> $ subversion/tests/libsvn_wc/wc-test 1
> ...
> svn_tests: E235000: In file '.../subversion/libsvn_wc/wc_db.c' line 10238:
> assertion failed (svn_dirent_is_absolute(local_abspath='C:ImNotAbsolute'))
> ...
> ]]]
>
> --
> - Julian
>
Re: Subversion Exception!
Posted by Julian Foad <ju...@apache.org>.
Branko Čibej wrote:
> On 13.12.2018 16:42, Johan Corveleyn wrote:
> >> [...] assertion failed (svn_dirent_is_absolute(local_abspath='C:ImNotAbsolute'))
> > Nice :-). I'm not sure what others think, but IMHO that would make
> > these error reports a lot more useful ...
>
> [...] this doesn't really
> help, other than putting possibly sensitive paths into the crash handler
> info? We really shouldn't do it this way, users *will* just copy and
> paste the output tot he 'net.
"possibly sensitive paths", Brane? I call FUD. Give me a concrete objection or I'm with Johan: this helps in the same way that more info about the problem always helps.
--
- Julian
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On Fri, 14 Dec 2018, 05:41 Nathan Hartman <hartman.nathan@gmail.com wrote:
> On Thu, Dec 13, 2018 at 11:59 AM Branko Čibej <br...@apache.org> wrote:
>
>> I never said that it's a good idea to abort in a library. We made a
>> mistake in the early days of this project to allow such patterns.
>
>
>>
>> I am quite angry at the contrariness and stubbornness of one TSVN
>> developer, who is *also* a Subversion PMC member.
>
>
> I propose a compromise.
>
> Remove this mistake in 2.0, when API/ABI compatibility with old mistakes
> no longer applies, but meanwhile improve the message in TSvn.
>
"Compromise" is not the issue. Attitude is.
Waiting for 2.0 isn't necessary, either. Stuff can be fixed before that.
> Just to be clear, I *don't* propose to make the jump to 2.0 anytime soon,
> nor to do so in haste. But I do propose the need for a Confluence page to
> start the conversation about what 2.0 should be. Right now I can think of
> two things for that page: (1) composing a new statement of Subversion's
> goal for 2.0 (being a better CVS is just so 1.0), and (2) a list of old
> mistakes that should be fixed at the turn of the major version number, like
> this abort() thing.
>
Mission statements are marketing sugar-coating that I have no talent for.
As for 2.0 ... The only reason to ever make such a release would be to
remove irredemably broken things from our API and protocols. There's
currently nothing of the sort in SVN that can't be solved with
documentation, API upgrades and bug fixes.
-- Brane
>
Re: Subversion Exception!
Posted by Nathan Hartman <ha...@gmail.com>.
On Thu, Dec 13, 2018 at 11:59 AM Branko Čibej <br...@apache.org> wrote:
> I never said that it's a good idea to abort in a library. We made a
> mistake in the early days of this project to allow such patterns.
>
> I am quite angry at the contrariness and stubbornness of one TSVN
> developer, who is *also* a Subversion PMC member.
I propose a compromise.
Remove this mistake in 2.0, when API/ABI compatibility with old mistakes no
longer applies, but meanwhile improve the message in TSvn.
Just to be clear, I *don't* propose to make the jump to 2.0 anytime soon,
nor to do so in haste. But I do propose the need for a Confluence page to
start the conversation about what 2.0 should be. Right now I can think of
two things for that page: (1) composing a new statement of Subversion's
goal for 2.0 (being a better CVS is just so 1.0), and (2) a list of old
mistakes that should be fixed at the turn of the major version number, like
this abort() thing.
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On 13.12.2018 17:39, Mark Phippard wrote:
>> On Dec 13, 2018, at 11:26 AM, Branko Čibej <br...@apache.org> wrote:
>>
>> On 13.12.2018 17:09, Mark Phippard wrote:
>>>> On Dec 13, 2018, at 10:53 AM, Michael Pilato <cm...@collab.net> wrote:
>>>>
>>>>> On 12/13/18 10:45 AM, Branko Čibej wrote:
>>>>> Uh. I forgot about the malfunction handler. However this doesn't really
>>>>> help, other than putting possibly sensitive paths into the crash handler
>>>>> info? We really shouldn't do it this way, users *will* just copy and
>>>>> paste the output tot he 'net.
>>>> Ahem. What Grandpa *meant* to say was:
>>>>
>>>> "Oh, cool! So there _is_ a way to report the non-canonical path.
>>>> Thanks for figuring this out, Julian! Unfortunately, it comes at a
>>>> cost, namely that of revealing potentially sensitive paths in the output
>>>> which I strongly suspect will get copied and paste to the 'net. If we
>>>> could mitigate that part of it, this might turn out to be truly beneficial."
>>>>
>>>> ;-)
>>> Honestly, it seems like complete FUD to me and trying to save face or just be obstinate.
>>>
>>> FWIW, I agree with Stefan on all of this. We should not be doing abort from a library. Whether TSVN could do more to avoid it seems like a separate issue. I do not see why the library cannot just return a useful error and allow the caller to handle it.
>>
>> Backwards compat dictates that we can't change the signatures of those
>> functions. We can write new ones with different signatures and deprecate
>> the old ones.
>>
>>
>> Or rather "they" because I'm not volunteering for the code churn and
>> related backporting fallout. That's not stopping anyone else from having
>> a go.
>>
> Fair enough. I have largely stayed out of the conversation for the same reason ... I am not offering to help with the effort.
>
> It is good to hear you are not necessarily opposed to someone doing this work because I was under the impression you would try to block those changes.
I never said that it's a good idea to abort in a library. We made a
mistake in the early days of this project to allow such patterns.
I am quite angry at the contrariness and stubbornness of one TSVN
developer, who is *also* a Subversion PMC member. He could have proposed
changes or fixed those functions years ago. Instead its always "you
should fix this" and "you should fix that", when it's his project, too.
WTF? Am I the only one who finds this to be really bad form?
-- Brane
Re: Subversion Exception!
Posted by Mark Phippard <ma...@gmail.com>.
> On Dec 13, 2018, at 11:26 AM, Branko Čibej <br...@apache.org> wrote:
>
> On 13.12.2018 17:09, Mark Phippard wrote:
>>> On Dec 13, 2018, at 10:53 AM, Michael Pilato <cm...@collab.net> wrote:
>>>
>>>> On 12/13/18 10:45 AM, Branko Čibej wrote:
>>>> Uh. I forgot about the malfunction handler. However this doesn't really
>>>> help, other than putting possibly sensitive paths into the crash handler
>>>> info? We really shouldn't do it this way, users *will* just copy and
>>>> paste the output tot he 'net.
>>> Ahem. What Grandpa *meant* to say was:
>>>
>>> "Oh, cool! So there _is_ a way to report the non-canonical path.
>>> Thanks for figuring this out, Julian! Unfortunately, it comes at a
>>> cost, namely that of revealing potentially sensitive paths in the output
>>> which I strongly suspect will get copied and paste to the 'net. If we
>>> could mitigate that part of it, this might turn out to be truly beneficial."
>>>
>>> ;-)
>> Honestly, it seems like complete FUD to me and trying to save face or just be obstinate.
>>
>> FWIW, I agree with Stefan on all of this. We should not be doing abort from a library. Whether TSVN could do more to avoid it seems like a separate issue. I do not see why the library cannot just return a useful error and allow the caller to handle it.
>
>
> Backwards compat dictates that we can't change the signatures of those
> functions. We can write new ones with different signatures and deprecate
> the old ones.
>
>
> Or rather "they" because I'm not volunteering for the code churn and
> related backporting fallout. That's not stopping anyone else from having
> a go.
>
Fair enough. I have largely stayed out of the conversation for the same reason ... I am not offering to help with the effort.
It is good to hear you are not necessarily opposed to someone doing this work because I was under the impression you would try to block those changes.
Mark
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On 13.12.2018 17:09, Mark Phippard wrote:
>> On Dec 13, 2018, at 10:53 AM, Michael Pilato <cm...@collab.net> wrote:
>>
>>> On 12/13/18 10:45 AM, Branko Čibej wrote:
>>> Uh. I forgot about the malfunction handler. However this doesn't really
>>> help, other than putting possibly sensitive paths into the crash handler
>>> info? We really shouldn't do it this way, users *will* just copy and
>>> paste the output tot he 'net.
>> Ahem. What Grandpa *meant* to say was:
>>
>> "Oh, cool! So there _is_ a way to report the non-canonical path.
>> Thanks for figuring this out, Julian! Unfortunately, it comes at a
>> cost, namely that of revealing potentially sensitive paths in the output
>> which I strongly suspect will get copied and paste to the 'net. If we
>> could mitigate that part of it, this might turn out to be truly beneficial."
>>
>> ;-)
> Honestly, it seems like complete FUD to me and trying to save face or just be obstinate.
>
> FWIW, I agree with Stefan on all of this. We should not be doing abort from a library. Whether TSVN could do more to avoid it seems like a separate issue. I do not see why the library cannot just return a useful error and allow the caller to handle it.
Backwards compat dictates that we can't change the signatures of those
functions. We can write new ones with different signatures and deprecate
the old ones.
Or rather "they" because I'm not volunteering for the code churn and
related backporting fallout. That's not stopping anyone else from having
a go.
-- Brane
Re: Subversion Exception!
Posted by Mark Phippard <ma...@gmail.com>.
> On Dec 13, 2018, at 11:30 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
>
> Mark Phippard wrote on Thu, 13 Dec 2018 11:09 -0500:
>> FWIW, I agree with Stefan on all of this. We should not be doing abort
>> from a library. Whether TSVN could do more to avoid it seems like a
>> separate issue. I do not see why the library cannot just return a
>> useful error and allow the caller to handle it.
>
> If what you're saying is that we should, whenever possible, use
> SVN_ERR_ASSERT() rather than SVN_ERR_ASSERT_NO_RETURN(), I don't
> think anyone will disagree. Anyone who wants to make that happen is
> welcome to.
I cannot speak to the code level you are here, but "maybe"? I think the issue is when an error happens, as a user of a library I want it to return an error. Obviously in most scenarios that is what SVN does today. But for some of these issues the library just aborts. So if you are a GUI function the whole app is killed when this happens which sucks and is just not very library-like.
If TSVN could get good errors back, Stefan probably would only send them to the TSVN list and he probably would have fixed these obscure problems years ago. But when the app just crashes and leaves only moderate breadcrumbs to help resolve the problem it does not leave much to go on.
Mark
Re: Subversion Exception!
Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
> At the same time, I think Brane had a valid point. It is not
> users@subversion's job to triage TortoiseSVN error reports;
It's a valid point of view but please let's stop pushing that point, especially don't ask Stefan to go over it again. Please let's concentrate on what we can do to help, both technical and social.
--
- Julian
Re: Subversion Exception!
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Mark Phippard wrote on Thu, 13 Dec 2018 11:09 -0500:
> FWIW, I agree with Stefan on all of this. We should not be doing abort
> from a library. Whether TSVN could do more to avoid it seems like a
> separate issue. I do not see why the library cannot just return a
> useful error and allow the caller to handle it.
If what you're saying is that we should, whenever possible, use
SVN_ERR_ASSERT() rather than SVN_ERR_ASSERT_NO_RETURN(), I don't
think anyone will disagree. Anyone who wants to make that happen is
welcome to.
At the same time, I think Brane had a valid point. It is not
users@subversion's job to triage TortoiseSVN error reports;
TortoiseSVN's developers should be the ones triaging bug reports from
their own users. Stefan, if I am wrong here please would you be kind
enough to explain how.
Daniel
Re: Subversion Exception!
Posted by Mark Phippard <ma...@gmail.com>.
> On Dec 13, 2018, at 10:53 AM, Michael Pilato <cm...@collab.net> wrote:
>
>> On 12/13/18 10:45 AM, Branko Čibej wrote:
>> Uh. I forgot about the malfunction handler. However this doesn't really
>> help, other than putting possibly sensitive paths into the crash handler
>> info? We really shouldn't do it this way, users *will* just copy and
>> paste the output tot he 'net.
>
> Ahem. What Grandpa *meant* to say was:
>
> "Oh, cool! So there _is_ a way to report the non-canonical path.
> Thanks for figuring this out, Julian! Unfortunately, it comes at a
> cost, namely that of revealing potentially sensitive paths in the output
> which I strongly suspect will get copied and paste to the 'net. If we
> could mitigate that part of it, this might turn out to be truly beneficial."
>
> ;-)
Honestly, it seems like complete FUD to me and trying to save face or just be obstinate.
FWIW, I agree with Stefan on all of this. We should not be doing abort from a library. Whether TSVN could do more to avoid it seems like a separate issue. I do not see why the library cannot just return a useful error and allow the caller to handle it.
Mark
Re: Subversion Exception!
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Thu, 13 Dec 2018 17:00 +0100:
> On 13.12.2018 16:53, Michael Pilato wrote:
> > On 12/13/18 10:45 AM, Branko Čibej wrote:
> >> Uh. I forgot about the malfunction handler. However this doesn't really
> >> help, other than putting possibly sensitive paths into the crash handler
> >> info? We really shouldn't do it this way, users *will* just copy and
> >> paste the output tot he 'net.
> > Ahem. What Grandpa *meant* to say was:
> >
> > "Oh, cool! So there _is_ a way to report the non-canonical path.
> > Thanks for figuring this out, Julian! Unfortunately, it comes at a
> > cost, namely that of revealing potentially sensitive paths in the output
> > which I strongly suspect will get copied and paste to the 'net. If we
> > could mitigate that part of it, this might turn out to be truly beneficial."
>
>
> Well, no, I meant to say exactly what I said. But if I were in a
> politically correct fame of mind, then I might have said something like
> what you wrote.
>
> Re FUD: it's not just paths, it's also URLs, and people do consider one
> or the other sensitive. Of course ... in the end that's no worse than
> printing paths or URLs in error messages.
>
The error message should include all information relevant to the
problem. (As a rule of thumb, if an error message doesn't have a printf
"%s" expando, it's probably incomplete.) If users consider some part
of it sensitive, they shouldn't intentionally post that part to the Internet.
The rub lies at "intentionally". It's conceivable that a user might not
realize that the support forum they're posting the error message to is
public. So, I'd say, have the error message include all relevant
information, and if a user tries to seek help about it, make it
*crystal* clear that users@ is publicly archived.
Cheers,
Daniel
Re: New canonicalization functions [was: Subversion Exception!]
Posted by Branko Čibej <br...@apache.org>.
On Fri, 14 Dec 2018, 16:18 Julian Foad <julianfoad@apache.org wrote:
> Branko Čibej wrote:
> > r1848943, for review. [...]
>
> Woo hoo! Thanks, Brane.
>
> Are you willing to add random-input testing for them?
>
> Minor comments:
> * the 'relpath' one is not needed because, AFAIK, it's possible to
> canonicalize any relpath and we already do;
>
Possibly. I admit I was quite short on time, so I went for consistency
first.
* the non-canonical 'result' and scratch pool params are probably
> over-kill; a subjective judgment, I know, so just a suggestion.
>
My thinking was that a GUI client may want to handle canonicalisation
errors differently than just by displaying the error message we happen to
generate.
Separate result and scratch pools are just policy, I think we've learned by
now that taking shortcuts with pools eventually leads to problems.
-- Brane
Re: New canonicalization functions [was: Subversion Exception!]
Posted by Branko Čibej <br...@apache.org>.
On 15.12.2018 12:24, Branko Čibej wrote:
> On 14.12.2018 19:05, Branko Čibej wrote:
>> On Fri, 14 Dec 2018, 18:14 Julian Foad <julianfoad@apache.org wrote:
>>
>>> Julian Foad wrote:
>>>> Are you willing to add random-input testing for them?
>>> The attached patch 'dirent-uri-test-random-2.patch' tests rules like:
>>>
>>> * every result should pass an X_is_canonical() test (obvious by code
>>> inspection);
>>> * every other input should produce SVN_ERR_CANONICALIZATION_FAILED;
>>> * when a path is "canonical", it should be unchanged by "canonicalize".
>>>
>>> Some findings:
>>>
>>> * svn_uri_canonicalize_safe("") aborts;
>>> * svn_uri_canonicalize_safe("/foo") aborts;
>>>
>> We can fix this in the private "canonicalize()" function that all these
>> eventually call. As before, I didn't have time to unknit the internal
>> implementation.
>
> I saw a funny thing in that function the other day ...
>
> if (SVN_PATH_IS_EMPTY(path))
> {
> assert(type != type_uri);
> return "";
> }
r1848990. There were two assertions in that function.
Note that we'll also have to create "safe" versions of the
_internal_style() functions, because those pass their result through
their _canonicalize() colleagues.
-- Brane
P.S.: Replacing all assertions in libsvn_subr/dirent_uri.c will cause
code churn of awseome proportions.
Re: New canonicalization functions [was: Subversion Exception!]
Posted by Branko Čibej <br...@apache.org>.
On 14.12.2018 19:05, Branko Čibej wrote:
> On Fri, 14 Dec 2018, 18:14 Julian Foad <julianfoad@apache.org wrote:
>
>> Julian Foad wrote:
>>> Are you willing to add random-input testing for them?
>> The attached patch 'dirent-uri-test-random-2.patch' tests rules like:
>>
>> * every result should pass an X_is_canonical() test (obvious by code
>> inspection);
>> * every other input should produce SVN_ERR_CANONICALIZATION_FAILED;
>> * when a path is "canonical", it should be unchanged by "canonicalize".
>>
>> Some findings:
>>
>> * svn_uri_canonicalize_safe("") aborts;
>> * svn_uri_canonicalize_safe("/foo") aborts;
>>
>
> We can fix this in the private "canonicalize()" function that all these
> eventually call. As before, I didn't have time to unknit the internal
> implementation.
I saw a funny thing in that function the other day ...
if (SVN_PATH_IS_EMPTY(path))
{
assert(type != type_uri);
return "";
}
-- Brane
Re: New canonicalization functions [was: Subversion Exception!]
Posted by Julian Foad <ju...@apache.org>.
Branko Čibej wrote:
> What inconsistencies? A canonical URI has the scheme and host name in
> lowercase, but the path, query, fragment and user info parts are case-
> sensitive.>
> Similarly, I think we (or APR?) make drive letters lowercase.
In both cases I found examples where is_canonical said an input was
canonical but canonicalize changed its case.
To everything else: ack.
- Julian
Re: New canonicalization functions [was: Subversion Exception!]
Posted by Branko Čibej <br...@apache.org>.
On Fri, 14 Dec 2018, 18:14 Julian Foad <julianfoad@apache.org wrote:
> Julian Foad wrote:
> > Are you willing to add random-input testing for them?
>
> The attached patch 'dirent-uri-test-random-2.patch' tests rules like:
>
> * every result should pass an X_is_canonical() test (obvious by code
> inspection);
> * every other input should produce SVN_ERR_CANONICALIZATION_FAILED;
> * when a path is "canonical", it should be unchanged by "canonicalize".
>
> Some findings:
>
> * svn_uri_canonicalize_safe("") aborts;
> * svn_uri_canonicalize_safe("/foo") aborts;
>
We can fix this in the private "canonicalize()" function that all these
eventually call. As before, I didn't have time to unknit the internal
implementation.
* upper/lower case inconsistencies in URIs
>
What inconsistencies? A canonical URI has the scheme and host name in
lowercase, but the path, query, fragment and user info parts are
case-sensitive.
> I previously also found upper/lower case inconsistencies in dirent drive
> letters, when running these tests with "#define SVN_USE_DOS_PATHS" set in
> dirent_uri.c, but am right now failing to replicate that.
>
Similarly, I think we (or APR?) make drive letters lowercase.
-- Brane
Re: New canonicalization functions [was: Subversion Exception!]
Posted by Julian Foad <ju...@apache.org>.
Julian Foad wrote:
> Are you willing to add random-input testing for them?
The attached patch 'dirent-uri-test-random-2.patch' tests rules like:
* every result should pass an X_is_canonical() test (obvious by code inspection);
* every other input should produce SVN_ERR_CANONICALIZATION_FAILED;
* when a path is "canonical", it should be unchanged by "canonicalize".
Some findings:
* svn_uri_canonicalize_safe("") aborts;
* svn_uri_canonicalize_safe("/foo") aborts;
* upper/lower case inconsistencies in URIs
I previously also found upper/lower case inconsistencies in dirent drive letters, when running these tests with "#define SVN_USE_DOS_PATHS" set in dirent_uri.c, but am right now failing to replicate that.
> * the 'relpath' one is not needed because, AFAIK, it's possible to canonicalize any
> relpath and we already do;
Maybe that will no longer be true if we decide to disallow some characters (control characters) and/or check for valid UTF-8 in the future.
--
- Julian
Re: New canonicalization functions [was: Subversion Exception!]
Posted by Julian Foad <ju...@apache.org>.
Branko Čibej wrote:
> r1848943, for review. [...]
Woo hoo! Thanks, Brane.
Are you willing to add random-input testing for them?
Minor comments:
* the 'relpath' one is not needed because, AFAIK, it's possible to canonicalize any relpath and we already do;
* the non-canonical 'result' and scratch pool params are probably over-kill; a subjective judgment, I know, so just a suggestion.
--
- Julian
New canonicalization functions [was: Subversion Exception!]
Posted by Branko Čibej <br...@apache.org>.
On 13.12.2018 17:00, Branko Čibej wrote:
> On 13.12.2018 16:53, Michael Pilato wrote:
>> On 12/13/18 10:45 AM, Branko Čibej wrote:
>>> Uh. I forgot about the malfunction handler. However this doesn't really
>>> help, other than putting possibly sensitive paths into the crash handler
>>> info? We really shouldn't do it this way, users *will* just copy and
>>> paste the output tot he 'net.
>> Ahem. What Grandpa *meant* to say was:
>>
>> "Oh, cool! So there _is_ a way to report the non-canonical path.
>> Thanks for figuring this out, Julian! Unfortunately, it comes at a
>> cost, namely that of revealing potentially sensitive paths in the output
>> which I strongly suspect will get copied and paste to the 'net. If we
>> could mitigate that part of it, this might turn out to be truly beneficial."
>
> Well, no, I meant to say exactly what I said. But if I were in a
> politically correct fame of mind, then I might have said something like
> what you wrote.
>
> Re FUD: it's not just paths, it's also URLs, and people do consider one
> or the other sensitive. Of course ... in the end that's no worse than
> printing paths or URLs in error messages.
>
> I still think we should add canonicalisation functions that validate
> their own output.
r1848943, for review. Obviouisly we'd have to start using these in our
own code for them to be any good, but TSVN could use them, too. Doing
that is independent of changing the is-canonical assertions as Johan and
Julian suggested.
-- Brane
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On 13.12.2018 16:53, Michael Pilato wrote:
> On 12/13/18 10:45 AM, Branko Čibej wrote:
>> Uh. I forgot about the malfunction handler. However this doesn't really
>> help, other than putting possibly sensitive paths into the crash handler
>> info? We really shouldn't do it this way, users *will* just copy and
>> paste the output tot he 'net.
> Ahem. What Grandpa *meant* to say was:
>
> "Oh, cool! So there _is_ a way to report the non-canonical path.
> Thanks for figuring this out, Julian! Unfortunately, it comes at a
> cost, namely that of revealing potentially sensitive paths in the output
> which I strongly suspect will get copied and paste to the 'net. If we
> could mitigate that part of it, this might turn out to be truly beneficial."
Well, no, I meant to say exactly what I said. But if I were in a
politically correct fame of mind, then I might have said something like
what you wrote.
Re FUD: it's not just paths, it's also URLs, and people do consider one
or the other sensitive. Of course ... in the end that's no worse than
printing paths or URLs in error messages.
I still think we should add canonicalisation functions that validate
their own output.
-- Brane
Re: Subversion Exception!
Posted by Michael Pilato <cm...@collab.net>.
On 12/13/18 10:45 AM, Branko Čibej wrote:
> Uh. I forgot about the malfunction handler. However this doesn't really
> help, other than putting possibly sensitive paths into the crash handler
> info? We really shouldn't do it this way, users *will* just copy and
> paste the output tot he 'net.
Ahem. What Grandpa *meant* to say was:
"Oh, cool! So there _is_ a way to report the non-canonical path.
Thanks for figuring this out, Julian! Unfortunately, it comes at a
cost, namely that of revealing potentially sensitive paths in the output
which I strongly suspect will get copied and paste to the 'net. If we
could mitigate that part of it, this might turn out to be truly beneficial."
;-)
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On 13.12.2018 16:42, Johan Corveleyn wrote:
> On Thu, Dec 13, 2018 at 4:05 PM Julian Foad <ju...@apache.org> wrote:
>> Johan Corveleyn wrote:
>>> Just thinking out loud here, but could we perhaps log the
>>> non-canonical value as part of the assertion output?
>>> [...]
>>> something like
>>>
>>> line 10238: assertion failed
>>> (svn_dirent_is_absolute(local_abspath='C:ImNotCanonical'))
>> Yes, Johan, we could do that.
>>
>> The attached patch demonstrates it working.
>>
>> Test output:
>> [[[
>> $ subversion/tests/libsvn_wc/wc-test 1
>> ...
>> svn_tests: E235000: In file '.../subversion/libsvn_wc/wc_db.c' line 10238: assertion failed (svn_dirent_is_absolute(local_abspath='C:ImNotAbsolute'))
>> ...
>> ]]]
> Nice :-). I'm not sure what others think, but IMHO that would make
> these error reports a lot more useful ...
Uh. I forgot about the malfunction handler. However this doesn't really
help, other than putting possibly sensitive paths into the crash handler
info? We really shouldn't do it this way, users *will* just copy and
paste the output tot he 'net.
-- Brane
Re: Subversion Exception!
Posted by Johan Corveleyn <jc...@gmail.com>.
On Thu, Dec 13, 2018 at 4:05 PM Julian Foad <ju...@apache.org> wrote:
>
> Johan Corveleyn wrote:
> > Just thinking out loud here, but could we perhaps log the
> > non-canonical value as part of the assertion output?
> > [...]
> > something like
> >
> > line 10238: assertion failed
> > (svn_dirent_is_absolute(local_abspath='C:ImNotCanonical'))
>
> Yes, Johan, we could do that.
>
> The attached patch demonstrates it working.
>
> Test output:
> [[[
> $ subversion/tests/libsvn_wc/wc-test 1
> ...
> svn_tests: E235000: In file '.../subversion/libsvn_wc/wc_db.c' line 10238: assertion failed (svn_dirent_is_absolute(local_abspath='C:ImNotAbsolute'))
> ...
> ]]]
Nice :-). I'm not sure what others think, but IMHO that would make
these error reports a lot more useful ...
--
Johan
Re: Subversion Exception!
Posted by Julian Foad <ju...@apache.org>.
Johan Corveleyn wrote:
> Just thinking out loud here, but could we perhaps log the
> non-canonical value as part of the assertion output?
> [...]
> something like
>
> line 10238: assertion failed
> (svn_dirent_is_absolute(local_abspath='C:ImNotCanonical'))
Yes, Johan, we could do that.
The attached patch demonstrates it working.
Test output:
[[[
$ subversion/tests/libsvn_wc/wc-test 1
...
svn_tests: E235000: In file '.../subversion/libsvn_wc/wc_db.c' line 10238: assertion failed (svn_dirent_is_absolute(local_abspath='C:ImNotAbsolute'))
...
]]]
--
- Julian
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On 13.12.2018 14:15, Johan Corveleyn wrote:
> On Wed, Dec 12, 2018 at 10:23 PM Daniel Shahaf <d....@daniel.shahaf.name> wrote:
>> Stefan Kueng wrote on Wed, 12 Dec 2018 21:16 +0100:
>>> And as I repeatedly said: TSVN does validate the input as good as it
>>> can. But if svn does neither describe the *exact* specs in the docs nor
>>> provide any APIs that do that, then TSVN has to guess.
>>> And no: specifying that paths/uris have to be "canonicalized" is not
>>> enough because I do that, using the svn APIs.
>>> So apparently that's not enough.
>> Stefan, there is nothing we can do with this information. If you have
>> found a value for which svn_foo_is_canonical(svn_foo_canonicalize(value))
>> returns false, please tell us what that value is.
> Just thinking out loud here, but could we perhaps log the
> non-canonical value as part of the assertion output?
> Instead of:
>
> line 10238: assertion failed (svn_dirent_is_absolute(local_abspath))
>
> something like
>
> line 10238: assertion failed
> (svn_dirent_is_absolute(local_abspath='C:ImNotCanonical'))
>
> That way, we would at least have some concrete information ...
>
There's no "logging" involved, the text comes from the expansion of the
assert() macro (or SVN_ERR_ASSERT_NO_RETURN), which can only show you
the assertion expression, not the values of the variables involved.
The only way to do that would be to change all those places to return
errors instead, and that would involve massive API churn.
An alternative would be to introduce new canonicalization wrapper
functions with the signature:
svn_error_t *
svn_xxx_canonicalize_safe(xxx_t **canonical, const xxx_t *original,
apr_pool_t *result_pool);
and these would perform the svn_xxx_is_canonical check internally. TSVN
would still have to upgrade to using the new functions to canonicalize
stuff before calling our other APIs. And of course nothing has been
stopping TSVN from using such canonicalization wrappers already; these
new functions would just be public wrappers around existing public
functions.
-- Brane
Re: Subversion Exception!
Posted by Johan Corveleyn <jc...@gmail.com>.
On Wed, Dec 12, 2018 at 10:23 PM Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Stefan Kueng wrote on Wed, 12 Dec 2018 21:16 +0100:
> > And as I repeatedly said: TSVN does validate the input as good as it
> > can. But if svn does neither describe the *exact* specs in the docs nor
> > provide any APIs that do that, then TSVN has to guess.
> > And no: specifying that paths/uris have to be "canonicalized" is not
> > enough because I do that, using the svn APIs.
> > So apparently that's not enough.
>
> Stefan, there is nothing we can do with this information. If you have
> found a value for which svn_foo_is_canonical(svn_foo_canonicalize(value))
> returns false, please tell us what that value is.
Just thinking out loud here, but could we perhaps log the
non-canonical value as part of the assertion output?
Instead of:
line 10238: assertion failed (svn_dirent_is_absolute(local_abspath))
something like
line 10238: assertion failed
(svn_dirent_is_absolute(local_abspath='C:ImNotCanonical'))
That way, we would at least have some concrete information ...
--
Johan
Re: Subversion Exception!
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Kueng wrote on Wed, 12 Dec 2018 21:16 +0100:
> And as I repeatedly said: TSVN does validate the input as good as it
> can. But if svn does neither describe the *exact* specs in the docs nor
> provide any APIs that do that, then TSVN has to guess.
> And no: specifying that paths/uris have to be "canonicalized" is not
> enough because I do that, using the svn APIs.
> So apparently that's not enough.
Stefan, there is nothing we can do with this information. If you have
found a value for which svn_foo_is_canonical(svn_foo_canonicalize(value))
returns false, please tell us what that value is.
Thanks.
Daniel
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On 12.12.2018 21:16, Stefan Kueng wrote:
>
>
> On 12.12.2018 21:12, Branko Čibej wrote:
>> On 12.12.2018 19:07, Stefan Kueng wrote:
>>>
>>>
>>> On 12.12.2018 13:55, TortoiseSVN-dev on behalf of Julian Foad wrote:
>>>>>> Subversion encountered a serious problem.
>>>>>> Please take the time to report this on the Subversion mailing list
>>>> […]
>>>>>> https://subversion.apache.org/mailing-lists.html
>>>>
>>>>> It is likely that this is a problem specific to TortoiseSVN, and not
>>>>> to core SVN. TortoiseSVN has its own mailinglists, so you should
>>>>> report your problem there:
>>>> (Cross-posting.)
>>>
>>> Since this happens in the project monitor, my best guess is that the
>>> path/url the user entered to be monitored is not correct.
>>>
>>>>
>>>> It makes me sad every time I see this pattern. Software is often
>>>> frustrating to use, but should at least aim to be polite to its
>>>> users. Telling the user "Please do X" and then when the user does X
>>>> saying "No, it's no good doing X; do Y" is not polite, and I would
>>>> not expect anyone but the most calm, patient and helpful of users to
>>>> gracefully comply with such a request.
>>>>
>>>> I'm not meaning to criticise Johan but rather our whole system.
>>>>
>>>> Can we please fix this problem. Both:
>>>> 1) Tsvn please change the message.
>>>
>>> Sorry, won't do that. Because I've argued multiple times over the
>>> years here that calling exit() or even abort() in a library is the
>>> worst idea ever. Especially if this can happen by having the user
>>> enter a wrong path/url.
>>
>>
>> It's not the user entering the wrong path or URL. It's the code that
>> uses the Subversion libraries — in this case TSVN — not validating and
>> de-tainting its input. Yes, this has been going on for years due to your
>
> And as I repeatedly said: TSVN does validate the input as good as it
> can. But if svn does neither describe the *exact* specs in the docs
> nor provide any APIs that do that, then TSVN has to guess.
> And no: specifying that paths/uris have to be "canonicalized" is not
> enough because I do that, using the svn APIs.
> So apparently that's not enough.
Get one of the dumps the crash reporter is supposed to generate, then
show us a stack trace that shows there's a bug in the Subversion code,
and you'll get results. Waxing philosophical about how you believe a
library should behave is not productive.
These silly "Subversion Exception" mails are no help at all, they
provide exactly *zero* information on which anyone can act. And your
refusal to direct TSVN users to TSVN support lists is just bloody
annoying and hence also not productive. If there is a bug in our code,
which of course is possible, we can do exactly nothing about it given
the amount of info we have.
Oh by the way, I doubt this had anything to do with user input, as the
OP states:
> got following exception at trying to start the tortoisesvn Project
> Monitor with a doubleclick on the tray icon.
-- Brane
P.S.: I keep wondering where these crash reports from all the other
Subversion clients out there are going. We don't seem to be seeing (m)any.
Re: Subversion Exception!
Posted by Stefan Kueng <to...@gmail.com>.
On 12.12.2018 21:12, Branko Čibej wrote:
> On 12.12.2018 19:07, Stefan Kueng wrote:
>>
>>
>> On 12.12.2018 13:55, TortoiseSVN-dev on behalf of Julian Foad wrote:
>>>>> Subversion encountered a serious problem.
>>>>> Please take the time to report this on the Subversion mailing list
>>> […]
>>>>> https://subversion.apache.org/mailing-lists.html
>>>
>>>> It is likely that this is a problem specific to TortoiseSVN, and not
>>>> to core SVN. TortoiseSVN has its own mailinglists, so you should
>>>> report your problem there:
>>> (Cross-posting.)
>>
>> Since this happens in the project monitor, my best guess is that the
>> path/url the user entered to be monitored is not correct.
>>
>>>
>>> It makes me sad every time I see this pattern. Software is often
>>> frustrating to use, but should at least aim to be polite to its
>>> users. Telling the user "Please do X" and then when the user does X
>>> saying "No, it's no good doing X; do Y" is not polite, and I would
>>> not expect anyone but the most calm, patient and helpful of users to
>>> gracefully comply with such a request.
>>>
>>> I'm not meaning to criticise Johan but rather our whole system.
>>>
>>> Can we please fix this problem. Both:
>>> 1) Tsvn please change the message.
>>
>> Sorry, won't do that. Because I've argued multiple times over the
>> years here that calling exit() or even abort() in a library is the
>> worst idea ever. Especially if this can happen by having the user
>> enter a wrong path/url.
>
>
> It's not the user entering the wrong path or URL. It's the code that
> uses the Subversion libraries — in this case TSVN — not validating and
> de-tainting its input. Yes, this has been going on for years due to your
And as I repeatedly said: TSVN does validate the input as good as it
can. But if svn does neither describe the *exact* specs in the docs nor
provide any APIs that do that, then TSVN has to guess.
And no: specifying that paths/uris have to be "canonicalized" is not
enough because I do that, using the svn APIs.
So apparently that's not enough.
> obstinately refusing to conform to our API specs. In the meantime,
> *your* users are left hanging.
I do conform to the specs.
> The rules are clear and consistent: pointers may not be NULL unless
> specifically allowed, paths must be absolute and canonical, URLs must be
> canonical, all strings must be encoded in UTF-8. We provide a wide range
> of helper functions that make it easy for API consumers to encode the
> parameters.
That's what I do.
>> Sorry if this message seems rude - but I'm tired of arguing the same
>> over and over again.
>
>
> You don't say.
I'll leave your sarcasm and won't respond to this thread anymore.
Stefan
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On 12.12.2018 19:07, Stefan Kueng wrote:
>
>
> On 12.12.2018 13:55, TortoiseSVN-dev on behalf of Julian Foad wrote:
>>>> Subversion encountered a serious problem.
>>>> Please take the time to report this on the Subversion mailing list
>> […]
>>>> https://subversion.apache.org/mailing-lists.html
>>
>>> It is likely that this is a problem specific to TortoiseSVN, and not
>>> to core SVN. TortoiseSVN has its own mailinglists, so you should
>>> report your problem there:
>> (Cross-posting.)
>
> Since this happens in the project monitor, my best guess is that the
> path/url the user entered to be monitored is not correct.
>
>>
>> It makes me sad every time I see this pattern. Software is often
>> frustrating to use, but should at least aim to be polite to its
>> users. Telling the user "Please do X" and then when the user does X
>> saying "No, it's no good doing X; do Y" is not polite, and I would
>> not expect anyone but the most calm, patient and helpful of users to
>> gracefully comply with such a request.
>>
>> I'm not meaning to criticise Johan but rather our whole system.
>>
>> Can we please fix this problem. Both:
>> 1) Tsvn please change the message.
>
> Sorry, won't do that. Because I've argued multiple times over the
> years here that calling exit() or even abort() in a library is the
> worst idea ever. Especially if this can happen by having the user
> enter a wrong path/url.
It's not the user entering the wrong path or URL. It's the code that
uses the Subversion libraries — in this case TSVN — not validating and
de-tainting its input. Yes, this has been going on for years due to your
obstinately refusing to conform to our API specs. In the meantime,
*your* users are left hanging.
The rules are clear and consistent: pointers may not be NULL unless
specifically allowed, paths must be absolute and canonical, URLs must be
canonical, all strings must be encoded in UTF-8. We provide a wide range
of helper functions that make it easy for API consumers to encode the
parameters.
> Sorry if this message seems rude - but I'm tired of arguing the same
> over and over again.
You don't say.
-- Brane
Re: Subversion Exception!
Posted by Branko Čibej <br...@apache.org>.
On 12.12.2018 19:07, Stefan Kueng wrote:
> On 12.12.2018 13:55, TortoiseSVN-dev on behalf of Julian Foad wrote:
>>>> Subversion encountered a serious problem.
>>>> Please take the time to report this on the Subversion mailing list
>> […]
>>>> https://subversion.apache.org/mailing-lists.html
>>
>>> It is likely that this is a problem specific to TortoiseSVN, and not
>>> to core SVN. TortoiseSVN has its own mailinglists, so you should
>>> report your problem there:
>> (Cross-posting.)
>
> Since this happens in the project monitor, my best guess is that the
> path/url the user entered to be monitored is not correct.
>
>>
>> It makes me sad every time I see this pattern. Software is often
>> frustrating to use, but should at least aim to be polite to its
>> users. Telling the user "Please do X" and then when the user does X
>> saying "No, it's no good doing X; do Y" is not polite, and I would
>> not expect anyone but the most calm, patient and helpful of users to
>> gracefully comply with such a request.
>>
>> I'm not meaning to criticise Johan but rather our whole system.
>>
>> Can we please fix this problem. Both:
>> 1) Tsvn please change the message.
>
> Sorry, won't do that. Because I've argued multiple times over the
> years here that calling exit() or even abort() in a library is the
> worst idea ever. Especially if this can happen by having the user
> enter a wrong path/url.
It's not the user entering the wrong path or URL. It's the code that
uses the Subversion libraries — in this case TSVN — not validating and
de-tainting its input. Yes, this has been going on for years due to your
obstinately refusing to conform to our API specs. In the meantime,
*your* users are left hanging.
The rules are clear and consistent: pointers may not be NULL unless
specifically allowed, paths must be absolute and canonical, URLs must be
canonical, all strings must be encoded in UTF-8. We provide a wide range
of helper functions that make it easy for API consumers to encode the
parameters.
> Sorry if this message seems rude - but I'm tired of arguing the same
> over and over again.
You don't say.
-- Brane
Re: Subversion Exception!
Posted by Stefan Kueng <to...@gmail.com>.
On 12.12.2018 13:55, TortoiseSVN-dev on behalf of Julian Foad wrote:
>>> Subversion encountered a serious problem.
>>> Please take the time to report this on the Subversion mailing list
> […]
>>> https://subversion.apache.org/mailing-lists.html
>
>> It is likely that this is a problem specific to TortoiseSVN, and not
>> to core SVN. TortoiseSVN has its own mailinglists, so you should
>> report your problem there:
> (Cross-posting.)
Since this happens in the project monitor, my best guess is that the
path/url the user entered to be monitored is not correct.
>
> It makes me sad every time I see this pattern. Software is often frustrating to use, but should at least aim to be polite to its users. Telling the user "Please do X" and then when the user does X saying "No, it's no good doing X; do Y" is not polite, and I would not expect anyone but the most calm, patient and helpful of users to gracefully comply with such a request.
>
> I'm not meaning to criticise Johan but rather our whole system.
>
> Can we please fix this problem. Both:
> 1) Tsvn please change the message.
Sorry, won't do that. Because I've argued multiple times over the years
here that calling exit() or even abort() in a library is the worst idea
ever. Especially if this can happen by having the user enter a wrong
path/url.
Just one of the many discussions I had here:
https://lists.apache.org/thread.html/924d8493ad71b9c428f4d575540b8688481f2ba1e00b70d58a421303@1433953117@%3Cdev.subversion.apache.org%3E
if svn_xxx_canonicalize() don't guarantee that a path/uri is correct and
won't throw an exception (or return an error), then how can I in an UI
client guarantee that before passing the user input to the library?
Since I can't (only the svn lib and specifically the API that consumes
the path/uri can know what exactly it expects), I made the dialog that
catches the abort() calls show exactly this message.
I figured that maybe if you get bored answering all the users bug
reports you might reconsider and finally not call abort() but return an
error message indicating which path is wrong.
It's like I said before: imagine an image lib that simply calls abort()
every time it tries to load a corrupted image. Now imagine photoshop or
paint.net or ... uses that lib: user has maybe 10 images open, tries to
load another one and the lib calls abort(). Now the user lost the other
10 images as well because abort() ends the application with NO
possibility to either save changes first.
Sorry if this message seems rude - but I'm tired of arguing the same
over and over again.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest interface to (Sub)version control
/_/ \_\ http://tortoisesvn.net
Re: Subversion Exception!
Posted by Stefan Kueng <to...@gmail.com>.
On 12.12.2018 13:55, TortoiseSVN-dev on behalf of Julian Foad wrote:
>>> Subversion encountered a serious problem.
>>> Please take the time to report this on the Subversion mailing list
> […]
>>> https://subversion.apache.org/mailing-lists.html
>
>> It is likely that this is a problem specific to TortoiseSVN, and not
>> to core SVN. TortoiseSVN has its own mailinglists, so you should
>> report your problem there:
> (Cross-posting.)
Since this happens in the project monitor, my best guess is that the
path/url the user entered to be monitored is not correct.
>
> It makes me sad every time I see this pattern. Software is often frustrating to use, but should at least aim to be polite to its users. Telling the user "Please do X" and then when the user does X saying "No, it's no good doing X; do Y" is not polite, and I would not expect anyone but the most calm, patient and helpful of users to gracefully comply with such a request.
>
> I'm not meaning to criticise Johan but rather our whole system.
>
> Can we please fix this problem. Both:
> 1) Tsvn please change the message.
Sorry, won't do that. Because I've argued multiple times over the years
here that calling exit() or even abort() in a library is the worst idea
ever. Especially if this can happen by having the user enter a wrong
path/url.
Just one of the many discussions I had here:
https://lists.apache.org/thread.html/924d8493ad71b9c428f4d575540b8688481f2ba1e00b70d58a421303@1433953117@%3Cdev.subversion.apache.org%3E
if svn_xxx_canonicalize() don't guarantee that a path/uri is correct and
won't throw an exception (or return an error), then how can I in an UI
client guarantee that before passing the user input to the library?
Since I can't (only the svn lib and specifically the API that consumes
the path/uri can know what exactly it expects), I made the dialog that
catches the abort() calls show exactly this message.
I figured that maybe if you get bored answering all the users bug
reports you might reconsider and finally not call abort() but return an
error message indicating which path is wrong.
It's like I said before: imagine an image lib that simply calls abort()
every time it tries to load a corrupted image. Now imagine photoshop or
paint.net or ... uses that lib: user has maybe 10 images open, tries to
load another one and the lib calls abort(). Now the user lost the other
10 images as well because abort() ends the application with NO
possibility to either save changes first.
Sorry if this message seems rude - but I'm tired of arguing the same
over and over again.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest interface to (Sub)version control
/_/ \_\ http://tortoisesvn.net
Re: Subversion Exception!
Posted by Julian Foad <ju...@foad.me.uk>.
>> Subversion encountered a serious problem.
>> Please take the time to report this on the Subversion mailing list
[…]
>> https://subversion.apache.org/mailing-lists.html
>It is likely that this is a problem specific to TortoiseSVN, and not
>to core SVN. TortoiseSVN has its own mailinglists, so you should
>report your problem there:
(Cross-posting.)
It makes me sad every time I see this pattern. Software is often frustrating to use, but should at least aim to be polite to its users. Telling the user "Please do X" and then when the user does X saying "No, it's no good doing X; do Y" is not polite, and I would not expect anyone but the most calm, patient and helpful of users to gracefully comply with such a request.
I'm not meaning to criticise Johan but rather our whole system.
Can we please fix this problem. Both:
1) Tsvn please change the message.
2) We should pass on the report to the appropriate place and thank the user for having helped us. We, the svn commumity, should not ask the user to do that: it's our problem; the user is the annoyed victim.
That would make me happier.
- Julian
Re: Subversion Exception!
Posted by Julian Foad <ju...@foad.me.uk>.
>> Subversion encountered a serious problem.
>> Please take the time to report this on the Subversion mailing list
[…]
>> https://subversion.apache.org/mailing-lists.html
>It is likely that this is a problem specific to TortoiseSVN, and not
>to core SVN. TortoiseSVN has its own mailinglists, so you should
>report your problem there:
(Cross-posting.)
It makes me sad every time I see this pattern. Software is often frustrating to use, but should at least aim to be polite to its users. Telling the user "Please do X" and then when the user does X saying "No, it's no good doing X; do Y" is not polite, and I would not expect anyone but the most calm, patient and helpful of users to gracefully comply with such a request.
I'm not meaning to criticise Johan but rather our whole system.
Can we please fix this problem. Both:
1) Tsvn please change the message.
2) We should pass on the report to the appropriate place and thank the user for having helped us. We, the svn commumity, should not ask the user to do that: it's our problem; the user is the annoyed victim.
That would make me happier.
- Julian