Replacement of Rivet_IsRivetFile patch

Hello, guys !

Here is small patch which replaces questionable (ugly ?)
Rivet_IsRivetFile function,
with cleaner  equivalent.
It compiles fine for me.
Please review and test.

Regards Valery.

------------------------------------------------------------
---------
To unsubscribe, e-mail: rivet-dev-unsubscribetcl.apache.org
For additional commands, e-mail: rivet-dev-helptcl.apache.org

Valery Masiutsin wrote:
> Hello, guys !
>
> Here is small patch which replaces questionable (ugly
?)
> Rivet_IsRivetFile function,
> with cleaner  equivalent.
> It compiles fine for me.
> Please review and test.
>
> Regards Valery.
>   
Hi Valery,

your patch looks more than reasonable. If the association
between .tcl/.rvt and the 'content_type' header is
correctly
done by the server (through the module conf), I don't see
why
we should check the file type in the empirical way Rivet
has
done so far. The question is, as usual, for the people who
know the history of the module: is there any tricky reason
(that we may be overlooking) for adopting the basic method
of scanning the whole file name in order to extract the
file type?

 -- Massimo


------------------------------------------------------------
---------
To unsubscribe, e-mail: rivet-dev-unsubscribetcl.apache.org
For additional commands, e-mail: rivet-dev-helptcl.apache.org

> Here is small patch which replaces questionable (ugly
?)
> Rivet_IsRivetFile function,

Definitely ugly, no question about it.

> with cleaner  equivalent.
> It compiles fine for me.
> Please review and test.

Looks good to me!

Massimo writes:
> is there any tricky reason
> (that we may be overlooking) for adopting the basic
method
> of scanning the whole file name in order to extract
the
> file type?

Nope.  The Apache2 code is pretty messy.  The 1.3 code does
pretty
much everything correctly, and should be used as an
example.

-- 
David N. Welton
http://www.welton.it/dav
idw/

------------------------------------------------------------
---------
To unsubscribe, e-mail: rivet-dev-unsubscribetcl.apache.org
For additional commands, e-mail: rivet-dev-helptcl.apache.org

> Hi Valery,
>
> your patch looks more than reasonable. If the
association
> between .tcl/.rvt and the 'content_type' header is
correctly
> done by the server (through the module conf), I don't
see why
> we should check the file type in the empirical way
Rivet has
> done so far. The question is, as usual, for the people
who
> know the history of the module: is there any tricky
reason
> (that we may be overlooking) for adopting the basic
method
> of scanning the whole file name in order to extract
the
> file type?
>
>  -- Massimo
>
Hello, Massimo.

It seems like it was just a quick hack. The single reason
why it
played with extensions (IMHO !) was that the
Rivet_IsRivetFile
function has been used in
Rivet_ParseUri (it is very questionable as well), at this
point
content-type is not available, i see no another reason.

Regards Valery

------------------------------------------------------------
---------
To unsubscribe, e-mail: rivet-dev-unsubscribetcl.apache.org
For additional commands, e-mail: rivet-dev-helptcl.apache.org

Hello, guys !

Updated version of patch.

1.Added one more check  inside the Rivet_CheckType function
(When file does not exist req->content_type is NULL, well
at least i hope so,
we have to check for NULL, before trying to do
dereference).
2.Corrected file existence check in Rivet_SendContent

Regards Valery.

------------------------------------------------------------
---------
To unsubscribe, e-mail: rivet-dev-unsubscribetcl.apache.org
For additional commands, e-mail: rivet-dev-helptcl.apache.org

a quick test of the patch didn't work to me. An .rvt file is
passed
verbatim to the client. I enabled the debug message you had
introduced and then commented out.

    fprintf(stderr, "content_type: %sn",
req->content_type);
    fflush(stderr);

in apache2/error.log the server keeps printing

content_type: (null)

rivet.conf in /etc/apache2/mods-available/  is

<IfModule mod_rivet.c>
  AddType application/x-httpd-rivet     .rvt
  AddType application/x-rivet-tcl       .tcl
  RivetServerConf ChildInitScript       "source 
/home/manghi/www/webdev/childinit.tcl"
  RivetServerConf ChildExitScript       "source 
/home/manghi/www/webdev/childexit.tcl"
</IfModule>


 -- Massimo


Valery Masiutsin wrote:
> Hello, guys !
>
> Updated version of patch.
>
> 1.Added one more check  inside the Rivet_CheckType
function
> (When file does not exist req->content_type is NULL,
well at least i hope so,
> we have to check for NULL, before trying to do
dereference).
> 2.Corrected file existence check in Rivet_SendContent
>
> Regards Valery.
>   


------------------------------------------------------------
---------
To unsubscribe, e-mail: rivet-dev-unsubscribetcl.apache.org
For additional commands, e-mail: rivet-dev-helptcl.apache.org

Hi Valery, 

I found out that my apache2 had a problem with
the conf files. Actually the former check for the rivet
files kept the problem submerged. If you didn't change
it I would not realized it. Rivet is working on apache2
now and, as a matter of fact, i was able to run the whole
site I developed some time ago for my department. 
It seemed to work fine. 

good job Valery, 

ciao

 -- Massimo

On Tue, 16 Oct 2007 14:17:41 +0300, Valery Masiutsin wrote
> Hello, guys !
> 
> Updated version of patch.
> 
> 1.Added one more check  inside the Rivet_CheckType
function
> 
> (When file does not exist req->content_type is NULL,
well at least i 
> hope so, we have to check for NULL, before trying to do
dereference).
> 2.Corrected file existence check in Rivet_SendContent
> 
> Regards Valery.


--
Universita' degli Studi di Parma (http://www.unipr.it)



------------------------------------------------------------
---------
To unsubscribe, e-mail: rivet-dev-unsubscribetcl.apache.org
For additional commands, e-mail: rivet-dev-helptcl.apache.org