Alon Bar-Lev 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. should be new Base64(0).encodeToString() 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? 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? ok, now I beginning to understand...? we chain the external trust store which is ours and the default jre. not sure even previous implementation is correct, as chain is empty also, right? oh! this is bad, it modify the chain during run. so I do not know what it did. please remove the chain handling it is totally incorrect. the only chain verification should be done by set trust store of the connection. 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
