Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-21 Thread Philip Martin
"C. Michael Pilato"  writes:

> Now, if we can find a way to expose the revprops to the start-commit hook,
> all three RA layers should be able to provide early detection of commits
> that would fail in pre-commit for revprop-related reasons.  (And we should
> consider this independently of the whole client-version capability stuff.)

Perhaps we need another hook that runs immediately after the transaction
is created?

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download


having revprops (eg svn:log) in start-commit Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-20 Thread Daniel Shahaf
[ Nice work indeed, but I'm here only to change the subject to make it
easier to notice. ]

C. Michael Pilato wrote on Mon, Aug 20, 2012 at 15:15:05 -0400:
> On 08/20/2012 11:02 AM, C. Michael Pilato wrote:
> > On 08/20/2012 10:36 AM, C. Michael Pilato wrote:
> >> [1] mod_dav_svn is an exception here, because the log message is
> >> PROPPATCHed into the transaction after it's creation.  start-commit
> >> hooks would have to assume that a non-existent log message is simply
> >> suffering delayed arrival, and their pre-commit hook will have to
> >> continue doing this validation just as it does today.
> > 
> > Actually, it occurs to me that HTTPv2 provides just the fix for this!
> > Rather than send an empty POST followed by the PROPPATCH for the log message
> > (and user revprops), couldn't we embed all those revprops in the POST body
> > itself?!  Might have just found a way to drop yet another network request
> > from commits over HTTP!
> 
> See r1375123 and r1375167.
> 
> Now, if we can find a way to expose the revprops to the start-commit hook,
> all three RA layers should be able to provide early detection of commits
> that would fail in pre-commit for revprop-related reasons.  (And we should
> consider this independently of the whole client-version capability stuff.)
> 
> -- 
> C. Michael Pilato 
> CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development
> 




Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-20 Thread Daniel Shahaf
Mark Moe wrote on Mon, Aug 20, 2012 at 14:38:43 -0500:
> > Now, if we can find a way to expose the revprops to the start-commit hook,
> > all three RA layers should be able to provide early detection of commits
> > that would fail in pre-commit for revprop-related reasons.  (And we should
> > consider this independently of the whole client-version capability stuff.)
> 
> The ideal solution (for me) would be a start-commit hook that could
> query a temporary transaction that had everything (except actual file
> content) just like pre-commit can.

Mike covered the details, but the high-level is this: start-commit
_cannot_ have access to all the data that pre-commit can, for the simple
reason that the server receives some data from the client after
start-commit but before pre-commit.  (And, naturally, if it weren't that
way, we would deprecate one of the hooks and subsume it into the other)

Specifically, start-commit is before the client uplods all the file
contents modifications.


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-20 Thread C. Michael Pilato
On 08/20/2012 03:38 PM, Mark Moe wrote:
>> Now, if we can find a way to expose the revprops to the start-commit hook,
>> all three RA layers should be able to provide early detection of commits
>> that would fail in pre-commit for revprop-related reasons.  (And we should
>> consider this independently of the whole client-version capability stuff.)
> 
> The ideal solution (for me) would be a start-commit hook that could
> query a temporary transaction that had everything (except actual file
> content) just like pre-commit can.  That way I could use svnlook -t to
> block based on snv:log and/or the list of incoming changed files.
> Doing all of those blocks that I do now with pre-commit but before
> sending the actual file content would be great!

I agree -- having access to the freshly created txn from within start-commit
would provide the most natural interface for examining the metadata of the
commit in terms of what's in the revprops.

Now, currently the server doesn't know what files are going to be changed in
a commit until they've actually been changed.  There's been talk, though,
about having the commit process pre-announce the to-be-modified files in the
new Editor v2 system.  If we go that route -- and respecting the existing
definition of 'start-commit' as "before the transaction is even made" --
perhaps we could add a new hook which is post-txn-create, post-revprops-set,
post-modified-path-announcement, but still
pre-all-the-real-commit-data-getting-sent-over-the-wire.

> This is a bit different but related.  I currently also block really
> big commits by looking at the transaction size on disk.  It would also
> be nice to get an idea of the transaction size too before the entire
> transaction content has been sent.  However, I don't know if the
> client knows this size before the transaction content is sent to the
> server.

Nope.  No idea of the transaction size in advance of the commit.  But it
certainly makes sense as a bit of information that an admin might want to
use to feed a commit allow/deny decision.

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


RE: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-20 Thread Mark Moe
> Now, if we can find a way to expose the revprops to the start-commit hook,
> all three RA layers should be able to provide early detection of commits
> that would fail in pre-commit for revprop-related reasons.  (And we should
> consider this independently of the whole client-version capability stuff.)

The ideal solution (for me) would be a start-commit hook that could
query a temporary transaction that had everything (except actual file
content) just like pre-commit can.  That way I could use svnlook -t to
block based on snv:log and/or the list of incoming changed files.
Doing all of those blocks that I do now with pre-commit but before
sending the actual file content would be great!

This is a bit different but related.  I currently also block really
big commits by looking at the transaction size on disk.  It would also
be nice to get an idea of the transaction size too before the entire
transaction content has been sent.  However, I don't know if the
client knows this size before the transaction content is sent to the
server.

Thanks,

- Mark


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-20 Thread C. Michael Pilato
On 08/20/2012 11:02 AM, C. Michael Pilato wrote:
> On 08/20/2012 10:36 AM, C. Michael Pilato wrote:
>> [1] mod_dav_svn is an exception here, because the log message is
>> PROPPATCHed into the transaction after it's creation.  start-commit
>> hooks would have to assume that a non-existent log message is simply
>> suffering delayed arrival, and their pre-commit hook will have to
>> continue doing this validation just as it does today.
> 
> Actually, it occurs to me that HTTPv2 provides just the fix for this!
> Rather than send an empty POST followed by the PROPPATCH for the log message
> (and user revprops), couldn't we embed all those revprops in the POST body
> itself?!  Might have just found a way to drop yet another network request
> from commits over HTTP!

See r1375123 and r1375167.

Now, if we can find a way to expose the revprops to the start-commit hook,
all three RA layers should be able to provide early detection of commits
that would fail in pre-commit for revprop-related reasons.  (And we should
consider this independently of the whole client-version capability stuff.)

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


RE: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-20 Thread Mark Moe
> ALSO:  I got a private mail response to my previous post on this thread
> which expressed a good deal of excitement regarding the possibility of
> start-commit having access to the revprops (especially svn:log).

That would be very useful.  We have transactions that can be 1GB in
size or more and blocking them for poor commit messages sooner rather
than later would be nice!

Thanks,

- Mark


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-20 Thread C. Michael Pilato
On 08/20/2012 10:36 AM, C. Michael Pilato wrote:
> [1] mod_dav_svn is an exception here, because the log message is
> PROPPATCHed into the transaction after it's creation.  start-commit
> hooks would have to assume that a non-existent log message is simply
> suffering delayed arrival, and their pre-commit hook will have to
> continue doing this validation just as it does today.

Actually, it occurs to me that HTTPv2 provides just the fix for this!
Rather than send an empty POST followed by the PROPPATCH for the log message
(and user revprops), couldn't we embed all those revprops in the POST body
itself?!  Might have just found a way to drop yet another network request
from commits over HTTP!

ALSO:  I got a private mail response to my previous post on this thread
which expressed a good deal of excitement regarding the possibility of
start-commit having access to the revprops (especially svn:log).  Apparently
this long-standing frustration is more frustrating than I even realized.
Something to consider.

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-20 Thread C. Michael Pilato
On 08/17/2012 12:14 PM, C. Michael Pilato wrote:
> Well, I just ran into my first hurdle:  the current svnserve protocol
> demands that capabilities in the standard capabilities string list be of the
> protocol data class "word", which is limited to strings with letters,
> numbers, and hyphens only.

Okay, how about an altogether different approach that just occurred to me.

  * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * *
  *  WARNING:  I WILL REFER TO THE USE OF PROPERTIES BELOW.  THOSE  *
  *WITH HEIGHTENED SENSITIVITIES SHOULD TURN BACK NOW.  *
  * *
  * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *

What if instead of the capabilities mechanism, we use txnprops?

The start-commit hook wrapper logic already has access to a set of txnprops
destined for the commit transaction which will be created pending successful
completion of the hook.  But the props aren't passed to the hook today.
They aren't really accessible to the hook as txnprops either because the
hook runs *before* the commit txn is created.

So, picture this crazyiness:

1. The RA layer drops the client version info we've been talking about into
   txnprops ("svn:client-name", "svn:client-version", and
   "svn:client-compat-version") along with svn:log and the user-provided
   txnprops ('svn ci --with-revprop') and hands that off to the run-the-
   start-commit-hook-and-then-make-a-txn function.

2. At this point we either:

   a. serialize the revprops-to-be and hand them off to the start-commit
  hook somehow (hands waving frantically), or

   b. change the nature of the start-commit hook such that it runs
  immediately *after* the txn is created, gets handed the txn-name so
  that txnprops can be examined, and simply aborts the txn if the
  start-commit hook fails.  (Does this create a race condition of
  txn existence that we don't want?)

3. After successful completion of the start-commit hook, we can then either:

   a. remove the magic txnprops, or

   b. just leave them to serve as a record of the client information
  forever stored in the repository and associated with the commit.

4. As a bonus side-effect, users can now deny commits with substandard log
   messages *before* the whole commit comes across the wire, which they
   can't do today.[1]

It's Monday.  My ceiling was leaking water over the weekend.  I have two
extra children in my house this morning.  [Insert additional excuses.]

Discuss.

-- C-Mike

[1] mod_dav_svn is an exception here, because the log message is
PROPPATCHed into the transaction after it's creation.  start-commit
hooks would have to assume that a non-existent log message is simply
suffering delayed arrival, and their pre-commit hook will have to
continue doing this validation just as it does today.

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-20 Thread C. Michael Pilato
On 08/19/2012 12:21 PM, Daniel Shahaf wrote:
> C. Michael Pilato wrote on Fri, Aug 17, 2012 at 14:01:03 -0400:
>> On 08/17/2012 12:43 PM, Daniel Shahaf wrote:
>>> Stefan Sperling wrote on Fri, Aug 17, 2012 at 18:24:03 +0200:
 On Fri, Aug 17, 2012 at 12:14:31PM -0400, C. Michael Pilato wrote:
>  4. something else?

 4. Marshall the version string before sending it across svn://,
escaping unsupported characters somehow in a reversible way,
and unescape them before passing them to hooks?
I.e. use something like strnvis() but adapted to the
restrictions of the svn:// protocol.
>>
>> Yeah, I forgot to add that I was thinking about this approach, too.  I'm not
>> familiar with strnvis(), but I assume it's similar to what we do with our
>> checksum API _readable() variants (where 'A' is 0x41 and represented as as
> 
> I don't see any public or interlibrary API with /_readable/ in its name?
> 

Sorry, I misremembered the names.  "_display" was the suffix I was seeking,
as in svn_checksum_to_cstring_display().

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-19 Thread Daniel Shahaf
Branko Čibej wrote on Fri, Aug 17, 2012 at 20:07:34 +0200:
> On 17.08.2012 20:01, C. Michael Pilato wrote:
> > On 08/17/2012 12:43 PM, Daniel Shahaf wrote:
> >> Stefan Sperling wrote on Fri, Aug 17, 2012 at 18:24:03 +0200:
> >>> On Fri, Aug 17, 2012 at 12:14:31PM -0400, C. Michael Pilato wrote:
>   4. something else?
> >>> 4. Marshall the version string before sending it across svn://,
> >>>escaping unsupported characters somehow in a reversible way,
> >>>and unescape them before passing them to hooks?
> >>>I.e. use something like strnvis() but adapted to the
> >>>restrictions of the svn:// protocol.
> > Yeah, I forgot to add that I was thinking about this approach, too.  I'm not
> > familiar with strnvis(), but I assume it's similar to what we do with our
> > checksum API _readable() variants (where 'A' is 0x41 and represented as as
> > "41")?
> >
> >> 5. Require the version string and client name to match
> >>/^[A-Za-z0-9-]+$/, and embed them directly as part of a capability
> >>name (so: capabilities might be named client-version-tsvn-1dot7dot0)
> > I'll give the honor of treating this suggestion as a serious one.  And then
> > the dishonor of a hearty -1.  :-)
> 
> Why? It's essentially the same as 4., just with a different and
> higher-level marshalling.

For the record, I don't really object to the -1... the suggestion
violates layering and hardcodes restriction where they don't quite
belong.  Its advantage is --- like stsp's suggestion --- not requiring a
protocol version bump.

And then there is ghudson's point, of course.


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-19 Thread Daniel Shahaf
C. Michael Pilato wrote on Fri, Aug 17, 2012 at 14:01:03 -0400:
> On 08/17/2012 12:43 PM, Daniel Shahaf wrote:
> > Stefan Sperling wrote on Fri, Aug 17, 2012 at 18:24:03 +0200:
> >> On Fri, Aug 17, 2012 at 12:14:31PM -0400, C. Michael Pilato wrote:
> >>>  4. something else?
> >>
> >> 4. Marshall the version string before sending it across svn://,
> >>escaping unsupported characters somehow in a reversible way,
> >>and unescape them before passing them to hooks?
> >>I.e. use something like strnvis() but adapted to the
> >>restrictions of the svn:// protocol.
> 
> Yeah, I forgot to add that I was thinking about this approach, too.  I'm not
> familiar with strnvis(), but I assume it's similar to what we do with our
> checksum API _readable() variants (where 'A' is 0x41 and represented as as

I don't see any public or interlibrary API with /_readable/ in its name?


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread Ben Reser
On Fri, Aug 17, 2012 at 6:23 PM, Ben Reser  wrote:
> If there are behaviors of clients in the wild that hook authors want
> to filter on that aren't capabilities then that may override this. It
> seems to me that only exposing capabilities would be a far more
> elegant solution.

Guess I should have read the bug first rather than just reading this
thread.  Since it seems I'm rehashing the old arguments against doing
this.

Still not fond of this feature but I get the point.


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread Ben Reser
On Fri, Aug 17, 2012 at 12:11 PM, Greg Hudson  wrote:
> I would expect a client version to be communicated over libsvn_ra_svn in
> a string, not a word.  The original design intent was that words are
> only for enumerated protocol values understood by the ra_svn code itself.
>
> It seems like we departed from this design with #2991 when we made a set
> of ra_svn words visible to hook scripts.  That seems like a mistake,
> just as it would be a mistake to put C identifiers into the UI.
>
> Anyway, for the case at hand, I think the client version needs to be
> marshalled as a string, not a word, and therefore not as a capability.

Is there real value in doing this?  Wouldn't it be better to expose
the client capabilities to the hook scripts?

I don't see how you're going to avoid the user agent nonsense as long
as you expose the client names and version numbers.

If you take the TSVN example and image that another client decided to
implement the client side hooks, then both TSVN and that hypothetical
client could report a capability that specified that it supported
client side hooks.  With the added benefit that if the server side
hook script was written for TSVN it would automatically start working
for a new client, making using this feature far less burdensome.

Exposing capabilities are far less likely to be abused to side step
server hooks since changing the capabilities will change the behavior
of the server, generally breaking the client (with the notable
exception of the above TSVN client side hook example).  Making a
client misreport a client name and version number that is only
provided to server hooks provides great incentive to modify these
values.

If there are behaviors of clients in the wild that hook authors want
to filter on that aren't capabilities then that may override this. It
seems to me that only exposing capabilities would be a far more
elegant solution.


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread C. Michael Pilato
On 08/17/2012 03:11 PM, Greg Hudson wrote:
> On 08/17/2012 02:35 PM, C. Michael Pilato wrote:
>> I'm sitting at this spot right now:  Unless we want hooks parsing literal
>> capability strings such as "client-version-tsvn-1dot7dot0", we have to
>> change the server.  And if we have to change the server *at all* to make
>> this all work, then we should simultaneously change the protocol so as not
>> to require some still-obscure marshaling mechanism.
> 
> I would expect a client version to be communicated over libsvn_ra_svn in
> a string, not a word.  The original design intent was that words are
> only for enumerated protocol values understood by the ra_svn code itself.
> 
> It seems like we departed from this design with #2991 when we made a set
> of ra_svn words visible to hook scripts.  That seems like a mistake,
> just as it would be a mistake to put C identifiers into the UI.
> 
> Anyway, for the case at hand, I think the client version needs to be
> marshalled as a string, not a word, and therefore not as a capability.

So, we marshal client-version/client-name/client-compat-version as strings
(as we do ""ra-client" and "client" now).  Extant servers ignore them.  New
servers are format those things in the way we've indicated elsethread
(ensuring that they match the specific regexp, slapping the likes of
"client-version=" on the head of them, etc.), merge them with the
capabilities array, and hand them to the start-commit hook.

Is that what you're proposing?

Or should we give these things unique placeholders in the start-commit
argument list rather than allowing them to masquerade as capabilities?

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread Greg Hudson
On 08/17/2012 02:35 PM, C. Michael Pilato wrote:
> I'm sitting at this spot right now:  Unless we want hooks parsing literal
> capability strings such as "client-version-tsvn-1dot7dot0", we have to
> change the server.  And if we have to change the server *at all* to make
> this all work, then we should simultaneously change the protocol so as not
> to require some still-obscure marshaling mechanism.

I would expect a client version to be communicated over libsvn_ra_svn in
a string, not a word.  The original design intent was that words are
only for enumerated protocol values understood by the ra_svn code itself.

It seems like we departed from this design with #2991 when we made a set
of ra_svn words visible to hook scripts.  That seems like a mistake,
just as it would be a mistake to put C identifiers into the UI.

Anyway, for the case at hand, I think the client version needs to be
marshalled as a string, not a word, and therefore not as a capability.



Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread Branko Čibej
On 17.08.2012 20:35, C. Michael Pilato wrote:
> On 08/17/2012 02:07 PM, Branko Čibej wrote:
>> On 17.08.2012 20:01, C. Michael Pilato wrote:
>>> On 08/17/2012 12:43 PM, Daniel Shahaf wrote:
 5. Require the version string and client name to match
/^[A-Za-z0-9-]+$/, and embed them directly as part of a capability
name (so: capabilities might be named client-version-tsvn-1dot7dot0)
>>> I'll give the honor of treating this suggestion as a serious one.  And then
>>> the dishonor of a hearty -1.  :-)
>> Why? It's essentially the same as 4., just with a different and
>> higher-level marshalling.
> Because it is higher-level marshaling.  The ideal scenario keeps our
> restrictions and conversion costs as close to the layers that actually
> require them as possible.  That said, I agree that in terms of the
> big-picture problem/solution set of this feature, #5 and #4 are roughly
> equivalent.  I just deem #5 to be more work and less flexible than #4 for
> the creators/consumers of this information.
>
> I'm sitting at this spot right now:  Unless we want hooks parsing literal
> capability strings such as "client-version-tsvn-1dot7dot0", we have to
> change the server.  And if we have to change the server *at all* to make
> this all work, then we should simultaneously change the protocol so as not
> to require some still-obscure marshaling mechanism.

Noted. Of course this won't actually make the parsing significantly
easier for script authors, it'll just appear that way at first glance. :)

BTW, in connection with this, I've been wondering which version of what
is actually relevant here. The version of libsvn_client? The version of
the command-line tool? How does this relate to TSVN and/or SharpSVN
and/or SvnKit?

The easy answer is to let the script authors worry about that, but
without at least some kind of guideline, that's a good way of
reproducing the user-agent madness. For example, it would very probably
make sense for TortoiseSVN to report the version of the Subversion
libraries it's built with, but I'm not at all clear how to sanely
formulate such a rule.

-- Brane

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download



Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread C. Michael Pilato
On 08/17/2012 02:07 PM, Branko Čibej wrote:
> On 17.08.2012 20:01, C. Michael Pilato wrote:
>> On 08/17/2012 12:43 PM, Daniel Shahaf wrote:
>>> Stefan Sperling wrote on Fri, Aug 17, 2012 at 18:24:03 +0200:
 On Fri, Aug 17, 2012 at 12:14:31PM -0400, C. Michael Pilato wrote:
>  4. something else?
 4. Marshall the version string before sending it across svn://,
escaping unsupported characters somehow in a reversible way,
and unescape them before passing them to hooks?
I.e. use something like strnvis() but adapted to the
restrictions of the svn:// protocol.
>> Yeah, I forgot to add that I was thinking about this approach, too.  I'm not
>> familiar with strnvis(), but I assume it's similar to what we do with our
>> checksum API _readable() variants (where 'A' is 0x41 and represented as as
>> "41")?
>>
>>> 5. Require the version string and client name to match
>>>/^[A-Za-z0-9-]+$/, and embed them directly as part of a capability
>>>name (so: capabilities might be named client-version-tsvn-1dot7dot0)
>> I'll give the honor of treating this suggestion as a serious one.  And then
>> the dishonor of a hearty -1.  :-)
> 
> Why? It's essentially the same as 4., just with a different and
> higher-level marshalling.

Because it is higher-level marshaling.  The ideal scenario keeps our
restrictions and conversion costs as close to the layers that actually
require them as possible.  That said, I agree that in terms of the
big-picture problem/solution set of this feature, #5 and #4 are roughly
equivalent.  I just deem #5 to be more work and less flexible than #4 for
the creators/consumers of this information.

I'm sitting at this spot right now:  Unless we want hooks parsing literal
capability strings such as "client-version-tsvn-1dot7dot0", we have to
change the server.  And if we have to change the server *at all* to make
this all work, then we should simultaneously change the protocol so as not
to require some still-obscure marshaling mechanism.

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread C. Michael Pilato
On 08/17/2012 02:07 PM, Branko Čibej wrote:
> On 17.08.2012 20:01, C. Michael Pilato wrote:
>> On 08/17/2012 12:43 PM, Daniel Shahaf wrote:
>>> Stefan Sperling wrote on Fri, Aug 17, 2012 at 18:24:03 +0200:
 On Fri, Aug 17, 2012 at 12:14:31PM -0400, C. Michael Pilato wrote:
>  4. something else?
 4. Marshall the version string before sending it across svn://,
escaping unsupported characters somehow in a reversible way,
and unescape them before passing them to hooks?
I.e. use something like strnvis() but adapted to the
restrictions of the svn:// protocol.
>> Yeah, I forgot to add that I was thinking about this approach, too.  I'm not
>> familiar with strnvis(), but I assume it's similar to what we do with our
>> checksum API _readable() variants (where 'A' is 0x41 and represented as as
>> "41")?
>>
>>> 5. Require the version string and client name to match
>>>/^[A-Za-z0-9-]+$/, and embed them directly as part of a capability
>>>name (so: capabilities might be named client-version-tsvn-1dot7dot0)
>> I'll give the honor of treating this suggestion as a serious one.  And then
>> the dishonor of a hearty -1.  :-)
> 
> Why? It's essentially the same as 4., just with a different and
> higher-level marshalling.

Because it is higher-level marshaling.  The ideal scenario keeps our
restrictions and conversion costs as close to the layers that actually
require them as possible.

I'm sitting at this spot right now:  Unless we want hooks parsing literal
capability strings such as "client-version-tsvn-1dot7dot0", we have to
change the server.  And if we have to change the server *at all* to make
this all work, then we should simultaneously change the protocol so as not
to require some still-obscure marshaling mechanism.

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread Branko Čibej
On 17.08.2012 20:01, C. Michael Pilato wrote:
> On 08/17/2012 12:43 PM, Daniel Shahaf wrote:
>> Stefan Sperling wrote on Fri, Aug 17, 2012 at 18:24:03 +0200:
>>> On Fri, Aug 17, 2012 at 12:14:31PM -0400, C. Michael Pilato wrote:
  4. something else?
>>> 4. Marshall the version string before sending it across svn://,
>>>escaping unsupported characters somehow in a reversible way,
>>>and unescape them before passing them to hooks?
>>>I.e. use something like strnvis() but adapted to the
>>>restrictions of the svn:// protocol.
> Yeah, I forgot to add that I was thinking about this approach, too.  I'm not
> familiar with strnvis(), but I assume it's similar to what we do with our
> checksum API _readable() variants (where 'A' is 0x41 and represented as as
> "41")?
>
>> 5. Require the version string and client name to match
>>/^[A-Za-z0-9-]+$/, and embed them directly as part of a capability
>>name (so: capabilities might be named client-version-tsvn-1dot7dot0)
> I'll give the honor of treating this suggestion as a serious one.  And then
> the dishonor of a hearty -1.  :-)

Why? It's essentially the same as 4., just with a different and
higher-level marshalling.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download



Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread C. Michael Pilato
On 08/17/2012 12:43 PM, Daniel Shahaf wrote:
> Stefan Sperling wrote on Fri, Aug 17, 2012 at 18:24:03 +0200:
>> On Fri, Aug 17, 2012 at 12:14:31PM -0400, C. Michael Pilato wrote:
>>>  4. something else?
>>
>> 4. Marshall the version string before sending it across svn://,
>>escaping unsupported characters somehow in a reversible way,
>>and unescape them before passing them to hooks?
>>I.e. use something like strnvis() but adapted to the
>>restrictions of the svn:// protocol.

Yeah, I forgot to add that I was thinking about this approach, too.  I'm not
familiar with strnvis(), but I assume it's similar to what we do with our
checksum API _readable() variants (where 'A' is 0x41 and represented as as
"41")?

> 5. Require the version string and client name to match
>/^[A-Za-z0-9-]+$/, and embed them directly as part of a capability
>name (so: capabilities might be named client-version-tsvn-1dot7dot0)

I'll give the honor of treating this suggestion as a serious one.  And then
the dishonor of a hearty -1.  :-)

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread Daniel Shahaf
Stefan Sperling wrote on Fri, Aug 17, 2012 at 18:24:03 +0200:
> On Fri, Aug 17, 2012 at 12:14:31PM -0400, C. Michael Pilato wrote:
> >  4. something else?
> 
> 4. Marshall the version string before sending it across svn://,
>escaping unsupported characters somehow in a reversible way,
>and unescape them before passing them to hooks?
>I.e. use something like strnvis() but adapted to the
>restrictions of the svn:// protocol.

5. Require the version string and client name to match
   /^[A-Za-z0-9-]+$/, and embed them directly as part of a capability
   name (so: capabilities might be named client-version-tsvn-1dot7dot0)


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread Stefan Sperling
On Fri, Aug 17, 2012 at 12:14:31PM -0400, C. Michael Pilato wrote:
>  4. something else?

4. Marshall the version string before sending it across svn://,
   escaping unsupported characters somehow in a reversible way,
   and unescape them before passing them to hooks?
   I.e. use something like strnvis() but adapted to the
   restrictions of the svn:// protocol.


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-17 Thread C. Michael Pilato
Well, I just ran into my first hurdle:  the current svnserve protocol
demands that capabilities in the standard capabilities string list be of the
protocol data class "word", which is limited to strings with letters,
numbers, and hyphens only.  I think that pretty much rules out having this
functionality "just work" for existing servers in the wild -- admins would
need 1.8+ servers.

But if we need a new server + protocol changes to accomplish this anyway,
then what changes to make?

 1. change the definition of "word" to include more characters?  Feels
wrong.  -1

 2. introduce a new protocol data type that covers the character classes we
need, and change the protocol to expect that primitive type instead of
"word" for capabilities?  Hrm... feels like smashing a fly with a
Buick.  -0

 3. just use the "string" type for capabilities, but with server-enforced
additional formatting constraints.  +0/1?  (But is this meaningfully
different from suggestion #2?)

 4. something else?

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-16 Thread Daniel Shahaf
C. Michael Pilato wrote on Thu, Aug 16, 2012 at 12:11:56 -0400:
> On 08/16/2012 10:50 AM, Daniel Shahaf wrote:
> > C. Michael Pilato wrote on Thu, Aug 16, 2012 at 10:10:49 -0400:
> >># Reports the general name of the client.
> >>#
> >># Examples: "svn", "tortoisesvn", "svnkit"
> >>#
> >># Default value: "svn".
> >>#
> >>client-name=[-_a-zA-Z0-9]+
> > 
> > I suggest the libsvn_client default be different from what the cmdline
> > client uses.  (So, for example, maybe the default is "svn" and
> > the cmdline client calls itself "cmdline".)
> 
> That makes sense. (I would use "subversion" and "svn" respectively.
> http://fuschia.bikeshed.org .)

And I would go for "libsvn_client" and "cmdline", but decided to use
"svn" as part of the example due to http://180952.bikeshed.com/


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-16 Thread C. Michael Pilato
On 08/16/2012 10:50 AM, Daniel Shahaf wrote:
> C. Michael Pilato wrote on Thu, Aug 16, 2012 at 10:10:49 -0400:
>># Reports the general name of the client.
>>#
>># Examples: "svn", "tortoisesvn", "svnkit"
>>#
>># Default value: "svn".
>>#
>>client-name=[-_a-zA-Z0-9]+
> 
> I suggest the libsvn_client default be different from what the cmdline
> client uses.  (So, for example, maybe the default is "svn" and
> the cmdline client calls itself "cmdline".)

That makes sense. (I would use "subversion" and "svn" respectively.
http://fuschia.bikeshed.org .)

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-16 Thread Stefan Küng
On Thu, Aug 16, 2012 at 4:46 PM, Cedric JP. Brasey
 wrote:
> Just my two pence worth...
>
>
>
> From a hooking point; I would have to say that i would only really care
> about what the client level is. For example. On pre-commit i would like to
> know if the client is at least 1.7, so that I can then reject the commit. I
> can't imagine, for the mo anyways, any scenario where i would care if this
> is a windows client, svn kit, etc.
>
>
>
> From a server point; I would imagine that this would be like the preverbal
> pot 'o gold. You could introduce new functions with the assurance that the
> client is of the correct spec. Backwards compatibility would also be easier
> with the introduction of a 'minimum level' for the methods...

It can also be important to know what client it is and its version,
not just the svn library version.

For example:
TSVN has client-side hooks, which can be used to run e.g. a full test
suite before a commit, or run a script which checks whether the user
has run all required tests.
Now, the server hook script might want to check whether the user
actually uses TSVN, and with a version that executes those client-side
hook scripts.
Only if e.g. TSVN is used with version > 1.8.0 then the hook script
allows the commit because it only then can assume that the commit only
happens if the tests were run.

So, if we can give that info back to the hook script, then we should do so.

Stefan

-- 
   ___
  oo  // \\  "De Chelonian Mobile"
 (_,\/ \_/ \ TortoiseSVN
   \ \_/_\_/>The coolest Interface to (Sub)Version Control
   /_/   \_\ http://tortoisesvn.net


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-16 Thread Daniel Shahaf
C. Michael Pilato wrote on Thu, Aug 16, 2012 at 10:10:49 -0400:
># Reports the general name of the client.
>#
># Examples: "svn", "tortoisesvn", "svnkit"
>#
># Default value: "svn".
>#
>client-name=[-_a-zA-Z0-9]+

I suggest the libsvn_client default be different from what the cmdline
client uses.  (So, for example, maybe the default is "svn" and
the cmdline client calls itself "cmdline".)


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-16 Thread Daniel Shahaf
Mark Phippard wrote on Thu, Aug 16, 2012 at 10:18:13 -0400:
> On Thu, Aug 16, 2012 at 10:10 AM, C. Michael Pilato  
> wrote:
> 
> > So, incorporating some of the great suggestions on this thread so far, but
> > adding some of my own, I'd like to reset this design discussion with the
> > following proposal:
> >
> ># Reports the general name of the client.
> >#
> ># Examples: "svn", "tortoisesvn", "svnkit"
> >#
> ># Default value: "svn".
> >#
> >client-name=[-_a-zA-Z0-9]+
> >
> ># Reports the specific version of the client.
> >#
> ># Examples: "1.6.0a2", "1.5.4dfsg1-1ubuntu2.1"
> >#
> ># Default value: ${SVN_VER_NUMBER}
> >#
> >client-version=[-_a-zA-Z0-9\.\+]+
> >
> ># Reports the version of the Apache Subversion libraries with which
> ># this client is most compatible.
> >#
> ># Examples: "1.6.0", "1.8.0-dev", "1.7.0-alpha1"
> >#
> ># Default value: ${SVN_VER_NUMBER}
> >#
> >client-compat-version=[0-9]+\.[0-9]+\.[0-9]+(-[a-z0-9]+)?
> >
> > Here are some examples:
> >
> >client-name=svn
> >client=version=1.8.0-dev
> >client-compat-version=1.8.0-dev
> >
> >client-name=svnkit
> >client-version=1.7.5-v1
> >client-compat-version=1.7.5
> >
> >client-name=tortoisesvn
> >client-version=1.7.8
> >client-compat-version=1.7.6
> >
> >client-name=svn
> >client=version=1.5.4dfsg1-1ubuntu2.1
> >client-compat-version=1.5.4
> >
> > I think this will allow the best of both worlds for hook authors:  the
> > deeper insight needed to rule in/rule out specific clients and client
> > versions, plus a way to make more general rulings based on the set of
> > features expected to be present in a given Apache Subversion release.
> >
> > Thoughts?
> 
> It probably does not hurt anything to "do more" as your proposal does.
>  But personally, I think that your "client-compat-version" value is
> the only one that is needed and I would be fine if that was the only
> thing we added.
> 

Did anyone ever ask for anything beyond "what libsvn version are we
implementing"?  

> -- 
> Thanks
> 
> Mark Phippard
> http://markphip.blogspot.com/


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-16 Thread Cedric JP. Brasey
 

Just my two pence worth...  

   

>From a hooking point; I would have to say that i would only really care about 
>what the client level is. For example. On pre-commit i would like to know if 
>the client is at least 1.7, so that I can then reject the commit. I can't 
>imagine, for the mo anyways, any scenario where i would care if this is a 
>windows client, svn kit, etc.  

   

>From a server point; I would imagine that this would be like the preverbal pot 
>'o gold. You could introduce new functions with the assurance that the client 
>is of the correct spec. Backwards compatibility would also be easier with the 
>introduction of a 'minimum level' for the methods...  

   

Food for thought really.  _  

  From: Mark Phippard [mailto:markp...@gmail.com]
To: C. Michael Pilato [mailto:cmpil...@collab.net]
Cc: Subversion Development [mailto:dev@subversion.apache.org]
Sent: Thu, 16 Aug 2012 15:18:13 +0000
Subject: Re: [Issue 4124] New - Give servers sufficient means to disallow 
commits from clients based on version numbers

On Thu, Aug 16, 2012 at 10:10 AM, C. Michael Pilato  wrote:

> So, incorporating some of the great suggestions on this thread so far, but
> adding some of my own, I'd like to reset this design discussion with the
> following proposal:
>
> # Reports the general name of the client.
> #
> # Examples: "svn", "tortoisesvn", "svnkit"
> #
> # Default value: "svn".
> #
> client-name=[-_a-zA-Z0-9]+
>
> # Reports the specific version of the client.
> #
> # Examples: "1.6.0a2", "1.5.4dfsg1-1ubuntu2.1"
> #
> # Default value: ${SVN_VER_NUMBER}
> #
> client-version=[-_a-zA-Z0-9\.\+]+
>
> # Reports the version of the Apache Subversion libraries with which
> # this client is most compatible.
> #
> # Examples: "1.6.0", "1.8.0-dev", "1.7.0-alpha1"
> #
> # Default value: ${SVN_VER_NUMBER}
> #
> client-compat-version=[0-9]+\.[0-9]+\.[0-9]+(-[a-z0-9]+)?
>
> Here are some examples:
>
> client-name=svn
> client=version=1.8.0-dev
> client-compat-version=1.8.0-dev
>
> client-name=svnkit
> client-version=1.7.5-v1
> client-compat-version=1.7.5
>
> client-name=tortoisesvn
> client-version=1.7.8
> client-compat-version=1.7.6
>
> client-name=svn
> client=version=1.5.4dfsg1-1ubuntu2.1
> client-compat-version=1.5.4
>
> I think this will allow the best of both worlds for hook authors: the
> deeper insight needed to rule in/rule out specific clients and client
> versions, plus a way to make more general rulings based on the set of
> features expected to be present in a given Apache Subversion release.
>
> Thoughts?

It probably does not hurt anything to "do more" as your proposal does.
But personally, I think that your "client-compat-version" value is
the only one that is needed and I would be fine if that was the only
thing we added.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/
  
   
 


]---[
This e-mail and any attached files are confidential and may also be legally 
privileged.
They are intended solely for the intended addressee. If you are not the 
addressee 
please e-mail it back to the sender and then immediately, permanently delete 
it. Do 
not read, print, re-transmit, store or act in reliance on it. This e-mail may 
be monitored
 by Planet Side Limited in accordance with current regulations. This footnote 
also 
confirms that this e-mail message has been swept for the presence of computer 
viruses 
currently known to Planet Side Limited. However, the recipient is responsible 
for virus-checking 
before opening this message and any attachment. Unless expressly stated to the 
contrary,
 any views expressed in this message are those of the individual sender and may 
not 
necessarily reflect the views of Planet Side Limited. 

http://www.planet-side.co.uk


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-16 Thread Mark Phippard
On Thu, Aug 16, 2012 at 10:10 AM, C. Michael Pilato  wrote:

> So, incorporating some of the great suggestions on this thread so far, but
> adding some of my own, I'd like to reset this design discussion with the
> following proposal:
>
># Reports the general name of the client.
>#
># Examples: "svn", "tortoisesvn", "svnkit"
>#
># Default value: "svn".
>#
>client-name=[-_a-zA-Z0-9]+
>
># Reports the specific version of the client.
>#
># Examples: "1.6.0a2", "1.5.4dfsg1-1ubuntu2.1"
>#
># Default value: ${SVN_VER_NUMBER}
>#
>client-version=[-_a-zA-Z0-9\.\+]+
>
># Reports the version of the Apache Subversion libraries with which
># this client is most compatible.
>#
># Examples: "1.6.0", "1.8.0-dev", "1.7.0-alpha1"
>#
># Default value: ${SVN_VER_NUMBER}
>#
>client-compat-version=[0-9]+\.[0-9]+\.[0-9]+(-[a-z0-9]+)?
>
> Here are some examples:
>
>client-name=svn
>client=version=1.8.0-dev
>client-compat-version=1.8.0-dev
>
>client-name=svnkit
>client-version=1.7.5-v1
>client-compat-version=1.7.5
>
>client-name=tortoisesvn
>client-version=1.7.8
>client-compat-version=1.7.6
>
>client-name=svn
>client=version=1.5.4dfsg1-1ubuntu2.1
>client-compat-version=1.5.4
>
> I think this will allow the best of both worlds for hook authors:  the
> deeper insight needed to rule in/rule out specific clients and client
> versions, plus a way to make more general rulings based on the set of
> features expected to be present in a given Apache Subversion release.
>
> Thoughts?

It probably does not hurt anything to "do more" as your proposal does.
 But personally, I think that your "client-compat-version" value is
the only one that is needed and I would be fine if that was the only
thing we added.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-08-16 Thread C. Michael Pilato
Trying to revive an old thread...

Daniel Shahaf wrote:
> Markus Schaber wrote on Fri, Feb 24, 2012 at 08:09:30 +:
>> There are two other cases we should define a rule for:
>>
>> 1) Clients like SharpSVN or TortoiseSVN, or distribution builds, when
>> they include their own patches, or backported patches which were not
>> officially part of that release. Especially Debian tends to prefer
>> backporting of fixes over updating to new releases.
>>
>> 2) Clients based on alternative implementations like SvnKit (is there
>> any other?).
>
> Solve both by adding a "capability" for the short client name?
>
> client-version=1.7.3 client-client=tortoisesvn
> client-version=1.7.3 client-client=svnkit

The point of this feature is to give server admins (or more specifically,
repository hook authors) the ability to make decisions about
allowing/disallowing interaction with particular clients.  But, like Julian,
I want to avoid having this explode into something as adhoc as a User-Agent
string.  Is it realistic to think that a server admin is going to make an
"interaction is allowed" decision based on whether the client is a specific
version of a Debian build with particular bugfix patches applied?  That
seems like overkill to me.  Further, every expansion of the valid
capability-string space we make is an additional burden to hook authors.

So, incorporating some of the great suggestions on this thread so far, but
adding some of my own, I'd like to reset this design discussion with the
following proposal:

   # Reports the general name of the client.
   #
   # Examples: "svn", "tortoisesvn", "svnkit"
   #
   # Default value: "svn".
   #
   client-name=[-_a-zA-Z0-9]+

   # Reports the specific version of the client.
   #
   # Examples: "1.6.0a2", "1.5.4dfsg1-1ubuntu2.1"
   #
   # Default value: ${SVN_VER_NUMBER}
   #
   client-version=[-_a-zA-Z0-9\.\+]+

   # Reports the version of the Apache Subversion libraries with which
   # this client is most compatible.
   #
   # Examples: "1.6.0", "1.8.0-dev", "1.7.0-alpha1"
   #
   # Default value: ${SVN_VER_NUMBER}
   #
   client-compat-version=[0-9]+\.[0-9]+\.[0-9]+(-[a-z0-9]+)?

Here are some examples:

   client-name=svn
   client=version=1.8.0-dev
   client-compat-version=1.8.0-dev

   client-name=svnkit
   client-version=1.7.5-v1
   client-compat-version=1.7.5

   client-name=tortoisesvn
   client-version=1.7.8
   client-compat-version=1.7.6

   client-name=svn
   client=version=1.5.4dfsg1-1ubuntu2.1
   client-compat-version=1.5.4

I think this will allow the best of both worlds for hook authors:  the
deeper insight needed to rule in/rule out specific clients and client
versions, plus a way to make more general rulings based on the set of
features expected to be present in a given Apache Subversion release.

Thoughts?

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-02-24 Thread Julian Foad
Daniel Shahaf wrote:

> Markus Schaber wrote:
>> There are two other cases we should define a rule for:
>> 
>> 1) Clients like SharpSVN or TortoiseSVN, or distribution builds, when they 
>> include their own patches, or backported patches which were not officially 
>> part 
>> of that release. [...]
>> 
>> 2) Clients based on alternative implementations like SvnKit (is there any 
>> other?).
> 
> Solve both by adding a "capability" for the short client name?
> 
> client-version=1.7.3 client-client=tortoisesvn
> client-version=1.7.3 client-client=svnkit


For years, as I understand, people have been using the "user-agent" string 
that's transmitted over HTTP 
connections to identify the client when they want this sort of ability 
to reject certain client versions.  I assume the main problem with the 
"user-agent" string is it's not available on the server in a consistent 
way for all RA protocols.  It would be better if it were available in a 
consistent way, and an obvious way is making it available to a hook 
script, as C-Mike's proposal says.

However,
 the exact format of the user-agent string has been left to the 
discretion of client authors.  Examples from a web search:

SVN/1.0.0 neon/0.24.4
SVN/1.0.1 (dev build) neon/0.24.4
SVN/1.6.5 (r38866)/TortoiseSVN-1.6.5.16974
SVN/1.7.0-dev neon/0.29.6
SVN/1.7.2 neon/0.29.6
SVN/1.7.2/TortoiseSVN-1.7.3.22386 neon/0.29.6
SVN/1.7.2 SVNKit/1.7.0-alpha2 (http://svnkit.com/) t20120112_2133
SVN/1.8.0-dev serf/2.0.0

So can we take a clue from those examples and define something perhaps a little 
more formally structured but still with enough flexibility for clients to 
specify their own version string as well as the version of SVN libraries that 
they're actually or nominally based on?

- Julian



Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-02-24 Thread Daniel Shahaf
Markus Schaber wrote on Fri, Feb 24, 2012 at 08:09:30 +:
> Hi, Daniel,
> 
> -Ursprüngliche Nachricht-
> Von: Daniel Shahaf [mailto:danie...@elego.de] 
> 
> >C. Michael Pilato wrote on Thu, Feb 23, 2012 at 10:48:12 -0500:
> >> On 02/23/2012 10:35 AM, Mark Phippard wrote:
> >> > On Thu, Feb 23, 2012 at 10:32 AM, Paul Burba  wrote:
> >> >>> Why not just give users what they want?  A capability string (or 
> >> >>> comparable
> >> >>> mechanism) that carries the client's version number, for use with 
> >> >>> start-commit hooks in allowing/denying clients which don't meet 
> >> >>> the administrator's quality requirements?
> >> >>
> >> >> Mike,
> >> >>
> >> >> +1 to this enhancement.  One question though, are you envisioning a
> >> >> new "capability" for each minor release or for each patch release?  
> >> >> I assume the latter, but that's not entirely clear from the issue 
> >> >> writeup.
> >> > 
> >> > I would suggest something easily parseable like "ClientVersion-1.7.3"
> >> > 
> >> > I would assume it would be a single string that contained the exact 
> >> > version number and it would be up to the hook script to do something 
> >> > with it.
> >> 
> >> As Mark indicated, I was *not* thinking about a different capability 
> >> per release -- just a single, changing one.  My suggested syntax was 
> >> to be "client-version=1.7.3", establishing precedent for the use of 
> >> the equal sign
> >
> >What client-version would dev builds report?
> 
> There are two other cases we should define a rule for:
> 
> 1) Clients like SharpSVN or TortoiseSVN, or distribution builds, when they 
> include their own patches, or backported patches which were not officially 
> part of that release. Especially Debian tends to prefer backporting of fixes 
> over updating to new releases.
> 
> 2) Clients based on alternative implementations like SvnKit (is there any 
> other?).
> 

Solve both by adding a "capability" for the short client name?

client-version=1.7.3 client-client=tortoisesvn
client-version=1.7.3 client-client=svnkit

> Best regards
> 
> Markus Schaber
> -- 
> ___
> We software Automation.
> 
> 3S-Smart Software Solutions GmbH
> Markus Schaber | Developer
> Memminger Str. 151 | 87439 Kempten | Germany | Tel. +49-831-54031-0 | Fax 
> +49-831-54031-50
> 
> Email: m.scha...@3s-software.com | Web: http://www.3s-software.com 
> CoDeSys internet forum: http://forum.3s-software.com
> Download CoDeSys sample projects: 
> http://www.3s-software.com/index.shtml?sample_projects
> 
> Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade 
> register: Kempten HRB 6186 | Tax ID No.: DE 167014915 
> 
> 


AW: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-02-24 Thread Markus Schaber
Hi, Daniel,

-Ursprüngliche Nachricht-
Von: Daniel Shahaf [mailto:danie...@elego.de] 

>C. Michael Pilato wrote on Thu, Feb 23, 2012 at 10:48:12 -0500:
>> On 02/23/2012 10:35 AM, Mark Phippard wrote:
>> > On Thu, Feb 23, 2012 at 10:32 AM, Paul Burba  wrote:
>> >>> Why not just give users what they want?  A capability string (or 
>> >>> comparable
>> >>> mechanism) that carries the client's version number, for use with 
>> >>> start-commit hooks in allowing/denying clients which don't meet 
>> >>> the administrator's quality requirements?
>> >>
>> >> Mike,
>> >>
>> >> +1 to this enhancement.  One question though, are you envisioning a
>> >> new "capability" for each minor release or for each patch release?  
>> >> I assume the latter, but that's not entirely clear from the issue 
>> >> writeup.
>> > 
>> > I would suggest something easily parseable like "ClientVersion-1.7.3"
>> > 
>> > I would assume it would be a single string that contained the exact 
>> > version number and it would be up to the hook script to do something 
>> > with it.
>> 
>> As Mark indicated, I was *not* thinking about a different capability 
>> per release -- just a single, changing one.  My suggested syntax was 
>> to be "client-version=1.7.3", establishing precedent for the use of 
>> the equal sign
>
>What client-version would dev builds report?

There are two other cases we should define a rule for:

1) Clients like SharpSVN or TortoiseSVN, or distribution builds, when they 
include their own patches, or backported patches which were not officially part 
of that release. Especially Debian tends to prefer backporting of fixes over 
updating to new releases.

2) Clients based on alternative implementations like SvnKit (is there any 
other?).

Best regards

Markus Schaber
-- 
___
We software Automation.

3S-Smart Software Solutions GmbH
Markus Schaber | Developer
Memminger Str. 151 | 87439 Kempten | Germany | Tel. +49-831-54031-0 | Fax 
+49-831-54031-50

Email: m.scha...@3s-software.com | Web: http://www.3s-software.com 
CoDeSys internet forum: http://forum.3s-software.com
Download CoDeSys sample projects: 
http://www.3s-software.com/index.shtml?sample_projects

Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade 
register: Kempten HRB 6186 | Tax ID No.: DE 167014915 




Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-02-23 Thread C. Michael Pilato
On 02/23/2012 10:24 PM, Daniel Shahaf wrote:
> C. Michael Pilato wrote on Thu, Feb 23, 2012 at 10:48:12 -0500:
>> As Mark indicated, I was *not* thinking about a different capability per
>> release -- just a single, changing one.  My suggested syntax was to be
>> "client-version=1.7.3", establishing precedent for the use of the equal sign
> 
> What client-version would dev builds report?

First guess:  SVN_VER_NUMBER.

If we feel we need higher granularity, we can go with a full-fledged
SVN_VERSION, but that *might* force us away from using the existing
capabilities mechanism.  (I don't know what restrictions it has regarding
the use of whitespace.)

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-02-23 Thread Daniel Shahaf
C. Michael Pilato wrote on Thu, Feb 23, 2012 at 10:48:12 -0500:
> On 02/23/2012 10:35 AM, Mark Phippard wrote:
> > On Thu, Feb 23, 2012 at 10:32 AM, Paul Burba  wrote:
> >>> Why not just give users what they want?  A capability string (or 
> >>> comparable
> >>> mechanism) that carries the client's version number, for use with 
> >>> start-commit
> >>> hooks in allowing/denying clients which don't meet the administrator's 
> >>> quality
> >>> requirements?
> >>
> >> Mike,
> >>
> >> +1 to this enhancement.  One question though, are you envisioning a
> >> new "capability" for each minor release or for each patch release?  I
> >> assume the latter, but that's not entirely clear from the issue
> >> writeup.
> > 
> > I would suggest something easily parseable like "ClientVersion-1.7.3"
> > 
> > I would assume it would be a single string that contained the exact
> > version number and it would be up to the hook script to do something
> > with it.
> 
> As Mark indicated, I was *not* thinking about a different capability per
> release -- just a single, changing one.  My suggested syntax was to be
> "client-version=1.7.3", establishing precedent for the use of the equal sign

What client-version would dev builds report?

> for name/value pairs.

+0

> 
> -- 
> C. Michael Pilato 
> CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
> 




Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-02-23 Thread C. Michael Pilato
On 02/23/2012 10:35 AM, Mark Phippard wrote:
> On Thu, Feb 23, 2012 at 10:32 AM, Paul Burba  wrote:
>>> Why not just give users what they want?  A capability string (or comparable
>>> mechanism) that carries the client's version number, for use with 
>>> start-commit
>>> hooks in allowing/denying clients which don't meet the administrator's 
>>> quality
>>> requirements?
>>
>> Mike,
>>
>> +1 to this enhancement.  One question though, are you envisioning a
>> new "capability" for each minor release or for each patch release?  I
>> assume the latter, but that's not entirely clear from the issue
>> writeup.
> 
> I would suggest something easily parseable like "ClientVersion-1.7.3"
> 
> I would assume it would be a single string that contained the exact
> version number and it would be up to the hook script to do something
> with it.

As Mark indicated, I was *not* thinking about a different capability per
release -- just a single, changing one.  My suggested syntax was to be
"client-version=1.7.3", establishing precedent for the use of the equal sign
for name/value pairs.

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand



signature.asc
Description: OpenPGP digital signature


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-02-23 Thread Mark Phippard
On Thu, Feb 23, 2012 at 10:32 AM, Paul Burba  wrote:
>> Why not just give users what they want?  A capability string (or comparable
>> mechanism) that carries the client's version number, for use with 
>> start-commit
>> hooks in allowing/denying clients which don't meet the administrator's 
>> quality
>> requirements?
>
> Mike,
>
> +1 to this enhancement.  One question though, are you envisioning a
> new "capability" for each minor release or for each patch release?  I
> assume the latter, but that's not entirely clear from the issue
> writeup.

I would suggest something easily parseable like "ClientVersion-1.7.3"

I would assume it would be a single string that contained the exact
version number and it would be up to the hook script to do something
with it.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/


Re: [Issue 4124] New - Give servers sufficient means to disallow commits from clients based on version numbers

2012-02-23 Thread Paul Burba
On Wed, Feb 22, 2012 at 11:13 AM,   wrote:
> http://subversion.tigris.org/issues/show_bug.cgi?id=4124
>                 Issue #|4124
>                 Summary|Give servers sufficient means to disallow commits from
>                        | clients based on version numbers
>               Component|subversion
>                 Version|1.7.x
>                Platform|All
>                     URL|
>              OS/Version|All
>                  Status|NEW
>       Status whiteboard|
>                Keywords|
>              Resolution|
>              Issue type|ENHANCEMENT
>                Priority|P3
>            Subcomponent|libsvn_ra
>             Assigned to|issues@subversion
>             Reported by|cmpilato
>
>
>
>
>
>
> --- Additional comments from cmpil...@tigris.org Wed Feb 22 08:13:31 
> -0800 2012 ---
> I'm seeing more and more requests from admins (in public and private channels)
> seeking ways to ensure that the users of their repositories are committing 
> with
> a particular pedigree of client, namely one that meets some version number
> threshold.
>
> In the past, we frowned on the idea of such version-number-centric mechanisms
> for allowing/denying commit access, citing the ease with which the value can 
> be
> spoofed and opting instead to push for capabilities strings that carried more
> specific information ("supports merge tracking", or "supports atomic revision
> property changes", etc.).  The problem we are seeing now is two-fold:
>
> 1.  As a community, we the developers aren't particularly good at identifying
> which changes that we make to the client codebase are of the sort that 
> justify a
> new capability string.  So, we've been extremely conservative about adding 
> them,
> even though practically each new merge-tracking related bugfix is likely a
> reason to wish clients weren't using un-fixed clients.
>
> 2.  As implied above, there are in reality many more such interesting changes
> than we wish to individually track.  I mean, do we *really* want to see the
> likes of "merginfo, atomic-revprops, fix-for-issue-5911,
> fix-for-the-commit-part-of-issue-5621, subtree-mergeinfo-minimized,
> subtree-mergeinfo-minimized-round-2, ..."?
>
> Why not just give users what they want?  A capability string (or comparable
> mechanism) that carries the client's version number, for use with start-commit
> hooks in allowing/denying clients which don't meet the administrator's quality
> requirements?

Mike,

+1 to this enhancement.  One question though, are you envisioning a
new "capability" for each minor release or for each patch release?  I
assume the latter, but that's not entirely clear from the issue
writeup.

Paul