-
Notifications
You must be signed in to change notification settings - Fork 15
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.
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
Play 3.0.0 upgrade & vulnerabilty fixes #114
Conversation
…via direct dependencies
`play-git-hub` makes use of internal OkHttp APIs which have been removed in 4.x
a2ad7de
to
5e317bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM.
This app lives in Heroku, which I think @rtyley manages, and who is best placed to give a 👍🏽.
This upgrade was prompted by guardian/prout#114, where Ash noted that updating to OkHttp 4 was blocked by `play-git-hub`s use of OkHttp 's internal `HttpDate` class. Thankfully the `java.time` package offers the `RFC_1123_DATE_TIME` date time formatter, which looks to be a good substitute for parsing the `Date` header returned by the GitHub API. https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html#RFC_1123_DATE_TIME
Hi @AshCorr thanks so much for this! I guess the list of vulnerabilities we're looking at here is coming from https://app.snyk.io/org/guardian-devtools/project/1bc32f14-0b81-428e-b01b-9942ba45885c, is that right?
I've prepared rtyley/play-git-hub#7 to get Regarding Play 2.9, it looks like there's a bit of a question about whether this is officially 'released' or not yet?
I can see that it's coming, and I definitely want to upgrade to it when it is, but has there been a proper release statement yet? Is adopting 2.9 this week, rather than next week, a bit early - or is this driven by a Snyk vuln fixed by Play 2.9? |
This upgrade was prompted by guardian/prout#114, where Ash noted that updating to OkHttp 4 was blocked by `play-git-hub`s use of OkHttp 's internal `HttpDate` class. Thankfully the `java.time` package offers the `RFC_1123_DATE_TIME` date time formatter, which looks to be a good substitute for parsing the `Date` header returned by the GitHub API. https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html#RFC_1123_DATE_TIME
This upgrade was prompted by guardian/prout#114, where Ash noted that updating to OkHttp 4 was blocked by `play-git-hub`s use of OkHttp 's internal `HttpDate` class. Thankfully the `java.time` package offers the `RFC_1123_DATE_TIME` date time formatter, which looks to be a good substitute for parsing the `Date` header returned by the GitHub API. https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html#RFC_1123_DATE_TIME
This upgrade was prompted by guardian/prout#114, where Ash noted that updating to OkHttp 4 was blocked by `play-git-hub`s use of OkHttp 's internal `HttpDate` class. Thankfully the `java.time` package offers the `RFC_1123_DATE_TIME` date time formatter, which looks to be a good substitute for parsing the `Date` header returned by the GitHub API. https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html#RFC_1123_DATE_TIME
Sounds like its a stable release, just no release statement yet. Should hopefully come out today if we want to wait for merging this PR! EDIT: It's out! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, merging now! Prout has CD, so will auto-deploy after merging.
This is working well on PROD! |
What does this change?
The last vuln is a total pain to fix - it involves updating okhttp to 4.x from 3.x, this should have been easy but
com.madgag.play-git-hub
makes use of an internal OkHttp package which isn't available anymore in okhttp 4.xHow to test
sbt compile
compiles fine.snyk test
before:snyk test
after: