grnq / joid

Java OpenID = JOID OpenID providers and relying parties for OpenID 1.1 and 2.0
Other
4 stars 0 forks source link

NullPointerException when obtaining authURL #10

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Try logging into openID provider http://openid.aol.com/<screenname>)

What is the expected output? What do you see instead?
Attempt the OpenID authentication process.  Instead, I get an NPE.

What version of the product are you using? On what operating system?
Recent version.

Please provide any additional information below.

Suggest the following code snippet to check for Null conditions.

        //String handle = p.getProperty("handle");
        if(p == null) {
            throw new OpenIdException("Could not get OpenId server 
properties from " +
                    "identifier.");
        }
        if(p.containsKey("handle")){ 
                // do remainder of method
        else {
            throw new OpenIdException ("Could not get OpenId server handle 
from " +
                    "identifier.");
        }

Original issue reported on code.google.com by tod...@us.ibm.com on 5 Nov 2007 at 8:18

GoogleCodeExporter commented 9 years ago
Here's my trace:

The exception comes in JoidConsumer.java:164, where p is the only possible null 
variable.

That means the previous line p = getProps(...) is where p is null.

In getProps() there's code that catches exceptions and prints a stack trace, but
blithely continues to return null.

By looking in my server logs I found that other stack trace.  It's for
MessageParser.java:79

From that it looks like MapEntry.getKey() must have returned null, and at that 
point
I'm giving up for now.

Original comment by ourdoi...@gmail.com on 29 Mar 2008 at 3:05

GoogleCodeExporter commented 9 years ago
Somebody may have a patch:

http://www.projectzero.org/forum/viewtopic.php?f=4&t=849&p=3595

Original comment by ourdoi...@gmail.com on 29 Mar 2008 at 3:32

GoogleCodeExporter commented 9 years ago
I am both the originator of this bug and the person mentioned in this forum 
posting
in comment #2.  I have not been able to figure out why this failure is 
occurring and
instead have added more null checks for this issue and some debug trace for the
situation when this problem occurs.  From what we can see, the problem is
intermittent in 
"http://www.projectzero.org/forum/viewtopic.php?f=4&t=849&p=3595" and
did not even think to consider these two issues as related but they might be.

Original comment by tod...@us.ibm.com on 29 Mar 2008 at 11:51

GoogleCodeExporter commented 9 years ago
BTW. Here is what was done for
http://www.projectzero.org/forum/viewtopic.php?f=4&t=849&p=3595:

if( key!= null && value != null){
    sb.append(URLEncoder.encode(key, "UTF-8")+kvDelim
              +URLEncoder.encode(value, "UTF-8"));
    if (iter.hasNext()) {
        sb.append(lineDelim);
    }
}else{
    if(log.isLoggable(Level.FINE)){
    log.logp(Level.FINE, CLASS_NAME, "toStringDelimitedBy", "null key or value:
message->" + message);
    log.logp(Level.FINE, CLASS_NAME, "toStringDelimitedBy", "null key or value: map->" +
map);
    log.logp(Level.FINE, CLASS_NAME, "toStringDelimitedBy", "key["+ key +"] value["
+value +"]");
    }
    if(key !=null){
        sb.append(URLEncoder.encode(key, "UTF-8")+kvDelim
              +URLEncoder.encode("", "UTF-8"));
    if (iter.hasNext()) {
            sb.append(lineDelim);
    }
    }else if ( value != null){
        sb.append(URLEncoder.encode("", "UTF-8")+kvDelim
              +URLEncoder.encode(value, "UTF-8"));
        if (iter.hasNext()) {
            sb.append(lineDelim);
        }
        }
}

Original comment by tod...@us.ibm.com on 29 Mar 2008 at 11:55

GoogleCodeExporter commented 9 years ago
If you look in Util.java, there seems to be a link to a fix that people have had
success with.  Change Util.send to use POST instead of GET.  Unfortunately I 
haven't
made the code linked to work yet.  I did remove the spurious asterisks and 
change
build.xml to work with 1.6 so generics would be supported, but I still got 
errors
that I'll have to look at later.  Or I may just write my own code to use POST 
the
old-fashioned way, e.g.

http://developers.sun.com/mobility/midp/ttips/HTTPPost/

Original comment by ourdoi...@gmail.com on 29 Mar 2008 at 3:53

GoogleCodeExporter commented 9 years ago
I was seeing the problem consistently before I made the attached fix to 
Utils.java. 
If it doesn't work for you, let me know and I'll also attach my changes for NPE
handling.  I don't think those are important, though.

Original comment by ourdoi...@gmail.com on 29 Mar 2008 at 6:21

Attachments:

GoogleCodeExporter commented 9 years ago
So changing the parameters from being part of the query string to being part of 
the
post body resolved some of these issues?  For application servers using servlet 
based
containers, there is no distinction between the query string param and the post 
param
since both are accessible via the request.getParameter.  Other application 
servers
(such as PHP) can look at a generic parameter or look at just the query string 
or
just the post data. However, from my understanding when these failures were 
occurring
in our env, it was not consistently failing so I do not think changing to POST 
from a
GET will make much difference unless there is a caching issue of some sort.

Note I am wondering if your proposed solution just gets around the issue that I
raised in bug #8 (http://code.google.com/p/joid/issues/detail?id=8) where the 
URL
already had a query string and the code tried to append a second ? and query 
string
data.  This caused NPEs for me before.  

As a general comment. In reviewing part of the spec, it does seem like POST is
required for both associate and authenticate so maybe this change is requires
regardless.  Do you agree with this finding?

http://openid.net/specs/openid-authentication-2_0.html

5.1.  Direct Communication

Direct communication is initiated by a Relying Party to an OP endpoint URL. It 
is
used for establishing associations (Establishing Associations) and verifying
authentication assertions (Verifying Directly with the OpenID Provider).

5.1.1.  Direct Request

The message MUST be encoded as a POST body, as specified by Section 4.1.2 (HTTP
Encoding).

All direct requests are HTTP POSTs, and so contain the required fields listed in
Section 4.1.2 (HTTP Encoding). 

Original comment by tod...@us.ibm.com on 29 Mar 2008 at 7:59

GoogleCodeExporter commented 9 years ago
The heart of the problem is

org.verisign.joid.OpenIdException: Cannot parse response from error:Unknown mode

LiveJournal stopped sending the error after I made the change.  I was getting 
the
error consistently before the change.

Error handling should still be better, but now I think it's a lot less likely 
to get
an error.

Original comment by ourdoi...@gmail.com on 30 Mar 2008 at 2:33

GoogleCodeExporter commented 9 years ago
I reproduced the LiveJournal and made the same change you suggested with some
success.  I would like to verify a few other providers I leverage but this looks
promising especially based upon the quote taken from the spec in comment #7.

Original comment by tod...@us.ibm.com on 30 Mar 2008 at 3:26

GoogleCodeExporter commented 9 years ago
validated fix still allows openid.net and openid.aol.com openid provider sites 
to work.

Original comment by tod...@us.ibm.com on 30 Mar 2008 at 3:29

GoogleCodeExporter commented 9 years ago
issue seems dead. cannot reproduce the exception with most recent version.

Original comment by birkan.d...@gmail.com on 21 Jul 2011 at 10:43