Skip to content

Conversation

@mtodd
Copy link
Member

@mtodd mtodd commented Oct 22, 2014

According to the spec (http://tools.ietf.org/html/rfc4511#section-5.1), we're currently encoding true incorrectly as \x01\x01\x01 instead of \x01\x01\xFF.

Fixes #31.

Wouldn't mind finding a good integration test for this, but nothing obvious or direct comes to mind.

cc @jch @schaary

mtodd added 2 commits October 22, 2014 00:18
We allow this since the library requires Ruby 1.9+
@mtodd
Copy link
Member Author

mtodd commented Oct 22, 2014

Ruby 1.8.7 has been retired for well over a year. Moving ahead, we should assume 1.9+ only.

Once this is merged and released, it will mark the first release that does not support ~1.8.7. We should make that explicit by setting appropriate Gemspec flags.

This was referenced Oct 22, 2014
@jch
Copy link
Member

jch commented Oct 22, 2014

ruby 1.9.3+ requirement enforced in #145

@jch
Copy link
Member

jch commented Oct 22, 2014

I think this is sufficiently covered by the search integration test since the default size limit is '0'.to_ber. If your encoding was wrong, then search would break. Yes, the test is implicit, but it's such a common value it'd be pretty obvious if it broke.

@mtodd
Copy link
Member Author

mtodd commented Oct 22, 2014

@jch size isn't a boolean, though, since this PR only applies to the BER encoding of the true value. This would apply for the "criticality" flag of a search control, and few other places (not much boolean usage in the spec from what I've seen).

@mtodd
Copy link
Member Author

mtodd commented Oct 22, 2014

Luckily for us, I think all we have to do is use a non-existing sort control and set the criticality to true, then assert that the response is "unavailable critical extension" (result code 12).

@jch
Copy link
Member

jch commented Oct 22, 2014

Oops, my brain just autocompleted boolean to 0 and 1, but ya what you said makes sense.

@mtodd
Copy link
Member Author

mtodd commented Oct 24, 2014

@jch ready for your review.

@jch
Copy link
Member

jch commented Oct 24, 2014

🚢

mtodd added a commit that referenced this pull request Oct 24, 2014
@mtodd mtodd merged commit 7b801fc into master Oct 24, 2014
@mtodd mtodd deleted the true-ber branch October 24, 2014 23:39
astratto pushed a commit to astratto/ruby-net-ldap that referenced this pull request Dec 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proper BER value for "true"

3 participants