patchCERTI - Patches: patch #6561, Support for RTIA-->RTIG...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #6561: Support for RTIA-->RTIG connections via HTTP proxy

Submitter:  Petr Gotthard <gotthardp>
Submitted:  Mon 07 Jul 2008 09:11:45 AM UTC
   
 
Category:  CERTI Priority:  5 - Normal
Status:  Ready For Test Privacy:  Public
Assigned to:  gotthardp Open/Closed:  Closed

Jump to the original submission

Wed 29 Apr 2009 09:38:01 AM UTC, comment #8: 

Will be in CERTI 3.3.2 release

Eric NOULARD <erk>
Group administrator
Sun 12 Oct 2008 11:49:18 AM UTC, comment #7: 

Done.

Thank you, Eric.

Petr Gotthard <gotthardp>
Group Member
Sat 11 Oct 2008 09:59:43 PM UTC, comment #6: 

I forgot to say it explicitly:

go ahead apply the patch.

Eric NOULARD <erk>
Group administrator
Sat 11 Oct 2008 09:59:06 PM UTC, comment #5: 

OK Petr,

I think your current patch is fine from structural
point of view. I have no time to test it against
a real HTTP proxy for functionnal testing but
I'm confident you will do that :=)

Some remarks:

1) May be createConnection should go up to Socket as a pure
   virtual method in order to enforce this specification
   for other Socket subclasses but this is a detail.

2) You miss 2 includes in SocketHTTPProxy.cc
   #include <cstdlib>
   #include <cstring>

gcc 4.3.2 is complaining about undefined atoi and strXXX
functions.

3) Concerning our beautiful debug mean:
   static pdCDebug D("SOCKHTTP", "(SocketHTTPProxy) - ");
   static PrettyDebug G("GENDOC",__FILE__);

   Beside the fact that we should replace PrettyDebug with
   something else, you may already replace
   pdCDebug with PrettyDebug because
   pdCDebug is an ancient class which is now
   typedef PrettyDebug pdCDebug ;


Eric NOULARD <erk>
Group administrator
Sat 11 Oct 2008 06:32:16 PM UTC, comment #4: 

I'll try to review your new patch tonight
but I'm experiencing bad weather locally which leads
to network cut too :=(

If I do not manage to do it tonight
I'll be able to do it on monday.

Eric NOULARD <erk>
Group administrator
Sat 11 Oct 2008 04:34:13 PM UTC, comment #3: 

Eric,
I removed the multiple inheritance from CVS, so here goes a second proposal for the HTTP proxy functionality.

I decided to make a small step only: it's not possible to have tunnel over SSL. [The SSL is "nice to have" only, while http tunnels is a Prio 1.]

 - The SocketTCP is subclassed as you proposed
 - I had to make some SocketTCP members protected; this will have to be enhanced to introduce SSL, but currently it's sufficient.
 - The Communications.cc decides what class to create, but the specific ENV variables are read in the subclass; this is to have all proxy-related code in one file


(file #16653)

Petr Gotthard <gotthardp>
Group Member
Mon 07 Jul 2008 07:08:47 PM UTC, comment #2: 

Eric,
first of all, I also don't like the multiple inheritance.

I didn't use subclassing to avoid dirty multiple inheritance
when building SSL/TCP and SSL/Tunnel/TCP.
What you're proposing would require a re-design of the Socket interface: you need to enable applications (RTIA) to use a socket (TCP) and you also also want to enable other sockets (SSL) to connect to it and build some kind of a stack.

I need to think about that.

Petr

Petr Gotthard <gotthardp>
Group Member
Mon 07 Jul 2008 04:23:38 PM UTC, comment #1: 

Hi Petr,

I need more time in order to revuiew the patch
but I would say that I would be more satisfied
with a subclassing of TCPSocket.

Communication may decide to instantiate the
appropriate socket type depending on the
proxy or not proxy usage.

In fact I don't like the current multiple inheritance
usage of the Communication object, because in this
case it forces us to complexify the SocketTCP class.

I'd rather have a Communication object which contain
some (possibly derived) socket objects which are
instantaite depending on local configuration
(taken from env var or config file).

Today we have 1 TCP and 1 UDP but we may have more
(or less) depending on the transport mean at disposal.
Imagine we want an SSH tunnel or compression or
kind of high speed non-socket connection....
We may even imagine peer-to-peer connection.

Concerning env var we may use
CERTI_HTTP_PROXY if defined
http_proxy if defined
none is none is defined

I find it better to try to support commonly
used env var.
One may set CERTI_HTTP_PROXY to None
if he wants to set http_proxy but does
not want CERTI to use this proxy.

Regarding the order of the argument
I do not see a good reason to keep it.

Eric NOULARD <erk>
Group administrator
Mon 07 Jul 2008 09:11:45 AM UTC, original submission:  

Hello!

Any comments to this patch?

I added new (optional) environment variable CERTI_HTTP_PROXY indicating that RTIA should use HTTP tunnel to RTIG. We may also use a standard environment variable "http_proxy", but I think that a CERTI specific is better. Or?

The connection functions use parameters in an unusal order (PORT, SERVER). The new functions use a more common order (SERVER, PORT). Is there any reason to keep the old order?

Petr Gotthard <gotthardp>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #16653:  proxy.zip added by gotthardp (4KiB - application/zip - Second proposal.)
file #16015:  proxy.patch added by gotthardp (9KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by erk (Posted a comment)
  • -email is unavailable- added by gotthardp (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-04-29 erk Open/ClosedOpen Closed
    2008-10-12 gotthardp CategoryNone CERTI
        StatusIn Progress Ready For Test
    2008-10-11 gotthardp Attached File- Added proxy.zip, #16653
    2008-07-07 gotthardp Attached File- Added proxy.patch, #16015

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code