You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2005/12/02 23:05:38 UTC
Re: Fixed length character buffers?
Erik Huelsmann <eh...@gmail.com> writes:
> > + char keyword_buf[SVN_KEYWORD_MAX_LEN];
> > + apr_size_t keyword_off;
> > + char src_format[2];
>
> The segment above is from my last commit creating the eol-recoding stream.
>
> Here, and some other places in our code, we still use fixed length
> character buffers. I understand the case where we use them for MD5
> sums: APR expects us to.
>
> In the above case, nobody *wants* us to use a fixed character buffer.
> Using stringbufs, the above character limit (SVN_KEYWORD_MAX_LEN) can
> easily be lifted, if we want to.
>
> Also, I consider it not very future proof to build a 1.3 client with
> that constant: if later clients have a higher value for that
> constant...
Well, we control the keywords, so the only potential problem here is
that we might one day make a keyword longer than 255 characters and
forget to update SVN_KEYWORD_MAX_LEN...
...oh, wait: under some circumstances, that length has to include both
the keyword and its value. For example, see the strncmp() that
appears early in libsvn_subr/subst.c:translate_keyword_subst().
Do we have a problem right now? Have we just been lucky that no one's
had, say, a really long URL as the value of the $URL$ keyword?
-Karl
--
www.collab.net <> CollabNet | Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixed length character buffers?
Posted by kf...@collab.net.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> On 02 Dec 2005 17:05:38 -0600, kfogel@collab.net <kf...@collab.net> wrote:
>
> > ...oh, wait: under some circumstances, that length has to include both
> > the keyword and its value. For example, see the strncmp() that
> > appears early in libsvn_subr/subst.c:translate_keyword_subst().
> >
> > Do we have a problem right now? Have we just been lucky that no one's
> > had, say, a really long URL as the value of the $URL$ keyword?
>
> No, we haven't been lucky, someone submitted a bug report about that a
> while back, there was a buffer overflow in that case.
And r17149 appears to have fixed it (thanks, Philip). I'm going over
that rev now, and doing some other explorations, just to make sure.
-Karl
--
www.collab.net <> CollabNet | Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixed length character buffers?
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 02 Dec 2005 17:05:38 -0600, kfogel@collab.net <kf...@collab.net> wrote:
> ...oh, wait: under some circumstances, that length has to include both
> the keyword and its value. For example, see the strncmp() that
> appears early in libsvn_subr/subst.c:translate_keyword_subst().
>
> Do we have a problem right now? Have we just been lucky that no one's
> had, say, a really long URL as the value of the $URL$ keyword?
No, we haven't been lucky, someone submitted a bug report about that a
while back, there was a buffer overflow in that case.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixed length character buffers?
Posted by kf...@collab.net.
Erik Huelsmann <eh...@gmail.com> writes:
> I see Garret found an issue on this. Why should we limit the length of
> the value? Anyway, I'd be willing to make this instance (and others?)
> use stringbufs...
I agree. And even if the code is safe right now, it's brittle -- it's
too easy for it to become insecure when modified. +1 on stringbufs.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixed length character buffers?
Posted by Erik Huelsmann <eh...@gmail.com>.
On 02 Dec 2005 17:05:38 -0600, kfogel@collab.net <kf...@collab.net> wrote:
> Erik Huelsmann <eh...@gmail.com> writes:
> > > + char keyword_buf[SVN_KEYWORD_MAX_LEN];
> > > + apr_size_t keyword_off;
> > > + char src_format[2];
> >
> > The segment above is from my last commit creating the eol-recoding stream.
> >
> > Here, and some other places in our code, we still use fixed length
> > character buffers. I understand the case where we use them for MD5
> > sums: APR expects us to.
> >
> > In the above case, nobody *wants* us to use a fixed character buffer.
> > Using stringbufs, the above character limit (SVN_KEYWORD_MAX_LEN) can
> > easily be lifted, if we want to.
> >
> > Also, I consider it not very future proof to build a 1.3 client with
> > that constant: if later clients have a higher value for that
> > constant...
>
> Well, we control the keywords, so the only potential problem here is
> that we might one day make a keyword longer than 255 characters and
> forget to update SVN_KEYWORD_MAX_LEN...
We control the keywords, but not for long anymore: large portions of
our code have been adapted for a feature called 'Custom keywords',
which implies, well... that we don't control them....
> ...oh, wait: under some circumstances, that length has to include both
> the keyword and its value. For example, see the strncmp() that
> appears early in libsvn_subr/subst.c:translate_keyword_subst().
>
> Do we have a problem right now? Have we just been lucky that no one's
> had, say, a really long URL as the value of the $URL$ keyword?
I see Garret found an issue on this. Why should we limit the length of
the value? Anyway, I'd be willing to make this instance (and others?)
use stringbufs...
bye,
Erik.
Re: Fixed length character buffers?
Posted by John Peacock <jp...@rowman.com>.
Peter N. Lundblad wrote:
> I added a file with an URL that's longer than 255 characters. It was just
> truncated, so we have a problem for people with very long paths...
Problem in the sense that $URL$ was not the entire file path, not in the sense
that it took over their machine and started sending cyborgs back in time. ;-)
> I'm not completely sure what happens with the translation routines fi we
> just lift this limit, though.
SVN_KEYWORD_MAX_LEN predates the keywords-as-hash code. There shouldn't be any
reason that it has to stay that way; AFAICT, this was just a "we'll never need
to exceed this for a single line" assumption (and we know what that leads to ;).
Since I know the keywords code better than most, I'll try and take a walk down
memory lane and see if there is any reason why we cannot drop the fixed length
limit. If Erik wants to rewrite this code to use stringbufs instead, more power
to him...
John
--
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4720 Boston Way
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5747
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixed length character buffers?
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sat, 2 Dec 2005 kfogel@collab.net wrote:
> Erik Huelsmann <eh...@gmail.com> writes:
> Do we have a problem right now? Have we just been lucky that no one's
> had, say, a really long URL as the value of the $URL$ keyword?
>
I added a file with an URL that's longer than 255 characters. It was just
truncated, so we have a problem for people with very long paths...
I'm not completely sure what happens with the translation routines fi we
just lift this limit, though. Maybe there is no problem, just want to
have said that:-) What happens if a file has a very long line with a
single $ sign in the beginning? ... Corner case? Maybe, but the
fixed-sized keywords are meant for binary files.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org