Closed ccutrer closed 9 years ago
Both comments addressed, and new version pushed
LGTM. Will merge after travis goes green.
the build isn't going green here....
it's green now. travis was using a strict C compiler (no variable declarations after statements) that my local compiler was okay with
Thanks for fixing. Yeah...I kinda, sorta, wanna just make this c99 but don't have the energy to do it.
Did you want me to release a new version of this too? Also, if you're going to continue hacking on it, wanna become a maintainer? You clearly seem to understand the xmlsec and C bits
No, I don't need a new version - the debugging isn't critical to a released version of anything, and I'm planning (hoping) to do some work on the verification side as well. As for maintainer - if you want, but it's really great to have another pair of eyes look things over before submitting anything.
Cool. I'll hold off on releasing then. Regarding maintainer...I think that regardless of whether or not one is a maintainer, any patch should still go through the same pull-request flow where someone reviews it before it goes in. The hope in making you maintainer was to give you the right to release the gem if necessary rather than blocking on me. I'm no longer on a project that actively uses this piece of code and thus will always be slower than ideal in doing things here cause I won't have an up-to-date ruby environment, etc. :(
Also, just noticed your ruby-saml2 project. Awesome! FYI, you might want to look at https://github.com/omb-awong/saml_idp (that's me too). It's a major overhaul of sportngin/saml_idp...probably 60-80% scrubbed.
The code is a bit of a mess. Both that and this project were done as part of a rush release. Thus I don't think you want to grab much of it verbatim however I implemented large chunks of the protocol from spec and added citations into the comments. Example here: https://github.com/omb-awong/saml_idp/blob/dev/lib/saml_idp/assertion_builder.rb
Also, I added validation against the official OASIS SAML2 Schemas into most of the spec and avoided purely gold-test style spec. Example here: https://github.com/omb-awong/saml_idp/blob/dev/spec/lib/saml_idp/assertion_builder_spec.rb#L38
I think the comments, the xsd validation, and some of the rspec-xsd patterns for testings are worth looking at and/or snagging...
matches the --store-references option to xmlsec1 utility