Skip to content

fix(RocketChatClientCallBuilder): fix proxy access without credentials (JENKINS-55890)#30

Merged
hypery2k merged 1 commit intojenkinsci:developfrom
famod:JENKINS-55890-fix-and-refactoring
Feb 1, 2019
Merged

fix(RocketChatClientCallBuilder): fix proxy access without credentials (JENKINS-55890)#30
hypery2k merged 1 commit intojenkinsci:developfrom
famod:JENKINS-55890-fix-and-refactoring

Conversation

@famod
Copy link
Contributor

@famod famod commented Jan 31, 2019

Including refactoring to improve code quality
and to reduce complexity of different execution paths.

Also fixes:
org.apache.http.conn.UnsupportedSchemeException: http protocol is not supported
when using proxy (with or without auth) and trustSSL is active.

Also switches from deprecated SSLSocketFactory to SSLConnectionSocketFactory.

https://issues.jenkins-ci.org/browse/JENKINS-55890

…s (JENKINS-55890)

Including refactoring to improve code quality
and to reduce complexity of different execution paths.

Also fixes:
org.apache.http.conn.UnsupportedSchemeException: http protocol is not supported
when using proxy (with or without auth) and trustSSL is active.

Also switches from deprecated SSLSocketFactory to SSLConnectionSocketFactory.
@famod
Copy link
Contributor Author

famod commented Jan 31, 2019

Please note that I did not create new tests because I did not introduce new features.
Nevertheless, test coverage in this area should be improved - especially regarding proxy handling!

PS: Credits to my colleague @uhanisch whom I worked with on this issue.

@hypery2k
Copy link
Contributor

thanks for the PR, but actually the integration test is failing. Will try to find time next week to check

@famod
Copy link
Contributor Author

famod commented Jan 31, 2019

The failures on travis don't seem related to my actual code changes:

[ERROR] jenkins.plugins.rocketchatnotifier.rocket.RocketChatClientCallBuilderIT  Time elapsed: 0.36 s  <<< ERROR!
java.lang.NoSuchMethodError: org.junit.rules.Timeout.seconds(J)Lorg/junit/rules/Timeout;
	at org.jvnet.hudson.test.JenkinsRule.apply(JenkinsRule.java:579)
	at org.junit.rules.RunRules.applyAll(RunRules.java:24)
	at org.junit.rules.RunRules.<init>(RunRules.java:13)
	at org.junit.runners.BlockJUnit4ClassRunner.withTestRules(BlockJUnit4ClassRunner.java:376)
	at org.junit.runners.BlockJUnit4ClassRunner.withRules(BlockJUnit4ClassRunner.java:331)
	at org.junit.runners.BlockJUnit4ClassRunner.methodBlock(BlockJUnit4ClassRunner.java:248)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:300)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)

@hypery2k hypery2k merged commit c5a8b34 into jenkinsci:develop Feb 1, 2019
@hypery2k
Copy link
Contributor

hypery2k commented Feb 1, 2019

thanks for the PR, I'll check for the test errors

@famod
Copy link
Contributor Author

famod commented Feb 1, 2019

@hypery2k thanks for merging! In case I find the time for this: What would be right approach for this project/plugin to test proxy access? Something docker-based? Or just a java proxy implementation that is embedded into the unit/integration test like https://github.com/adamfisk/LittleProxy?

@famod famod deleted the JENKINS-55890-fix-and-refactoring branch February 1, 2019 15:08
@hypery2k
Copy link
Contributor

hypery2k commented Feb 1, 2019

I like the Java based integration test approach. Feel free to send a PR. Any help is appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants