Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[#2089 ] upgrade bouncycastle #1159

Merged
merged 8 commits into from
Sep 18, 2017
Merged

[#2089 ] upgrade bouncycastle #1159

merged 8 commits into from
Sep 18, 2017

Conversation

flybyray
Copy link
Contributor

this is a proposed pull request needs at least some other pr merged first. not all are required but i added them to test more easily from a windows system.

it adds a new tests for ssl certificate usage

@flybyray flybyray mentioned this pull request May 31, 2017
Closed
@asolntsev
Copy link
Contributor

@flybyray Which "other" PR do you mean?

@asolntsev asolntsev self-assigned this Jun 1, 2017
@flybyray
Copy link
Contributor Author

flybyray commented Jun 1, 2017

#1155

all others are not needed for this bouncycastle update. i just added them for myself testing it on windows os system. hence they are only needed if you like play be portable to windows os.

since you merged #1155 the pr #1159 could be an independent pr with a single commit

@flybyray flybyray force-pushed the proposed-lighthouse-2089-missing-bouncycastle branch from 211998c to e2ae4fe Compare June 1, 2017 22:15
@flybyray
Copy link
Contributor Author

flybyray commented Jun 1, 2017 •
edited
Loading

ok looks good!
code/plugins using for example License3j(newer bouncycastle jars) do not conflict with play runtime jars.

and we have better code coverage for at least PEM cert usage. maybe keystore tests could be added in similar way.

asolntsev
asolntsev approved these changes Jun 11, 2017
View reviewed changes
Copy link
Contributor

@asolntsev asolntsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I am not able to understand if this is good change or not.

framework/dependencies.yml Outdated
@@ -42,7 +42,8 @@ require: &allDependencies
- oauth.signpost -> signpost-core 1.2.1.2
- org.apache.geronimo.specs -> geronimo-servlet_2.5_spec 1.2
- org.apache.ivy -> ivy 2.4.0
- org.bouncycastle -> bcprov-jdk15 1.46
- org.bouncycastle -> bcprov-jdk15on 157
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the right version "1.57", not "157".
See http://search.maven.org/#search%7Cgav%7C1%7Cg%3A%22org.bouncycastle%22%20AND%20a%3A%22bcprov-jdk15on%22

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. But on the original homepage you will find bcprov-jdk15on-157.jar without a period dot between numbers. see https://www.bouncycastle.org/latest_releases.html
I am not sure if all Play! dependencies are really from maven. That would be nice because of checksums and sometimes additional signatures.

@asolntsev
Copy link
Contributor

@xael-fry To be honest, I am not able to understand if this is good change or not.
So I suggest to trust the PR author and merge it.

@asolntsev asolntsev requested a review from xael-fry June 11, 2017 15:33
@asolntsev asolntsev added this to the 1.5.0 milestone Jun 11, 2017
@flybyray flybyray force-pushed the proposed-lighthouse-2089-missing-bouncycastle branch from e2ae4fe to ff4c91d Compare July 3, 2017 08:16
@flybyray
Copy link
Contributor Author

flybyray commented Jul 3, 2017 •
edited
Loading

changed the upgraded bouncycastle dependencies to its maven versions (adding the point into version number)

somehow it seems to be necessary to adjust the travis.yml with installing ant-optional

@flybyray flybyray force-pushed the proposed-lighthouse-2089-missing-bouncycastle branch 3 times, most recently from 5774635 to 5765a63 Compare July 3, 2017 11:13
@flybyray flybyray force-pushed the proposed-lighthouse-2089-missing-bouncycastle branch from 5765a63 to 5405468 Compare July 3, 2017 13:39
@flybyray flybyray mentioned this pull request Jul 18, 2017
Open
@asolntsev asolntsev merged commit f90f501 into playframework:master Sep 18, 2017
@asolntsev
Copy link
Contributor

@flybyray Thank you! Merged.

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

@asolntsev asolntsev asolntsev approved these changes

@xael-fry xael-fry Awaiting requested review from xael-fry

Assignees

@asolntsev asolntsev

Labels
None yet
Projects
None yet
Milestone
1.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@flybyray @asolntsev