MariusM95 / android-rcs-ims-stack

Automatically exported from code.google.com/p/android-rcs-ims-stack
0 stars 0 forks source link

Problems with sendRequestToXDMS() function in XdmManager.java #139

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.When Presence service is starting we have the following call stack:

start()
▼
xdm.initialize()
▼
getXcapDocuments()
▼
sendRequestToXDMS(HttpRequest request)
▼
sendRequestToXDMS(HttpRequest request, HttpAuthenticationAgent 
authenticationAgent)
▼
sendHttpRequest()
▼
generateAuthorizationHeader()
▼
generateAuthorizationHeaderValue()
▼
public void updateNonceParameters() {
    // Update nonce and nc
    if (nextnonce.equals(nonce)) {
        // Next nonce == nonce
        nc++;
    } else {
        // Next nonce != nonce
        nc = 1;
        nonce = nextnonce;
    }       
}

2. BOOM! We got NullPointerException, because nextnonce is null.

3.Why? Because in function sendRequestToXDMS(HttpRequest request) we are 
creating our authenticationAgent as follows:
new HttpAuthenticationAgent(xdmServerLogin, xdmServerPwd)

4. we proceed to use it without any doubts in functions
sendRequestToXDMS(HttpRequest request, HttpAuthenticationAgent 
authenticationAgent) and
sendHttpRequest()
which is wrong because in the function sendHttpRequest()
we have a check 
if (authenticationAgent != null) {
    // Set the Authorization header
    String authorizationHeader = authenticationAgent.generateAuthorizationHeader(
            request.getMethod(), requestUri, request.getContent());
    httpRequest += authorizationHeader + HttpUtils.CRLF;
}

and authenticationAgent passes it without problems, even if its 
HttpDigestMd5Authentication member was not properly initialized before.

What is the expected output? What do you see instead?

What version of the product are you using? On what operating system?
I am using version 2.5.7

Please provide any additional information below.
I propose the following solution to the problem:
1. Don't call function sendRequestToXDMS(HttpRequest request) at all
2. All occurrences when this function is used replace with the following 
statement:
sendRequestToXDMS(request, null);
3. modify the function
private HttpResponse sendRequestToXDMS(HttpRequest request, 
HttpAuthenticationAgent authenticationAgent) throws CoreException

as follows:
private HttpResponse sendRequestToXDMS(HttpRequest request, 
HttpAuthenticationAgent authenticationAgent) throws CoreException {
    try {
        // Send first request
        HttpResponse response = sendHttpRequest(request, authenticationAgent);

        // Analyze the response
        if (response.getResponseCode() == 401) {
            // 401 response received
            if (logger.isActivated()) {
                logger.debug("401 Unauthorized response received");
            }

            authenticationAgent = new HttpAuthenticationAgent(xdmServerLogin, xdmServerPwd);

            if (authenticationAgent != null) {
                // Update the authentication agent
                authenticationAgent.readWwwAuthenticateHeader(response.getHeader("www-authenticate"));
            }

            // Set the cookie from the received response
            String cookie = response.getHeader("set-cookie");
            request.setCookie(cookie);

            // Send second request with authentification header
            response = sendRequestToXDMS(request, authenticationAgent);
        } else
        if (response.getResponseCode() == 412) {
            // 412 response received
            if (logger.isActivated()) {
                logger.debug("412 Precondition failed");
            }

            authenticationAgent = new HttpAuthenticationAgent(xdmServerLogin, xdmServerPwd);

            if (authenticationAgent != null) {
                // Update the authentication agent
                authenticationAgent.readWwwAuthenticateHeader(response.getHeader("www-authenticate"));
            }

            // Reset the etag
            documents.remove(request.getAUID());

            // Send second request with authentification header
            response = sendRequestToXDMS(request, authenticationAgent);
        } else {        
            // Other response received
            if (logger.isActivated()) {
                logger.debug(response.getResponseCode() + " response received");
            }
        }
        return response;
    } catch(CoreException e) {
        throw e;
    } catch(Exception e) {
        throw new CoreException("Can't send HTTP request: " + e.getMessage());
    }
}

4. See, during the first try we would receive message "4401 Unauthorized 
response received" from our SIP server.
We will use this responce to create new authenticationAgent and obtain nonce 
information from the responce.
5. after that we call sendRequestToXDMS(..) function again, this time 
successfully.

Original issue reported on code.google.com by vin...@broadcom.com on 16 Aug 2013 at 3:52

GoogleCodeExporter commented 9 years ago
We don't test this part of the stack. Presence is optional and not deployed by 
Telco. We have no more presence server to test, so we are very interested if 
you can realize non regresion on this part and provide bug fixes in case of 
problems.

Original comment by jmauffret@gmail.com on 16 Aug 2013 at 8:39

GoogleCodeExporter commented 9 years ago
What presence server did you used before? What happened to it?

I am not sure whether I can dedicate my time specifically to fixing this bug, 
however I did my best to outline the problem and identifying proposed fix, 
therefore implementing this change would not be a problem.

I have one more question: on the roadmap I saw that you are planning to 
implement RCS 5.1 features as well (and Presence is one of them), then how are 
you going to test them?

Original comment by vin...@broadcom.com on 16 Aug 2013 at 9:18

GoogleCodeExporter commented 9 years ago
The presence server was a product.
Presence services will be not deployed by Telcos in Europe, the same for 
Albatros and Blackbird release.

Original comment by jmauffret@gmail.com on 18 Aug 2013 at 8:41