Skip to content

Commit d4d5236

Browse files
duncdrumduncdrum
committed
[bugfix] fix persistent-login cookies on standalone Jetty "/" context
On root context deployments getContextPath() returns "", which produced Set-Cookie Path="" and caused clients to ignore org.exist.login. Map empty context paths to "/" in login.xql and omit blank paths in HttpResponseWrapper. Use "|" as the token separator so strict Set-Cookie parsers accept the value. LoginModuleIT failed at step 3 (admin→guest) because HttpClient 4.x DEFAULT (NetscapeDraftSpec) rejects Jetty 12 RFC6265 Expires dates. Use CookieSpecs.STANDARD with a shared cookie store instead of manually parsing Set-Cookie headers; apply the same spec in AbstractHttpTest for future ITs. Refs jetty/jetty.project#12771
1 parent d942298 commit d4d5236

6 files changed

Lines changed: 157 additions & 15 deletions

File tree

‎exist-core/src/main/java/org/exist/http/servlets/HttpResponseWrapper.java‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,25 @@ public void addCookie(final String name, final String value, final int maxAge, b
7878
final Cookie cookie = new Cookie(name, encode(value));
7979
cookie.setMaxAge(maxAge);
8080
cookie.setSecure( secure );
81-
if (domain != null) {
81+
if (domain != null && !domain.isEmpty()) {
8282
cookie.setDomain(domain);
8383
}
84-
if (path != null) {
84+
setCookiePath(cookie, path);
85+
response.addCookie(cookie);
86+
}
87+
88+
/**
89+
* Apply a cookie path only when it is a non-empty string.
90+
* <p>
91+
* Standalone Jetty deployments use a root context ({@code getContextPath()} returns {@code ""}).
92+
* Passing that empty string as an explicit Path makes many HTTP clients (including Apache HttpClient
93+
* used in integration tests) reject the cookie entirely. Omitting Path lets the container apply
94+
* the RFC 6265 default for the request URI.
95+
*/
96+
private static void setCookiePath(final Cookie cookie, final String path) {
97+
if (path != null && !path.isEmpty()) {
8598
cookie.setPath(path);
8699
}
87-
response.addCookie(cookie);
88100
}
89101

90102
@Override

‎exist-core/src/test/java/org/exist/http/AbstractHttpTest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import com.evolvedbinary.j8fu.function.FunctionE;
2626
import org.apache.http.HttpHost;
2727
import org.apache.http.client.HttpClient;
28+
import org.apache.http.client.config.CookieSpecs;
29+
import org.apache.http.client.config.RequestConfig;
2830
import org.apache.http.client.fluent.Executor;
2931
import org.apache.http.impl.client.CloseableHttpClient;
3032
import org.apache.http.impl.client.HttpClientBuilder;
@@ -85,6 +87,9 @@ protected static <T> T withHttpClient(final FunctionE<HttpClient, T, IOException
8587
try (final CloseableHttpClient client = HttpClientBuilder
8688
.create()
8789
.disableAutomaticRetries()
90+
.setDefaultRequestConfig(RequestConfig.custom()
91+
.setCookieSpec(CookieSpecs.STANDARD)
92+
.build())
8893
.build()) {
8994
return fn.apply(client);
9095
}

‎extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLogin.java‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,17 @@ public static PersistentLogin getInstance() {
6262

6363
public final static int INVALIDATION_TIMEOUT = 20000;
6464

65+
/**
66+
* Separator between series and token in newly issued cookie values.
67+
* Must not appear in base64 output ({@code A-Za-z0-9+/=}).
68+
* {@code :} was used historically but breaks Apache HttpClient's Set-Cookie parser
69+
* (and some other clients) because unquoted cookie values containing {@code :} are rejected.
70+
*/
71+
private static final String TOKEN_SEPARATOR = "|";
72+
73+
/** Historical separator; still accepted when parsing incoming cookies. */
74+
private static final String LEGACY_TOKEN_SEPARATOR = ":";
75+
6576
private Map<String, LoginDetails> seriesMap = Collections.synchronizedMap(new HashMap<>());
6677

6778
private SecureRandom random;
@@ -100,7 +111,11 @@ public LoginDetails register(String user, String password, DurationValue timeToL
100111
* or an out-of-sequence request.
101112
*/
102113
public LoginDetails lookup(String token) throws XPathException {
103-
String[] tokens = token.split(":");
114+
final String[] tokens = splitTokenValue(token);
115+
if (tokens.length < 2) {
116+
LOG.debug("Malformed persistent login token");
117+
return null;
118+
}
104119

105120
LoginDetails data = seriesMap.get(tokens[0]);
106121
if (data == null) {
@@ -135,8 +150,22 @@ public LoginDetails lookup(String token) throws XPathException {
135150
* @param token token string provided by the user
136151
*/
137152
public void invalidate(String token) {
138-
String[] tokens = token.split(":");
139-
seriesMap.remove(tokens[0]);
153+
final String[] tokens = splitTokenValue(token);
154+
if (tokens.length > 0) {
155+
seriesMap.remove(tokens[0]);
156+
}
157+
}
158+
159+
/**
160+
* Split a cookie value into series and token. New cookies use {@link #TOKEN_SEPARATOR};
161+
* {@link #LEGACY_TOKEN_SEPARATOR} is still accepted for in-flight sessions created before the switch.
162+
*/
163+
private static String[] splitTokenValue(final String token) {
164+
final String[] pipeParts = token.split("\\" + TOKEN_SEPARATOR, 2);
165+
if (pipeParts.length == 2) {
166+
return pipeParts;
167+
}
168+
return token.split(LEGACY_TOKEN_SEPARATOR, 2);
140169
}
141170

142171
private String generateSeriesToken() {
@@ -229,7 +258,7 @@ private void timeoutCheck() {
229258

230259
@Override
231260
public String toString() {
232-
return this.series + ":" + this.token;
261+
return this.series + TOKEN_SEPARATOR + this.token;
233262
}
234263
}
235264
}

‎extensions/modules/persistentlogin/src/main/resources/org/exist/xquery/modules/persistentlogin/login.xql‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,18 @@ declare function login:set-user($domain as xs:string, $maxAge as xs:dayTimeDurat
9898
login:set-user($domain, (), $maxAge, $asDba)
9999
};
100100

101+
declare %private function login:cookie-path($path as xs:string?) as xs:string {
102+
if (exists($path) and $path != "") then
103+
$path
104+
else
105+
let $ctx := request:get-context-path()
106+
return
107+
if ($ctx = "") then
108+
"/"
109+
else
110+
$ctx
111+
};
112+
101113
declare %private function login:callback($newToken as xs:string?, $user as xs:string, $password as xs:string,
102114
$expiration as xs:duration, $domain as xs:string, $path as xs:string?, $asDba as xs:boolean) {
103115
if (not($asDba) or sm:is-dba($user)) then (
@@ -106,7 +118,7 @@ declare %private function login:callback($newToken as xs:string?, $user as xs:st
106118
request:set-attribute("xquery.password", $password),
107119
if ($newToken) then
108120
response:set-cookie($domain, $newToken, $expiration, false(), (),
109-
if (exists($path)) then $path else request:get-context-path())
121+
login:cookie-path($path))
110122
else
111123
()
112124
) else
@@ -128,7 +140,7 @@ declare %private function login:create-login-session($domain as xs:string, $path
128140

129141
declare %private function login:clear-credentials($token as xs:string?, $domain as xs:string, $path as xs:string?) as empty-sequence() {
130142
response:set-cookie($domain, "deleted", xs:dayTimeDuration("-P1D"), false(), (),
131-
if (exists($path)) then $path else request:get-context-path()),
143+
login:cookie-path($path)),
132144
if ($token and $token != "deleted") then
133145
plogin:invalidate($token)
134146
else

‎extensions/modules/persistentlogin/src/test/java/org/exist/xquery/modules/persistentlogin/LoginModuleIT.java‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,12 @@
2323

2424
import org.apache.http.HttpEntity;
2525
import org.apache.http.HttpResponse;
26-
import org.apache.http.client.HttpClient;
26+
import org.apache.http.client.config.CookieSpecs;
27+
import org.apache.http.client.config.RequestConfig;
2728
import org.apache.http.client.methods.HttpGet;
29+
import org.apache.http.client.protocol.HttpClientContext;
2830
import org.apache.http.impl.client.BasicCookieStore;
31+
import org.apache.http.impl.client.CloseableHttpClient;
2932
import org.apache.http.impl.client.HttpClientBuilder;
3033
import org.apache.http.util.EntityUtils;
3134
import org.exist.TestUtils;
@@ -65,7 +68,9 @@ public class LoginModuleIT {
6568
private final static String XQUERY_FILENAME = "test-login.xql";
6669

6770
private static Collection root;
68-
private static HttpClient client;
71+
private static CloseableHttpClient client;
72+
private static BasicCookieStore cookieStore;
73+
private static HttpClientContext httpContext;
6974

7075
@BeforeClass
7176
public static void beforeClass() throws XMLDBException {
@@ -100,12 +105,23 @@ public static void beforeClass() throws XMLDBException {
100105
final UserManagementService ums = root.getService(UserManagementService.class);
101106
ums.chmod(res, 0777);
102107

103-
final BasicCookieStore store = new BasicCookieStore();
104-
client = HttpClientBuilder.create().setDefaultCookieStore(store).build();
108+
cookieStore = new BasicCookieStore();
109+
httpContext = HttpClientContext.create();
110+
httpContext.setCookieStore(cookieStore);
111+
// Jetty 12 emits RFC 6265 Set-Cookie (RFC1123 Expires). HttpClient 4.x DEFAULT (NetscapeDraftSpec)
112+
// rejects that format; STANDARD is required for automatic cookie storage. See jetty/jetty.project#12771.
113+
client = HttpClientBuilder.create()
114+
.setDefaultRequestConfig(RequestConfig.custom()
115+
.setCookieSpec(CookieSpecs.STANDARD)
116+
.build())
117+
.build();
105118
}
106119

107120
@AfterClass
108-
public static void afterClass() throws XMLDBException {
121+
public static void afterClass() throws Exception {
122+
if (client != null) {
123+
client.close();
124+
}
109125
if (root != null) {
110126
final org.xmldb.api.base.Resource res = root.getResource(XQUERY_FILENAME);
111127
if (res != null) {
@@ -132,10 +148,11 @@ public void loginAndLogout() throws IOException {
132148
private void doGet(@Nullable String params, String expected) throws IOException {
133149
final HttpGet httpGet = new HttpGet("http://localhost:" + existWebServer.getPort() + "/rest" + XmldbURI.ROOT_COLLECTION + '/' + XQUERY_FILENAME +
134150
(params == null ? "" : "?" + params));
135-
HttpResponse response = client.execute(httpGet);
151+
HttpResponse response = client.execute(httpGet, httpContext);
136152
HttpEntity entity = response.getEntity();
137153
final String responseBody = EntityUtils.toString(entity);
138154
assertEquals(responseBody, SC_OK, response.getStatusLine().getStatusCode());
139155
assertEquals(expected, responseBody);
140156
}
157+
141158
}

‎extensions/modules/persistentlogin/src/test/java/org/exist/xquery/modules/persistentlogin/PersistentLoginTest.java‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* eXist-db Open Source Native XML Database
3+
* Copyright (C) 2001 The eXist-db Authors
4+
*
5+
* info@exist-db.org
6+
* http://www.exist-db.org
7+
*
8+
* This library is free software; you can redistribute it and/or
9+
* modify it under the terms of the GNU Lesser General Public
10+
* License as published by the Free Software Foundation; either
11+
* version 2.1 of the License, or (at your option) any later version.
12+
*
13+
* This library is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
15+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
16+
* Lesser General Public License for more details.
17+
*
18+
* You should have received a copy of the GNU Lesser General Public
19+
* License along with this library; if not, write to the Free Software
20+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
21+
*/
22+
package org.exist.xquery.modules.persistentlogin;
23+
24+
import org.exist.xquery.XPathException;
25+
import org.exist.xquery.value.DayTimeDurationValue;
26+
import org.exist.xquery.value.DurationValue;
27+
import org.junit.BeforeClass;
28+
import org.junit.Test;
29+
30+
import static org.junit.Assert.assertNotNull;
31+
import static org.junit.Assert.assertNull;
32+
import static org.junit.Assert.assertTrue;
33+
34+
public class PersistentLoginTest {
35+
36+
private static DurationValue oneDay;
37+
38+
@BeforeClass
39+
public static void initDuration() throws XPathException {
40+
oneDay = new DayTimeDurationValue("P1D");
41+
}
42+
43+
@Test
44+
public void newTokensUsePipeSeparator() throws XPathException {
45+
final PersistentLogin login = new PersistentLogin();
46+
final PersistentLogin.LoginDetails details = login.register("admin", "admin", oneDay);
47+
assertTrue(details.toString().contains("|"));
48+
assertNotNull(login.lookup(details.toString()));
49+
}
50+
51+
@Test
52+
public void lookupAcceptsLegacyColonSeparator() throws XPathException {
53+
final PersistentLogin login = new PersistentLogin();
54+
final PersistentLogin.LoginDetails details = login.register("admin", "admin", oneDay);
55+
final String legacyToken = details.getSeries() + ":" + details.getToken();
56+
assertNotNull(login.lookup(legacyToken));
57+
}
58+
59+
@Test
60+
public void invalidateAcceptsLegacyColonSeparator() throws XPathException {
61+
final PersistentLogin login = new PersistentLogin();
62+
final PersistentLogin.LoginDetails details = login.register("admin", "admin", oneDay);
63+
final String legacyToken = details.getSeries() + ":" + details.getToken();
64+
login.invalidate(legacyToken);
65+
assertNull(login.lookup(legacyToken));
66+
}
67+
}

0 commit comments

Comments
 (0)