Skip to content

Commit

Permalink
Use the CookieManager to populate response cookies
Browse files Browse the repository at this point in the history
Vs manually parsing them. This corrects handling for the various properties (order, path, TLS only, etc) that were not handled in the simple cookie map previously.

Fixes #1831.
  • Loading branch information
jhy committed Sep 11, 2024
1 parent 0daab3d commit b6bd4b4
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 48 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
children. [2187](https://github.com/jhy/jsoup/issues/2187)
* A selector query that included multiple `:has()` components in a nested `:has()` might incorrectly
execute. [2131](https://github.com/jhy/jsoup/issues/2131)
* Updated the simple view of cookies available via `Connection.Response#cookies()` to reflect the contents of the
current cookie jar for the current URL. [1831](https://github.com/jhy/jsoup/issues/1831)

## 1.18.1 (2024-Jul-10)

Expand Down
5 changes: 1 addition & 4 deletions src/main/java/org/jsoup/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -604,10 +604,6 @@ interface Base<T extends Base<T>> {

/**
* Get a cookie value by name from this request/response.
* <p>
* Response objects have a simplified cookie model. Each cookie set in the response is added to the response
* object's cookie key=value map. The cookie's path, domain, and expiry date are ignored.
* </p>
* @param name name of cookie to retrieve.
* @return value of cookie, or null if not set
*/
Expand Down Expand Up @@ -638,6 +634,7 @@ interface Base<T extends Base<T>> {
/**
* Retrieve all of the request/response cookies as a map
* @return cookies
* @see #cookieStore()
*/
Map<String, String> cookies();
}
Expand Down
19 changes: 17 additions & 2 deletions src/main/java/org/jsoup/helper/CookieUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import org.jsoup.internal.StringUtil;

import java.io.IOException;
import java.net.CookieManager;
import java.net.HttpCookie;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URI;
Expand Down Expand Up @@ -83,8 +85,21 @@ static URI asUri(URL url) throws IOException {
}
}

static void storeCookies(HttpConnection.Request req, URL url, Map<String, List<String>> resHeaders) throws IOException {
req.cookieManager().put(CookieUtil.asUri(url), resHeaders); // stores cookies for session
/** Store the Result cookies into the cookie manager, and place relevant cookies into the Response object. */
static void storeCookies(HttpConnection.Request req, HttpConnection.Response res, URL url, Map<String, List<String>> resHeaders) throws IOException {
CookieManager manager = req.cookieManager();
URI uri = CookieUtil.asUri(url);
manager.put(uri, resHeaders); // stores cookies for session

// set up the simple cookie(name, value) map:
Map<String, List<String>> cookieMap = manager.get(uri, resHeaders); // get cookies for url; may have been set on this or earlier requests. the headers here are ignored other than a null check
for (List<String> values : cookieMap.values()) {
for (String headerVal : values) {
List<HttpCookie> cookies = HttpCookie.parse(headerVal);
for (HttpCookie cookie : cookies) {
res.cookie(cookie.getName(), cookie.getValue());
}
}
}
}
}
20 changes: 1 addition & 19 deletions src/main/java/org/jsoup/helper/HttpConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -1133,14 +1133,9 @@ private Response(HttpURLConnection conn, HttpConnection.Request request, HttpCon

Map<String, List<String>> resHeaders = createHeaderMap(conn);
processResponseHeaders(resHeaders); // includes cookie key/val read during header scan
CookieUtil.storeCookies(req, url, resHeaders); // add set cookies to cookie store
CookieUtil.storeCookies(req, this, url, resHeaders); // add set cookies to cookie store

if (previousResponse != null) { // was redirected
// map previous response cookies into this response cookies() object
for (Map.Entry<String, String> prevCookie : previousResponse.cookies().entrySet()) {
if (!hasCookie(prevCookie.getKey()))
cookie(prevCookie.getKey(), prevCookie.getValue());
}
previousResponse.safeClose();

// enforce too many redirects:
Expand Down Expand Up @@ -1176,19 +1171,6 @@ void processResponseHeaders(Map<String, List<String>> resHeaders) {
continue; // http/1.1 line

List<String> values = entry.getValue();
if (name.equalsIgnoreCase("Set-Cookie")) {
for (String value : values) {
if (value == null)
continue;
TokenQueue cd = new TokenQueue(value);
String cookieName = cd.chompTo("=").trim();
String cookieVal = cd.consumeTo(";").trim();
// ignores path, date, domain, validateTLSCertificates et al. full details will be available in cookiestore if required
// name not blank, value not null
if (cookieName.length() > 0 && !cookies.containsKey(cookieName)) // if duplicates, only keep the first
cookie(cookieName, cookieVal);
}
}
for (String value : values) {
addHeader(name, fixHeaderEncoding(value));
}
Expand Down
21 changes: 0 additions & 21 deletions src/test/java/org/jsoup/helper/HttpConnectionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -155,27 +155,6 @@ public void caseInsensitiveHeaders(Locale locale) {
assertEquals(0, res.cookies().size());
}

@Test public void ignoresEmptyCookieNameAndVals() {
// prep http response header map
Map<String, List<String>> headers = new HashMap<>();
List<String> cookieStrings = new ArrayList<>();
cookieStrings.add(null);
cookieStrings.add("");
cookieStrings.add("one");
cookieStrings.add("two=");
cookieStrings.add("three=;");
cookieStrings.add("four=data; Domain=.example.com; Path=/");

headers.put("Set-Cookie", cookieStrings);
HttpConnection.Response res = new HttpConnection.Response();
res.processResponseHeaders(headers);
assertEquals(4, res.cookies().size());
assertEquals("", res.cookie("one"));
assertEquals("", res.cookie("two"));
assertEquals("", res.cookie("three"));
assertEquals("data", res.cookie("four"));
}

@Test public void connectWithUrl() throws MalformedURLException {
Connection con = HttpConnection.connect(new URL("http://example.com"));
assertEquals("http://example.com", con.request().url().toExternalForm());
Expand Down
9 changes: 8 additions & 1 deletion src/test/java/org/jsoup/integration/ConnectTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.jsoup.parser.StreamParser;
import org.jsoup.parser.XmlTreeBuilder;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
Expand Down Expand Up @@ -60,6 +61,12 @@ public static void setUp() {
echoUrl = EchoServlet.Url;
}

@BeforeEach
public void emptyCookieJar() {
// empty the cookie jar, so cookie tests are independent.
Jsoup.connect("http://example.com").cookieStore().removeAll();
}

@Test
public void canConnectToLocalServer() throws IOException {
String url = HelloServlet.Url;
Expand Down Expand Up @@ -427,7 +434,7 @@ public void multiCookieSet() throws IOException {
// test cookies set by redirect:
Map<String, String> cookies = res.cookies();
assertEquals("asdfg123", cookies.get("token"));
assertEquals("jhy", cookies.get("uid"));
assertEquals("jhy", cookies.get("uid")); // two uids set, order dependent

// send those cookies into the echo URL by map:
Document doc = Jsoup.connect(echoUrl).cookies(cookies).get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ protected void doIt(HttpServletRequest req, HttpServletResponse res) throws IOEx

if (req.getParameter(SetCookiesParam) != null) {
res.addCookie(new Cookie("token", "asdfg123"));
res.addCookie(new Cookie("uid", "jhy"));
res.addCookie(new Cookie("uid", "foobar"));
res.addCookie(new Cookie("uid", "jhy")); // dupe, should use latter
}

res.setHeader("Location", location);
Expand Down

0 comments on commit b6bd4b4

Please sign in to comment.