Subversion Exception!

classic Classic list List threaded Threaded
37 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Subversion Exception!

Peta Miller

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  
---------------------------

 

 

Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Johan Corveleyn-3
On Mon, Dec 10, 2018 at 9:47 AM Peta Miller <[hidden email]> 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 [hidden email] -- 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
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Stefan Kueng


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
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Branko Čibej
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

Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Stefan Kueng


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
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Branko Čibej
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.

Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Daniel Shahaf-2
In reply to this post by Stefan Kueng
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
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Johan Corveleyn-3
On Wed, Dec 12, 2018 at 10:23 PM Daniel Shahaf <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Branko Čibej
On 13.12.2018 14:15, Johan Corveleyn wrote:

> On Wed, Dec 12, 2018 at 10:23 PM Daniel Shahaf <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Julian Foad-5
In reply to this post by Johan Corveleyn-3
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

assert-dirent-absolute-1.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Johan Corveleyn-3
On Thu, Dec 13, 2018 at 4:05 PM Julian Foad <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Branko Čibej
On 13.12.2018 16:42, Johan Corveleyn wrote:

> On Thu, Dec 13, 2018 at 4:05 PM Julian Foad <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Julian Foad-5
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
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Michael Pilato
In reply to this post by Branko Čibej
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."

;-)
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Branko Čibej
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

Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Mark Phippard-3
In reply to this post by Michael Pilato

> On Dec 13, 2018, at 10:53 AM, Michael Pilato <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Daniel Shahaf-2
In reply to this post by Branko Čibej
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
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Branko Čibej
In reply to this post by Mark Phippard-3
On 13.12.2018 17:09, Mark Phippard wrote:

>> On Dec 13, 2018, at 10:53 AM, Michael Pilato <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Julian Foad-5
In reply to this post by Daniel Shahaf-2
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
Reply | Threaded
Open this post in threaded view
|

Re: Subversion Exception!

Daniel Shahaf-2
In reply to this post by Mark Phippard-3
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
12