You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by David Engberg <de...@evernote.com> on 2008/09/11 23:05:28 UTC

Patch: add https support to PHP THttpClient

The trunk PHP library doesn't support HTTPS transport via 
THttpClient.php.  I've attached a relatively small change to this class 
that adds an optional fourth constructor parameter ('protocol') which 
can be used to specify 'http' or 'https'.

Usage:

    $transport = new THttpClient('www.evernote.com', 80, '/edam/user', 
'http');
    $sslTransport = new THttpClient('www.evernote.com', 443, 
'/edam/user', 'https');

This change should be backward compatible with existing code:

    $transport = new THttpClient('www.evernote.com', 80, '/edam/user');


Please let me know if this mailing list isn't the right way to submit 
patches ... I somehow managed to lose track of the mailing list when it 
went into incubation at Apache, so I'm a few months out of it.

Thanks

Re: Patch: add https support to PHP THttpClient

Posted by Dave Engberg <de...@evernote.com>.
Ack ... I was hitting the old SVN repository (since that's what comes up 
when you search for "thrift svn" on Google).

My patch is arguably slightly better, but the current head is good 
enough for our purposes, thanks.


Dave Engberg wrote:
>
> I don't feel qualified to comment on the first part of your email, but 
> I'm looking at the trunk head, and that still seems to hard-code in 
> 'http://' :
> http://svn.facebook.com/svnroot/thrift/trunk/lib/php/src/transport/THttpClient.php 
>
>
> I think my patch might be slightly cleaner, since it allows you to 
> specify the port correctly.  The git diff you sent wouldn't let me use 
> SSL on any port other than 443, and I'd need to construct the https 
> THttpClient with a dummy port=80 to work around the hard-coded 
> assumptions in the rest of the class:
> $trans = new THttpClient('www.evernote.com', 80, '/foo', 'https');
>
>
>
> David Reiss wrote:
>> Am I just crazy, or don't we already have this feature?
>> http://gitweb.thrift-rpc.org/?p=thrift.git;a=commitdiff;h=2c6b42c63c0e757e6b855409804543c059fbbc9b 
>>
>>
>> Mark Slee wrote:
>>  
>>> Thanks Dave, this looks fine to me.
>>>
>>> The official process is to create a JIRA ticket and attach the patch to
>>> it here:
>>> http://issues.apache.org/jira/browse/THRIFT
>>>
>>> Cheers,
>>> Mark
>
>

Re: Patch: add https support to PHP THttpClient

Posted by Dave Engberg <de...@evernote.com>.
I don't feel qualified to comment on the first part of your email, but 
I'm looking at the trunk head, and that still seems to hard-code in 
'http://' :
http://svn.facebook.com/svnroot/thrift/trunk/lib/php/src/transport/THttpClient.php

I think my patch might be slightly cleaner, since it allows you to 
specify the port correctly.  The git diff you sent wouldn't let me use 
SSL on any port other than 443, and I'd need to construct the https 
THttpClient with a dummy port=80 to work around the hard-coded 
assumptions in the rest of the class:
$trans = new THttpClient('www.evernote.com', 80, '/foo', 'https');



David Reiss wrote:
> Am I just crazy, or don't we already have this feature?
> http://gitweb.thrift-rpc.org/?p=thrift.git;a=commitdiff;h=2c6b42c63c0e757e6b855409804543c059fbbc9b
>
> Mark Slee wrote:
>   
>> Thanks Dave, this looks fine to me.
>>
>> The official process is to create a JIRA ticket and attach the patch to
>> it here:
>> http://issues.apache.org/jira/browse/THRIFT
>>
>> Cheers,
>> Mark

RE: Patch: add https support to PHP THttpClient

Posted by Mark Slee <ms...@facebook.com>.
And... apparently we do.

-----Original Message-----
From: David Reiss [mailto:dreiss@facebook.com] 
Sent: Thursday, September 11, 2008 2:24 PM
To: thrift-dev@incubator.apache.org
Subject: Re: Patch: add https support to PHP THttpClient

Am I just crazy, or don't we already have this feature?
http://gitweb.thrift-rpc.org/?p=thrift.git;a=commitdiff;h=2c6b42c63c0e75
7e6b855409804543c059fbbc9b

Mark Slee wrote:
> Thanks Dave, this looks fine to me.
> 
> The official process is to create a JIRA ticket and attach the patch 
> to it here:
> http://issues.apache.org/jira/browse/THRIFT
> 
> Cheers,
> Mark
> 
> 
> -----Original Message-----
> From: David Engberg [mailto:dengberg@evernote.com]
> Sent: Thursday, September 11, 2008 2:05 PM
> To: thrift-dev@incubator.apache.org
> Subject: Patch: add https support to PHP THttpClient
> 
> 
> The trunk PHP library doesn't support HTTPS transport via 
> THttpClient.php.  I've attached a relatively small change to this 
> class that adds an optional fourth constructor parameter ('protocol') 
> which can be used to specify 'http' or 'https'.
> 
> Usage:
> 
>     $transport = new THttpClient('www.evernote.com', 80, '/edam/user',

> 'http');
>     $sslTransport = new THttpClient('www.evernote.com', 443, 
> '/edam/user', 'https');
> 
> This change should be backward compatible with existing code:
> 
>     $transport = new THttpClient('www.evernote.com', 80, 
> '/edam/user');
> 
> 
> Please let me know if this mailing list isn't the right way to submit 
> patches ... I somehow managed to lose track of the mailing list when 
> it went into incubation at Apache, so I'm a few months out of it.
> 
> Thanks
>

Re: Patch: add https support to PHP THttpClient

Posted by David Reiss <dr...@facebook.com>.
Am I just crazy, or don't we already have this feature?
http://gitweb.thrift-rpc.org/?p=thrift.git;a=commitdiff;h=2c6b42c63c0e757e6b855409804543c059fbbc9b

Mark Slee wrote:
> Thanks Dave, this looks fine to me.
> 
> The official process is to create a JIRA ticket and attach the patch to
> it here:
> http://issues.apache.org/jira/browse/THRIFT
> 
> Cheers,
> Mark
> 
> 
> -----Original Message-----
> From: David Engberg [mailto:dengberg@evernote.com]
> Sent: Thursday, September 11, 2008 2:05 PM
> To: thrift-dev@incubator.apache.org
> Subject: Patch: add https support to PHP THttpClient
> 
> 
> The trunk PHP library doesn't support HTTPS transport via
> THttpClient.php.  I've attached a relatively small change to this class
> that adds an optional fourth constructor parameter ('protocol') which
> can be used to specify 'http' or 'https'.
> 
> Usage:
> 
>     $transport = new THttpClient('www.evernote.com', 80, '/edam/user',
> 'http');
>     $sslTransport = new THttpClient('www.evernote.com', 443,
> '/edam/user', 'https');
> 
> This change should be backward compatible with existing code:
> 
>     $transport = new THttpClient('www.evernote.com', 80, '/edam/user');
> 
> 
> Please let me know if this mailing list isn't the right way to submit
> patches ... I somehow managed to lose track of the mailing list when it
> went into incubation at Apache, so I'm a few months out of it.
> 
> Thanks
>

RE: Patch: add https support to PHP THttpClient

Posted by Mark Slee <ms...@facebook.com>.
Thanks Dave, this looks fine to me.

The official process is to create a JIRA ticket and attach the patch to
it here:
http://issues.apache.org/jira/browse/THRIFT

Cheers,
Mark
 

-----Original Message-----
From: David Engberg [mailto:dengberg@evernote.com] 
Sent: Thursday, September 11, 2008 2:05 PM
To: thrift-dev@incubator.apache.org
Subject: Patch: add https support to PHP THttpClient


The trunk PHP library doesn't support HTTPS transport via
THttpClient.php.  I've attached a relatively small change to this class
that adds an optional fourth constructor parameter ('protocol') which
can be used to specify 'http' or 'https'.

Usage:

    $transport = new THttpClient('www.evernote.com', 80, '/edam/user',
'http');
    $sslTransport = new THttpClient('www.evernote.com', 443,
'/edam/user', 'https');

This change should be backward compatible with existing code:

    $transport = new THttpClient('www.evernote.com', 80, '/edam/user');


Please let me know if this mailing list isn't the right way to submit
patches ... I somehow managed to lose track of the mailing list when it
went into incubation at Apache, so I'm a few months out of it.

Thanks