Skip to content

Upgrade to CraSH 1.3.1 #2425

New issue

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

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

Already on GitHub? # to your account

Closed
wilkinsona opened this issue Jan 28, 2015 · 17 comments
Closed

Upgrade to CraSH 1.3.1 #2425

wilkinsona opened this issue Jan 28, 2015 · 17 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

The Crashub project appears to be dormant. The latest release, 1.3.0, was made in June 2014 and no commits (other than adding Google analytics to their javadoc) have been made since. Some of its dependencies are getting rather old, for example the version of Bouncycastle it uses is no longer supported and is causing problems.

@snicoll
Copy link
Member

snicoll commented Jan 28, 2015

hold on a minute. I remember I discussed this with @vietj and he specifically did something regarding bouncycastle in one of the RC. Or am I totally misunderstanding the problem?

@wilkinsona
Copy link
Member Author

PEMReader is PEMParser in Bouncycastle 1.5. PEMParser extends PemReader (note the different capitalisation)

@snicoll
Copy link
Member

snicoll commented Jan 29, 2015

well, ok. I can see what I can do on the CRaSH side but it all boils down (like I wrote in the SO issue) to the way the Bouncycastle team manages their backward compatibility.

we can upgrade but then someone else on 1.49- will complain that it's broken for him. What can we do, really?

@wilkinsona
Copy link
Member Author

Indeed. There's no obvious right answer here.

@snicoll
Copy link
Member

snicoll commented Jan 29, 2015

@aheritier
Copy link
Contributor

Hi,

The project isn't actively maintained but always alive.
This bouncy castle backward compete issue is a real problem. Last time I checked about this issue there were many libraries using bouncycastle < 1.50 thus it will be difficult to satisfy everyone. One solution may be to provide different modules and let the user select the one he want to use but it won't be easy to maintain. If you have any idea/proposal, don't hesitate.

Cheers

@wilkinsona
Copy link
Member Author

@aheritier How about falling back to using reflection when PEMReader is not on the classpath? Like this:

private Object readKey(File keyFile) throws Exception {
  try {
    PEMReader r = new PEMReader(new FileReader(keyFile));
    try {
      return r.readObject();
    } finally {
      r.close();
    }
  } catch (NoClassDefFoundError e) {
    Class<?> pemParserClass = Class.forName("org.bouncycastle.openssl.PEMParser");
    Reader reader = new FileReader(keyFile);
    Reader pemReader = (Reader) pemParserClass.getConstructor(Reader.class).newInstance(reader);
    try {
      return pemParserClass.getMethod("readObject").invoke(pemReader);
    } finally {
      pemReader.close();
    }
  }
}

We do something similar in a few places in Boot where we need to cope with breaking API changes

@aheritier
Copy link
Contributor

@wilkinsona yes for sure it is a good solution, but is it the only incompatible API change? We need to investigate.
Quickly checking for this issue I noticed that it wasn't the first time we had it with BC :(
In the past for https://jira.exoplatform.org/browse/CRASH-188 there was another incompatibility between bouncycastle 1.46 and 1.47. We decided to keep crash 1.2.x with BC 1.146 and to do the upgrade to 1.47 (and 1.49) in 1.3.0. The problem was a little bit different also because BC changed few artifacts coordinates.

@nttuyen
Copy link

nttuyen commented Jan 30, 2015

I prefer and agree with the solution of @wilkinsona and I think this problem is only incompatible API changes. We will take a look more and provide the fix next week.

@aheritier
Copy link
Contributor

I applied the fix on crash master codebase. @wilkinsona @snicoll Can we verify together that it solves your issue before releasing a 1.3.1 with this fix ?

@wilkinsona
Copy link
Member Author

@aheritier Do you publish snapshots anywhere or should I build from source?

@aheritier
Copy link
Contributor

@wilkinsona Can you rebuild it from source please ?
There is a CI job (https://vietj.ci.cloudbees.com/job/CRaSH/) but I don't have the hand on it and it seems it doesn't deploy its binaries in a repository (ping @vietj ). I will try to fix this ASAP.

@wilkinsona
Copy link
Member Author

@aheritier 1.3.1-SNAPSHOT looks good to me with both 1.49 and 1.51 of Bouncycastle. Thanks!

@vietj
Copy link

vietj commented Feb 5, 2015

awesome, bravo @aheritier :-)

@aheritier
Copy link
Contributor

I didn't do anything awesome. All greetings are for @wilkinsona and @nttuyen
Thus now let's release it
Note : I succeeded to automate the deployment of snapshots on oss.sonatype .org : https://ci.exoplatform.org/job/crsh-master-ci/

@aheritier
Copy link
Contributor

1.3.1 is out @wilkinsona . HTH.
Cheers.

@philwebb
Copy link
Member

philwebb commented Feb 6, 2015

Sweet. Thanks!

@philwebb philwebb changed the title What should we do with the Crashub starter? Upgrade to CraSH 1.3.1 Feb 6, 2015
@philwebb philwebb added type: enhancement A general enhancement and removed discussion labels Feb 6, 2015
@philwebb philwebb added this to the 1.2.2 milestone Feb 6, 2015
@wilkinsona wilkinsona modified the milestones: 1.1.11, 1.2.2 Feb 9, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants