5
public boolean isGood(String path)
{
    if (p != path)
    {
        good = false;
    }

    if (good)
    {
        try 
        {
            Connection connection = Jsoup.connect(path);
            Map<String, String> cookys = Jsoup.connect(path).response().cookies();

            if (cookys != cookies)
                cookies = cookys;

            for (Entry<String, String> cookie : cookies.entrySet()) 
            {
                connection.cookie(cookie.getKey(), cookie.getValue());
            }

            Doc = connection.get();
            good = true;
        }
        catch (Exception e) 
        {
            rstring = e.getMessage().toString();
            good = false;
        }
    }
    else
    {
        try
        {
            Response response = Jsoup.connect(path).execute();
            cookies = response.cookies();
            Doc = response.parse();
            good = true;
        }
        catch (Exception e) 
        {
            rstring = e.getMessage().toString();
            good = false;
        } 
    }       
    return good;
}

This method is not right. What I'm trying to figure out is a way to without know what cookies will exist, be able to handle cookie changes as well as maintain sessions.

I'm writing an app for my simple machines forum, and it changes its cookie config as you click around for some custom behavior.

But if the app does well for my site, I was going to publish a version for others to use for other forums.

I know I'm heading in the right direction, but the logic is kind of kicking my butt.

Any advice would be greatly appreciated.

BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
texasman1979
  • 473
  • 1
  • 6
  • 17
  • In addition to BalusC's comments, it's unlikely that `p != path` thing is what you really mean, although it's *possible*. – Dave Newton Oct 11 '11 at 16:12

2 Answers2

15

This code is very confusing. The flow is illogical and the exception handling is bad. The object reference comparisons like if (p != path) and if (cookys != cookies) makes no utter sense. To compare object's contents you need to use equals() method instead.

To the point, I understand that you want to maintain cookies in a bunch of subsequent Jsoup requests on the same domain. In that case, you need to basically adhere the following flow:

Map<String, String> cookies = new HashMap<String, String>();

// First request.
Connection connection1 = Jsoup.connect(url1);
for (Entry<String, String> cookie : cookies.entrySet()) {
    connection1.cookie(cookie.getKey(), cookie.getValue());
}
Response response1 = connection1.execute();
cookies.putAll(response1.cookies());
Document document1 = response1.parse();
// ...

// Second request.
Connection connection2 = Jsoup.connect(url2);
for (Entry<String, String> cookie : cookies.entrySet()) {
    connection2.cookie(cookie.getKey(), cookie.getValue());
}
Response response2 = connection2.execute();
cookies.putAll(response2.cookies());
Document document2 = response2.parse();
// ...

// Third request.
Connection connection3 = Jsoup.connect(url3);
for (Entry<String, String> cookie : cookies.entrySet()) {
    connection3.cookie(cookie.getKey(), cookie.getValue());
}
Response response3 = connection3.execute();
cookies.putAll(response3.cookies());
Document document3 = response3.parse();
// ...

// Etc.

This can be refactored to the following method:

private Map<String, String> cookies = new HashMap<String, String>();

public Document get(url) throws IOException {
    Connection connection = Jsoup.connect(url);
    for (Entry<String, String> cookie : cookies.entrySet()) {
        connection.cookie(cookie.getKey(), cookie.getValue());
    }
    Response response = connection.execute();
    cookies.putAll(response.cookies());
    return response.parse();
}

which can be used as

YourJsoupWrapper jsoupWrapper = new YourJsoupWrapper();

Document document1 = jsoupWrapper.get(url1);
// ...

Document document2 = jsoupWrapper.get(url2);
// ...

Document document3 = jsoupWrapper.get(url3);
// ...

Note that the upcoming Jsoup 1.6.2 will come with a new Connection#cookies(Map) method which should make that for loop everytime superfluous.

BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • i thank you very much. i searched and searched for an example of the right way to do it. this way is definitely better than the way i was attempting. lol – texasman1979 Oct 11 '11 at 16:41
  • Someone from the future! for some reason I found JSoup to be very convenient. Just my two cents are that in the current JSoup library there's a method named cookies (Map cookies) that does the addition of the key/value pairs. So the above foreach can be replaced for: connection.cookies (cookies) – Jose Cifuentes Mar 21 '14 at 03:29
  • you can do connection.cookies(cookies) instead of the for loop – caub Mar 19 '15 at 21:16
  • @crl: Indeed, see also last paragraph of above answer and the comments of Jsoup developer on [this answer](http://stackoverflow.com/a/7206268/157882). – BalusC Mar 19 '15 at 21:18
1

+1 for BalusC

I changed some in your code and it works for me, so you get cookie from site and only than get Document

public Document get(String url) throws IOException {
    Connection connection = Jsoup.connect(url).userAgent("Mozilla/5.0 (Windows; U; WindowsNT 5.1; en-US; rv1.8.1.6) Gecko/20070725 Firefox/2.0.0.6");
    Connection.Response response = connection.execute();
    connection.cookies(response.cookies());
    return connection.get();
}
Bender
  • 327
  • 1
  • 7
  • 13