Yair Zaslavsky has posted comments on this change. Change subject: core: remove commons httpclient from provider proxy ......................................................................
Patch Set 14: (3 comments) http://gerrit.ovirt.org/#/c/33458/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHostProviderProxy.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHostProviderProxy.java: Line 381: if (hostProvider.getUsername() != null && hostProvider.getPassword() != null) { Line 382: connection.setRequestProperty( Line 383: "Auhtorization", Line 384: String.format("Basic %1$s", Line 385: Base64.encodeBase64( > oh... this is not good, as long password will wrap. Done Line 386: String.format("%1$s:%2$s", hostProvider.getUsername(), hostProvider.getPassword()).getBytes(Charset.forName("UTF-8"))) Line 387: ) Line 388: .toString()); Line 389: } http://gerrit.ovirt.org/#/c/33458/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/BaseProviderProxy.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/BaseProviderProxy.java: Line 75 Line 76 Line 77 Line 78 Line 79 > again... why do we need this? see previous comments on this. Why do we want to call setters related to HTTPS at the builder if not needed? And also with certificate chain retrieval - if non secured, the retrieved chain should be null. This is used by GetProviderCertificateChainQuery Line 41: Line 42: final List<X509Certificate> chain = new ArrayList<X509Certificate>(); Line 43: HttpURLConnection conn = null; Line 44: try { Line 45: conn = new HttpURLConnectionBuilder(url).create(chain); > what is it good if chain is always empty? chain is potentially being populated inside HttpURLConnectionBuilder. What did you mean by the last sentence? Line 46: return chain; Line 47: } catch (SSLHandshakeException e) { Line 48: return chain; Line 49: } catch (IOException | GeneralSecurityException e) { -- To view, visit http://gerrit.ovirt.org/33458 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I727d34c33f357b93560d4b5a1784b3733b7fa293 Gerrit-PatchSet: 14 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
