> On June 24, 2014, 11:10 p.m., Demetrius Tsitrelis wrote: > > The only change I can see regarding "SSL enabling" is to the getProperty() > > method for the default value. It looks like that change to null wouldn't > > matter as the constructor for CloudStackApi() would just reassign 8080? > > Dmitry Batkovich wrote: > Maybe I don't understand you but states of properties > "cloudAPISSLEnabled" and "cloudAPIPort" are independent. And port value not > depends on "cloudAPISSLEnabled" value. (I will assume that you look at not my > code) > > Demetrius Tsitrelis wrote: > Sorry Dmitry - I missed a section of code. > > Related to the port number changes you made is that now in the > constructor for CloudstackClient there is no way for a user to set the port > if SSL is used; you have hard-coded port 443. > > Dmitry Batkovich wrote: > No, I didn't ,see > https://hc.apache.org/httpclient-3.x/apidocs/org/apache/commons/httpclient/HostConfiguration.html#setHost(java.lang.String, > int, org.apache.commons.httpclient.protocol.Protocol) > > > Demetrius Tsitrelis wrote: > I mean this change: > > if (sslEnabled) { > httpClient.getHostConfiguration().setHost(host, port, new > Protocol("https", new EasySSLProtocolSocketFactory(), 443)); > } else { > httpClient.getHostConfiguration().setHost(host, port); > } > >
After more careful evaluation of the HttpHost constructor (http://grepcode.com/file/repo1.maven.org/maven2/commons-httpclient/commons-httpclient/3.1/org/apache/commons/httpclient/HttpHost.java#HttpHost), I gladly admit I was wrong since as long as the "port" parameter is non-negative it's value will be set into the equivalent instance member; only negative port values would cause the Protocol port value to be used. Good then! - Demetrius ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21776/#review46589 ----------------------------------------------------------- On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21776/ > ----------------------------------------------------------- > > (Updated May 27, 2014, 8:04 p.m.) > > > Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi > Damle. > > > Repository: cloudstack-git > > > Description > ------- > > * CloudStackClient.java http request mechanism replaced from GET requests to > POST for supporting EC2 requests larger than 2KB > * SSL enabling fixed in EC2Engine.java > > continuation of https://reviews.apache.org/r/17586/ > > > Diffs > ----- > > awsapi/conf/ec2-service.properties.in 82f5ad8 > awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214 > awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea > awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION > awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210 > awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96 > awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION > awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68 > awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION > awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a > awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59 > awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION > > Diff: https://reviews.apache.org/r/21776/diff/ > > > Testing > ------- > > > Thanks, > > Dmitry Batkovich > >
