1287774 - ldapjdk needs to support IPv6 addresses

Status: RESOLVED FIXED

(Directory :: LDAP C SDK, defect)

Description

[Mark Reynolds]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160606100503

Steps to reproduce:

Use a java client application that uses ldapjdk to connect to a Directory Server.  In this case I am using the 389-console package found on a Fedora OS.


Actual results:

When setting LDAP referrals each character that is entered is validated by the ldpajdk, but it only expects IPv4 characters.  So when you enter a colon, or a bracket, the value becomes invalid.


Expected results:

The ldapjdk should allow the characters needed for IPv6 addresses.  This also includes the ldap URL format for IPv6, rfc 2732:

ldap://[IPv6 Address]:389

Comment 1

[Mark Reynolds]

Proposed patch coming soon...

Comment 2

[Mark Reynolds]

Attachment: bug1287774.diff

hg diff for ldapjdk fix for IPv6 addresses

Comment 3

[Mark Reynolds]

I tried this, but it failed:

hg push review
pushing to ssh://reviewboard-hg.mozilla.org/autoreview
searching for appropriate review repository
abort: no review repository found

So the attached patch is all I got.

Comment 4

[Mark Reynolds]

This is the repo I was working on:

hg clone https://hg.mozilla.org/projects/ldap-sdks/

Comment 5

[Gregory Szorc [:gps]]

The https://hg.mozilla.org/projects/ldap-sdks repo isn't configured in MozReview because practically nobody uses it. You'll have to ask for review the old fashioned way.

I'll adjust the Bugzilla state to direct this to someone appropriate...

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8773799 [details] [diff] [review]
bug1287774.diff

glandium reviewed the last commit to this repo. So assigning review to him.

Comment 7

[Mark Reynolds]

Attachment: bug1287774.diff

Revised patch based off of review by [e-mail]

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8774944 [details] [diff] [review]
bug1287774.diff

Review of attachment 8774944 [details] [diff] [review]:
-----------------------------------------------------------------

I only reviewed build system changes in that repository. I don't know who can review java changes there, or if there's actual interest in taking fixes for the java code there. I think this is largely unmaintained. I think the closest module associated with this code is https://wiki.mozilla.org/Modules/All#Directory_SDK . I haven't heard from any of the people listed there in years. The latest thread on dev-tech-ldap mentioning Java or JDK was in 2011.

Comment 9

[nkinder]

(In reply to Mike Hommey [:glandium] from comment #8)
> I don't know who
> can review java changes there, or if there's actual interest in taking fixes
> for the java code there. I think this is largely unmaintained. I think the
> closest module associated with this code is
> https://wiki.mozilla.org/Modules/All#Directory_SDK . I haven't heard from
> any of the people listed there in years. The latest thread on dev-tech-ldap
> mentioning Java or JDK was in 2011.

I've previously worked on both the LDAP JDK and the LDAP C SDK (though much of my history is goign to be in CVS and not Mercurial).  They are largely unmaintained, but they are still used.  I think that this is a worthwhile change to accept.  Noriko Hosoi ([e-mail]) was also a previous committer/reviewer, and she has already reviewed this patch outside of this bug.

Comment 10

[nkinder]

Comment on attachment 8774944 [details] [diff] [review]
bug1287774.diff

Review of attachment 8774944 [details] [diff] [review]:
-----------------------------------------------------------------

The patch looks fine, and this seems like a good addition given that IPv6 usage is becoming much more common.

Comment 11

[Rich Megginson]

https://bugzilla.mozilla.org/attachment.cgi?id=8774944&action=diff#a/java-sdk/ldapjdk/netscape/ldap/LDAPUrl.java_sec3

looks like the indentation is off at line 190

Other than that, LGTM

Comment 12

[Mark Reynolds]

Attachment: bug1287774.diff

Revised patch with indentation fix

Comment 13

[Mark Reynolds]

(In reply to Rich Megginson from comment #11)
> https://bugzilla.mozilla.org/attachment.cgi?id=8774944&action=diff#a/java-
> sdk/ldapjdk/netscape/ldap/LDAPUrl.java_sec3
> 
> looks like the indentation is off at line 190
> 
> Other than that, LGTM

New patch attached that fixed the indentation

Comment 14

[Mark Reynolds]

Attachment: bug1287774.diff

Fixed indentation

Comment 15

[Mark Reynolds]

Attachment: bug1287774.diff

Fixed indentation

Comment 16

[Elio Maldonado]

Rich, could you commit it? Thanks.

Comment 17

[Rich Megginson]

pushed

remote:   https://hg.mozilla.org/projects/ldap-sdks/rev/f9d7552c8188