Please properly close the streams and readers

[jrevillard]

Hi,

I had a lot of problems during the past few days due to the famous "Too many open files" problem. I looked at the jglobus source code and found a LOT of streams and readers that were not properly closed. I closed them and now I don't have any problem anymore.

The important commit is a8e31b2 but I think that you can apply most of them (apart a1230cd which is only for our developments).

All the Best, Jérôme

[bbockelm]

Hi Jerome,

I hit these too and have recently been added as a committer. Unfortunately, these can no longer be merged cleanly - I had separately fixed a few of the issues prior to the noticing your pull request.

Any interest in rebasing? Otherwise, I can try to pick apart all the pieces of the pull request.

I'm trying to get jglobus ready for release in one of our software products, so I'm interested in smashing as many bugs as possible.

Brian

[jrevillard]

Hi,

Sorry, I just see your comment now. Indeed, the pull request a bit old now. I will look into this but actually, as you can see in my fork, my priority is to update JGlobus to the latest boucycastle version (1.47) and to delegate most of the work to it (cert parsing etc....). It's mostly done and works pretty well (all the unit tests are ok). Of course, the curent pull request is already integrated.

Is it something that you would be interrested in merging?

I will try to rebase the current pull request (if I can).

Best, Jerome

[bbockelm]

Hi Jerome,

How large is the bouncycastle update? We need to retain compatibility with 1.45 for other users.

For a (simpler) issue with Tomcat 5/6/7, we are thinking about using Maven profiles - default to the latest code, but maintain backward compatibility via using a different profile. This will likely work for Tomcat as it only really affects a single source file.

Regardless, I'm always happy to review pull requests!

Brian

[jrevillard]

Hi Brian,

I just pull your master in our develop branch. If you diff both, you will see the needed modifications forthe BC update (+ some cleanup, import myproxy source code, add possibility to load a Globus Keystore using load(null,null))

I still have 2 problems:

1. in 'org.globus.gsi.bc.BouncyCastleCertProcessingFactory', I have commented, from line 382 to 539, the new code to generate proxy certificates because, even if all the tests are ok, the myproxy communication is not working (I get 'X509_verify_cert() failed: unable to get local issuer certificate' server side)
2. There might be abug in BC 1.47 which affect KeyUsage extension so I display a warinng for the moment in org.globus.gsi.trustmanager.X509ProxyCertPathValidator#checkKeyUsage: indeed the 'nonRepudiation bit is always set

If you have a bit of time, I would really appreciate if you could have a look at the porblem n°1... I have tested many things without success.

I hope that we will find a way to merge at some point.

Best Jerome

[bbockelm]

Hi Jerome,

Just took a look at the branch - you've been busy!

I'll be happy to start merging in things, but I'd definitely need you to do the work of breaking it up into bite-sized pieces. Is there any low-hanging fruit you can put into a pull request?

Brian

[jrevillard]

Hi Brian,

Just to let you know that point 2 is solved... is was a stupid bug I introduced. So remains the point 1... even without it anyway everything seems to be fonctional.

Now, concerning the split, I will try to do it but the BC update will have to be done in one shot I think as it has an impact mostly everywhere...

Best, Jérôme

[bbockelm]

Hi Jérôme,

I'm going to close this pull request for now. Feel free to start creating pull requests for the individual features in your dev branch (probably there will be a lot of "cherry-picking" with git!).

I will probably delay the BC 1.47 update for now - maybe make it the default for JGlobus 2.1?

Thanks for the contribution,

Brian