[PATCH 2 of 3 V6] hgweb: teach archive how to download a specific directory or file

[Angel Ezquerra]

On Wed, Mar 20, 2013 at 3:38 PM, Pierre-Yves David
<pierre-yves.david at logilab.fr> wrote:
> On Wed, Mar 13, 2013 at 09:34:08AM +0100, Angel Ezquerra wrote:
>> I checked archival.archive() and it does not check whether the number
>> of files matching the pattern is 0.
>>
>> I cannot tell if that is the desired behavior or a bug.
>>
>> In any case I think that is something that, if needs fixing, should be
>> fixed on a separate patch?
>
> That should probably be fixed. But this certainly deserve another
> patches.

So should we raise an exception in that case?

>> Also, I personally like the fact that we give a 403 error. IMHO it
>> tells you exactly what is the problem without compromising the
>> security of the server because we append "path:" to the requested file
>> path anyway (unless there is some way to feed '\b' into the requested
>> path?).
>>
>> So I'd vote to take the latest version of my patch series (V7!) which
>> explicitly checks for known patterns, and I would deal with the
>> possible archive.archival bug separately.
>
> Black-listing security does not work. extension and futur version may
> (will) add extra patterns ("et bim"). I vote for requesting plain file
> name all the time. this means inconditionnaly adding "path:" in the
> front of the parameter.

I agree. In fact that is what the patch does already. This means that
if you try to use _any_ pattern (including "path") the web server will
respond with an error. The only difference is that if you try to use a
"known" pattern the server will respond with a nice 403 "Archive
pattern not allowed" message. If in the future a new pattern type is
added and this part of the code is not properly updated the web server
will return an empty archive (due to the archival.archive() bug
discussed above).

> This will returns 404 on any pattern (including "path:" but plain
> pattern are requested)

Not until the archival.archive() but is fixed.

BTW, I don't know if you noticed that this series has been crewed.

Cheers,

Angel