You are viewing a plain text version of this content. The canonical link for it is here.
Posted to rivet-dev@tcl.apache.org by Valery Masiutsin <va...@gmail.com> on 2007/10/15 18:14:24 UTC

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.

Re: Replacement of Rivet_IsRivetFile patch

Posted by Valery Masiutsin <va...@gmail.com>.
> 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-unsubscribe@tcl.apache.org
For additional commands, e-mail: rivet-dev-help@tcl.apache.org


Re: Replacement of Rivet_IsRivetFile patch

Posted by Massimo Manghi <ma...@unipr.it>.
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-unsubscribe@tcl.apache.org
For additional commands, e-mail: rivet-dev-help@tcl.apache.org


Re: Replacement of Rivet_IsRivetFile patch

Posted by David Welton <da...@gmail.com>.
> 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/davidw/

---------------------------------------------------------------------
To unsubscribe, e-mail: rivet-dev-unsubscribe@tcl.apache.org
For additional commands, e-mail: rivet-dev-help@tcl.apache.org


Re: Replacement of Rivet_IsRivetFile patch

Posted by Massimo Manghi <ma...@unipr.it>.
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: %s\n", 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-unsubscribe@tcl.apache.org
For additional commands, e-mail: rivet-dev-help@tcl.apache.org


Re: Replacement of Rivet_IsRivetFile patch

Posted by Massimo Manghi <ma...@unipr.it>.
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-unsubscribe@tcl.apache.org
For additional commands, e-mail: rivet-dev-help@tcl.apache.org


Re: Replacement of Rivet_IsRivetFile patch

Posted by Valery Masiutsin <va...@gmail.com>.
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.