You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Lieven Govaerts <lg...@mobsol.be> on 2005/08/03 20:33:24 UTC

ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Hi, 

I encountered a situation this morning ( which I first mailed to the
users-list ) in which the xml generated by svn log --xml, and more
specific svn_client_log2, is invalid. 

Consequences are:
1. Validation of xml with the current DTD files fails.
2. JavaHL bindings will crash the Java VM when parsing the xml!

The situation is very specific, but easily reproduced with apache &
subversion.

I have a public repository at http://www.mobsol.be/svn/public, which is 
world readable. In that repository I have a branch branches/1.0, which
is private, and can only be read with correct credentials.

My svnaccess.conf:
...
[public:/]
* = r
@publicdev = rw

[public:/branches/1.0]
*=
@publicdev = rw
...

My apache is configured to first try anonymous access to the repository
for read-only actions: 
( extract of httpd subversion.conf )
...
<Limit  GET PROPFIND OPTIONS REPORT>
   Satisfy any
</Limit>
...
Require valid-user

This works without problem in most situations. 

But, when I request the log of this repository ( feel free to try ), I 
get:

# svn log http://www.mobsol.be/svn/public
...
------------------------------------------------------------------------
r14 | (no author) | (no date) | 1 line

------------------------------------------------------------------------
r13 | (no author) | (no date) | 1 line
...

Since the target of svn log is in the public zone, no credentials are
asked. Later, revision 13 & 14 alter files in the branches/1.0 folder,
which is private. Since no credentials were asked at first, svn log
cannot read these specific revisions.

This seems a bit tricky, but is understandable and probably correct
behaviour.

The two consequenses:
1. When asking for XML output, this results in logentries with undefined
author&date tags:
...
<logentry
   revision="14">
<msg></msg>
</logentry>
<logentry
   revision="13">
<msg></msg>
</logentry>
...
This is not allowed according to log.dtd, so I guess this is a problem.

I have no real suggestion how this should be solved since I don't know
how handling this situation is specified.

2. I made a Java application using the javahl bindings, doing the same
log request. Both on Windows as Linux this application will crash the 
Java VM.
   LogMessage[] lms = cl.logMessages("http://www.mobsol.be/svn/public",\
                                     null, null);
   System.out.println(lms.toString());
This happens in the MessageReceiver callback function in SVNClient.cpp.

Attached to this mail is a patch for SVNClient.cpp. The patch makes sure
that the date object returned to the Java client is 'null' in this
situation, by testing specifically for that situation. Other solution 
might be to fix function svn_time_from_cstring to return NULL when date
is NULL.

[[[

* subversion/bindings/java/javahl/native/SVNClient.cpp
  SVNClient::messageReceiver: Check if date of the revision is not NULL,
before trying to convert to the apr_time_t format. 

]]]

Hope this makes sense.

Lieven.


Re: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by Lieven Govaerts <lg...@mobsol.be>.
A bit embarrassing when you send your first patch to a dev-list, to
notice a few seconds later that you took the wrong file. 
Attached patch should be correct.

Sorry.

Lieven.

On Wed, 2005-08-03 at 22:33 +0200, Lieven Govaerts wrote:
> Hi, 
> 
> I encountered a situation this morning ( which I first mailed to the
> users-list ) in which the xml generated by svn log --xml, and more
> specific svn_client_log2, is invalid. 
> 
> Consequences are:
> 1. Validation of xml with the current DTD files fails.
> 2. JavaHL bindings will crash the Java VM when parsing the xml!
> 
> The situation is very specific, but easily reproduced with apache &
> subversion.
> 
> I have a public repository at http://www.mobsol.be/svn/public, which is 
> world readable. In that repository I have a branch branches/1.0, which
> is private, and can only be read with correct credentials.
> 
> My svnaccess.conf:
> ...
> [public:/]
> * = r
> @publicdev = rw
> 
> [public:/branches/1.0]
> *=
> @publicdev = rw
> ...
> 
> My apache is configured to first try anonymous access to the repository
> for read-only actions: 
> ( extract of httpd subversion.conf )
> ...
> <Limit  GET PROPFIND OPTIONS REPORT>
>    Satisfy any
> </Limit>
> ...
> Require valid-user
> 
> This works without problem in most situations. 
> 
> But, when I request the log of this repository ( feel free to try ), I 
> get:
> 
> # svn log http://www.mobsol.be/svn/public
> ...
> ------------------------------------------------------------------------
> r14 | (no author) | (no date) | 1 line
> 
> ------------------------------------------------------------------------
> r13 | (no author) | (no date) | 1 line
> ...
> 
> Since the target of svn log is in the public zone, no credentials are
> asked. Later, revision 13 & 14 alter files in the branches/1.0 folder,
> which is private. Since no credentials were asked at first, svn log
> cannot read these specific revisions.
> 
> This seems a bit tricky, but is understandable and probably correct
> behaviour.
> 
> The two consequenses:
> 1. When asking for XML output, this results in logentries with undefined
> author&date tags:
> ...
> <logentry
>    revision="14">
> <msg></msg>
> </logentry>
> <logentry
>    revision="13">
> <msg></msg>
> </logentry>
> ...
> This is not allowed according to log.dtd, so I guess this is a problem.
> 
> I have no real suggestion how this should be solved since I don't know
> how handling this situation is specified.
> 
> 2. I made a Java application using the javahl bindings, doing the same
> log request. Both on Windows as Linux this application will crash the 
> Java VM.
>    LogMessage[] lms = cl.logMessages("http://www.mobsol.be/svn/public",\
>                                      null, null);
>    System.out.println(lms.toString());
> This happens in the MessageReceiver callback function in SVNClient.cpp.
> 
> Attached to this mail is a patch for SVNClient.cpp. The patch makes sure
> that the date object returned to the Java client is 'null' in this
> situation, by testing specifically for that situation. Other solution 
> might be to fix function svn_time_from_cstring to return NULL when date
> is NULL.
> 
> [[[
> 
> * subversion/bindings/java/javahl/native/SVNClient.cpp
>   SVNClient::messageReceiver: Check if date of the revision is not NULL,
> before trying to convert to the apr_time_t format. 
> 
> ]]]
> 
> Hope this makes sense.
> 
> Lieven.
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org

RE: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 8 Aug 2005, Lieven Govaerts wrote:

> >> 2. I made a Java application using the javahl bindings, doing the same
> >> log request. Both on Windows as Linux this application will crash the
> >> Java VM.
> >>    LogMessage[] lms = cl.logMessages("http://www.mobsol.be/svn/public",\
> >>                                      null, null);
> >>    System.out.println(lms.toString());
> >> This happens in the MessageReceiver callback function in SVNClient.cpp.
> >>
> >> Attached to this mail is a patch for SVNClient.cpp. The patch makes
> >> sure that the date object returned to the Java client is 'null' in
> >> this situation, by testing specifically for that situation. Other
> >> solution might be to fix function svn_time_from_cstring to return NULL
> >> when date is NULL.
> >>
> >I know nothing about JavaHL, so I can't test the patch. By looking at it,
> it seems like you introduce the same bug I'm fixing in my upcoming commit,
> namely that an
> >empty date string should be treated as no date at all. It is strange, but
> it is documented in the API docs:-)
> >
> Actually, this patch does two things:
> 1. Check if the date in the log entry is empty; in that case avoid the call
> to svn_time_from_cstring.

No, it checks for the date being NULL, which is not the same as an empty
string. You need something like:
if (date != NULL && date[0] != '\0')
(you can get rid of the explicit comparisons if you like). That checks for
both a null date and an empty one. Read the docstring for
svn_log_msg_receiver_t and you'll see that either a NULL date or an empty
one will count as "no date". I have no clue to why it is that way, but you
can actually trigger it by setting the svn:date property to an empty
string (instead of deleting it).


> This makes it acting the same as for the empty author field, which is also
> null when returned to Java.
>
I haven't checked, but does it really special-case an *empty* author? I
think you mean NULL here.

> Actually, I don't know how you normally handle parameter validation ( is it
> the job of the caller? )
> but I would expect the function svn_time_from_cstring to return NULL when
> date with value NULL
> is passed to it.
>
It is normally the job of the caller to avoid programming errors like
passing invalid arguments. That's how we do it in svn; there are arguments
for and against it. For svn_time_from_cstring, if we added a check for a
null string, we would return an error. That would avoid the crash, but
you'd still not get the log you wanted. Better to fix the javahl bindings
then, which contain the real bug.

> If you'll update the DTD's, I'll also update the XSD files I created a few
> days ago, and post them again
> to this list.
>
See r15631.

I hope someone will take care of the JavaHL bug.

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

RE: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by Lieven Govaerts <lg...@mobsol.be>.
Peter,

-----Original Message-----
From: Peter N. Lundblad [mailto:peter@famlundblad.se] 
Sent: maandag 8 augustus 2005 16:42
To: Lieven Govaerts
Cc: dev@subversion.tigris.org
Subject: Re: ISSUE+PATCH: author & date of logentry can be empty in specific
situation, problems with log.dtd & javahl
...
>> 2. I made a Java application using the javahl bindings, doing the same 
>> log request. Both on Windows as Linux this application will crash the 
>> Java VM.
>>    LogMessage[] lms = cl.logMessages("http://www.mobsol.be/svn/public",\
>>                                      null, null);
>>    System.out.println(lms.toString());
>> This happens in the MessageReceiver callback function in SVNClient.cpp.
>>
>> Attached to this mail is a patch for SVNClient.cpp. The patch makes 
>> sure that the date object returned to the Java client is 'null' in 
>> this situation, by testing specifically for that situation. Other 
>> solution might be to fix function svn_time_from_cstring to return NULL 
>> when date is NULL.
>>
>I know nothing about JavaHL, so I can't test the patch. By looking at it,
it seems like you introduce the same bug I'm fixing in my upcoming commit,
namely that an 
>empty date string should be treated as no date at all. It is strange, but
it is documented in the API docs:-)
>
>Thanks for the report,
>//Peter 

Actually, this patch does two things:
1. Check if the date in the log entry is empty; in that case avoid the call
to svn_time_from_cstring.
It's that function that causes the VM to crash. 
2. Instead of returning an uninitalized date object, which in Java results
in something random date,
return NULL . 

This makes it acting the same as for the empty author field, which is also
null when returned to Java. 

Actually, I don't know how you normally handle parameter validation ( is it
the job of the caller? )
but I would expect the function svn_time_from_cstring to return NULL when
date with value NULL
is passed to it.

If you'll update the DTD's, I'll also update the XSD files I created a few
days ago, and post them again
to this list. 

You're welcome on the report, had to patch it anyway otherwise my reporting
application didn't work
on our production repository. I hope this or a better patch makes it in the
next SVN release.

regards,

Lieven.





-- 
No virus found in this outgoing message.
Checked by AVG Anti-Virus.
Version: 7.0.338 / Virus Database: 267.10.2/65 - Release Date: 7/08/2005
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 3 Aug 2005, Lieven Govaerts wrote:

> I encountered a situation this morning ( which I first mailed to the
> users-list ) in which the xml generated by svn log --xml, and more
> specific svn_client_log2, is invalid.
>
> Consequences are:
> 1. Validation of xml with the current DTD files fails.
> 2. JavaHL bindings will crash the Java VM when parsing the xml!
>
...
> But, when I request the log of this repository ( feel free to try ), I
> get:
>
> # svn log http://www.mobsol.be/svn/public
> ...
> ------------------------------------------------------------------------
> r14 | (no author) | (no date) | 1 line
>
> ------------------------------------------------------------------------
> r13 | (no author) | (no date) | 1 line
> ...
>
I'm addressing the problem of invalid XML here. As you can see from the
above, it is perfectly valid to have no author or no date (these
properties are not mandatory on a revision). I think the current DTD is
buggy in that it should leave author and date optional. I've a patch ready
that will fix this (and a bug where an empty date will cause an emtpy date
element, which I also consider as invalid even if the DTD can't validate
it.)

> The two consequenses:
> 1. When asking for XML output, this results in logentries with undefined
> author&date tags:
> ...
> <logentry
>    revision="14">
> <msg></msg>
> </logentry>
> <logentry
>    revision="13">
> <msg></msg>
> </logentry>
> ...
> This is not allowed according to log.dtd, so I guess this is a problem.
>
> I have no real suggestion how this should be solved since I don't know
> how handling this situation is specified.
>
ONe could argue that fixing the DTD breaks compatibility, but since that's
how the client has always behaved, it's not a problem according to me.


> 2. I made a Java application using the javahl bindings, doing the same
> log request. Both on Windows as Linux this application will crash the
> Java VM.
>    LogMessage[] lms = cl.logMessages("http://www.mobsol.be/svn/public",\
>                                      null, null);
>    System.out.println(lms.toString());
> This happens in the MessageReceiver callback function in SVNClient.cpp.
>
> Attached to this mail is a patch for SVNClient.cpp. The patch makes sure
> that the date object returned to the Java client is 'null' in this
> situation, by testing specifically for that situation. Other solution
> might be to fix function svn_time_from_cstring to return NULL when date
> is NULL.
>
I know nothing about JavaHL, so I can't test the patch. By looking at it,
it seems like you introduce the same bug I'm fixing in my upcoming commit,
namely that an empty date string should be treated as no date at all. It
is strange, but it is documented in the API docs:-)

Thanks for the report,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Wed, 17 Aug 2005, Branko ??ibej wrote:

> kfogel@collab.net wrote:
...
> >So what's the right thing -- send back empty XML elements, or omit
> >them entirely, changing the DTD to allow this?
> > 
> >
> Omit them, of course. Revprops can have empty values.

omit them, +1

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:

>Michael W Thelen <mi...@pietdepsi.com> writes:
>  
>
>>kfogel@collab.net wrote:
>>    
>>
>>>>Since the target of svn log is in the public zone, no credentials are
>>>>asked. Later, revision 13 & 14 alter files in the branches/1.0 folder,
>>>>which is private. Since no credentials were asked at first, svn log
>>>>cannot read these specific revisions.
>>>>        
>>>>
>>>I don't quite understand this explanation.
>>>
>>>Whether the target of 'svn log' requires credentials or not shouldn't
>>>matter.  The question is whether the *commit* (the one that resulted
>>>in the revisions being displayed) required credentials.
>>>      
>>>
>>I believe this is the same issue I posted about in this message:
>>
>>http://svn.haxx.se/dev/archive-2005-08/0286.shtml
>>
>>... see Ben Collins-Sussman's response to that message for an
>>explanation of what's going on.
>>    
>>
>
>Aaaaah, thank you.  Yes, now I understand.
>
>So what's the right thing -- send back empty XML elements, or omit
>them entirely, changing the DTD to allow this?
>  
>
Omit them, of course. Revprops can have empty values.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by kf...@collab.net.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:
> Omit them. See r15631 for one way of doing it:-) (it's fun to be catching
> up, I know...:-)

Sigh :-).

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 17 Aug 2005 kfogel@collab.net wrote:

> Michael W Thelen <mi...@pietdepsi.com> writes:
> > kfogel@collab.net wrote:
> > >>Since the target of svn log is in the public zone, no credentials are
> > >>asked. Later, revision 13 & 14 alter files in the branches/1.0 folder,
> > >>which is private. Since no credentials were asked at first, svn log
> > >>cannot read these specific revisions.
> > >
> > > I don't quite understand this explanation.
> > >
> > > Whether the target of 'svn log' requires credentials or not shouldn't
> > > matter.  The question is whether the *commit* (the one that resulted
> > > in the revisions being displayed) required credentials.
> >
> > I believe this is the same issue I posted about in this message:
> >
> > http://svn.haxx.se/dev/archive-2005-08/0286.shtml
> >
> > ... see Ben Collins-Sussman's response to that message for an
> > explanation of what's going on.
>
> Aaaaah, thank you.  Yes, now I understand.
>
> So what's the right thing -- send back empty XML elements, or omit
> them entirely, changing the DTD to allow this?
>
Omit them. See r15631 for one way of doing it:-) (it's fun to be catching
up, I know...:-)

Also, if that hasn't been pointed out, if all paths in a revision are
unreadable, *all* revprops are cleared (including the date) are stripped.
See send_change_rev() in libsvn_repos/log.c

regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by kf...@collab.net.
Michael W Thelen <mi...@pietdepsi.com> writes:
> kfogel@collab.net wrote:
> >>Since the target of svn log is in the public zone, no credentials are
> >>asked. Later, revision 13 & 14 alter files in the branches/1.0 folder,
> >>which is private. Since no credentials were asked at first, svn log
> >>cannot read these specific revisions.
> > 
> > I don't quite understand this explanation.
> > 
> > Whether the target of 'svn log' requires credentials or not shouldn't
> > matter.  The question is whether the *commit* (the one that resulted
> > in the revisions being displayed) required credentials.
> 
> I believe this is the same issue I posted about in this message:
> 
> http://svn.haxx.se/dev/archive-2005-08/0286.shtml
> 
> ... see Ben Collins-Sussman's response to that message for an
> explanation of what's going on.

Aaaaah, thank you.  Yes, now I understand.

So what's the right thing -- send back empty XML elements, or omit
them entirely, changing the DTD to allow this?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by Michael W Thelen <mi...@pietdepsi.com>.
kfogel@collab.net wrote:
>>Since the target of svn log is in the public zone, no credentials are
>>asked. Later, revision 13 & 14 alter files in the branches/1.0 folder,
>>which is private. Since no credentials were asked at first, svn log
>>cannot read these specific revisions.
> 
> I don't quite understand this explanation.
> 
> Whether the target of 'svn log' requires credentials or not shouldn't
> matter.  The question is whether the *commit* (the one that resulted
> in the revisions being displayed) required credentials.

I believe this is the same issue I posted about in this message:

http://svn.haxx.se/dev/archive-2005-08/0286.shtml

... see Ben Collins-Sussman's response to that message for an
explanation of what's going on.

-- 
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes.       -- Douglas Adams

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: ISSUE+PATCH: author & date of logentry can be empty in specific situation, problems with log.dtd & javahl

Posted by kf...@collab.net.
Lieven Govaerts <lg...@mobsol.be> writes:
> I encountered a situation this morning ( which I first mailed to the
> users-list ) in which the xml generated by svn log --xml, and more
> specific svn_client_log2, is invalid. 
> 
> Consequences are:
> 1. Validation of xml with the current DTD files fails.
> 2. JavaHL bindings will crash the Java VM when parsing the xml!
> 
> The situation is very specific, but easily reproduced with apache &
> subversion.
> 
> I have a public repository at http://www.mobsol.be/svn/public, which is 
> world readable. In that repository I have a branch branches/1.0, which
> is private, and can only be read with correct credentials.
> 
> My svnaccess.conf:
> ...
> [public:/]
> * = r
> @publicdev = rw
> 
> [public:/branches/1.0]
> *=
> @publicdev = rw
> ...
> 
> My apache is configured to first try anonymous access to the repository
> for read-only actions: 
> ( extract of httpd subversion.conf )
> ...
> <Limit  GET PROPFIND OPTIONS REPORT>
>    Satisfy any
> </Limit>
> ...
> Require valid-user
> 
> This works without problem in most situations. 
> 
> But, when I request the log of this repository ( feel free to try ), I 
> get:
> 
> # svn log http://www.mobsol.be/svn/public
> ...
> ------------------------------------------------------------------------
> r14 | (no author) | (no date) | 1 line
> 
> ------------------------------------------------------------------------
> r13 | (no author) | (no date) | 1 line
> ...
> 
> Since the target of svn log is in the public zone, no credentials are
> asked. Later, revision 13 & 14 alter files in the branches/1.0 folder,
> which is private. Since no credentials were asked at first, svn log
> cannot read these specific revisions.

I don't quite understand this explanation.

Whether the target of 'svn log' requires credentials or not shouldn't
matter.  The question is whether the *commit* (the one that resulted
in the revisions being displayed) required credentials.

In your last paragraph above, you seem to be saying that r13 and r14
alter files in a "private" folder, which I presume means a folder that
would require credentials for committing inside.  Therefore, there
should be non-empty authors on those revisions.

Furthermore, regardless of whether authn credentials were submitted or
not, every revision should have a date.  Thus, the only way I can
understand the output above is if someone went in and *deleted* the
'svn:date' revision property, and perhaps the 'svn:author' revprop as
well.

What's going on here? :-)

> This seems a bit tricky, but is understandable and probably correct
> behaviour.

As I understand it so far, it is not correct, or at least it is very
suspicious.

> The two consequenses:
> 1. When asking for XML output, this results in logentries with undefined
> author&date tags:
> ...
> <logentry
>    revision="14">
> <msg></msg>
> </logentry>
> <logentry
>    revision="13">
> <msg></msg>
> </logentry>
> ...
> This is not allowed according to log.dtd, so I guess this is a problem.

They don't appear to be "undefined", they appear to be "absent"! :-)

We could just include them, with empty content.  But first I'd like to
know how the repository got into this situation in the first place.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org