Skip to content

Commit 2919643

Browse files
jknackjknack
committed
Set default cookie path to application.path
* fix jooby-project#466 * fix jooby-project#471
1 parent 78819fe commit 2919643

File tree

11 files changed

+188
-61
lines changed

11 files changed

+188
-61
lines changed

‎coverage-report/src/test/java/org/jooby/cookies/CookiesFeature.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public void clearCookie() throws Exception {
8686
.header("Cookie", "X=x; $Path=/clear; $Version=1")
8787
.expect(200)
8888
.header("Set-Cookie",
89-
"X=;Version=1;Max-Age=0;Expires=Thu, 01-Jan-1970 00:00:00 GMT");
89+
"X=;Version=1;Path=/;Max-Age=0;Expires=Thu, 01-Jan-1970 00:00:00 GMT");
9090
});
9191

9292
}

‎coverage-report/src/test/java/org/jooby/issues/Issue427.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public void sessionData() throws Exception {
8181
.get("/427/destroy")
8282
.expect(200)
8383
.header("Set-Cookie",
84-
"jooby.sid=;Version=1;Max-Age=0;Expires=Thu, 01-Jan-1970 00:00:00 GMT");
84+
"jooby.sid=;Version=1;Path=/;HttpOnly;Max-Age=0;Expires=Thu, 01-Jan-1970 00:00:00 GMT");
8585
}
8686

8787
}

‎coverage-report/src/test/java/org/jooby/issues/Issue466.java‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package org.jooby.issues;
2+
3+
import org.jooby.Results;
4+
import org.jooby.Session;
5+
import org.jooby.test.ServerFeature;
6+
import org.junit.Test;
7+
8+
import com.typesafe.config.ConfigFactory;
9+
import com.typesafe.config.ConfigValueFactory;
10+
11+
public class Issue466 extends ServerFeature {
12+
13+
{
14+
15+
use(ConfigFactory.empty().withValue("application.secret",
16+
ConfigValueFactory.fromAnyRef("1234Querty")));
17+
18+
cookieSession()
19+
.cookie()
20+
.maxAge(86400)
21+
.name("user");
22+
23+
get("/466/home", req -> {
24+
Session session = req.session();
25+
return session.attributes();
26+
});
27+
28+
get("/466/nosession", req -> {
29+
return "OK";
30+
});
31+
32+
get("/466/emptysession", req -> {
33+
Session session = req.session();
34+
return session.attributes();
35+
});
36+
37+
get("/466/session", req -> {
38+
Session session = req.session();
39+
session.set("foo", "bar");
40+
return session.attributes();
41+
});
42+
43+
get("/466/destroy", req -> {
44+
req.session().destroy();
45+
return Results.redirect("/466/home");
46+
});
47+
48+
}
49+
50+
@Test
51+
public void shouldDestroyAllSessionData() throws Exception {
52+
request()
53+
.dontFollowRedirect()
54+
.get("/466/session")
55+
.expect("{foo=bar}");
56+
57+
request()
58+
.dontFollowRedirect()
59+
.get("/466/destroy")
60+
.execute()
61+
.header("Set-Cookie", "user=;Version=1;Path=/;HttpOnly;Max-Age=0;Expires=Thu, 01-Jan-1970 00:00:00 GMT");
62+
}
63+
64+
@Test
65+
public void shouldNotCreateSessionCookie() throws Exception {
66+
request()
67+
.dontFollowRedirect()
68+
.get("/466/nosession")
69+
.execute()
70+
.header("Set-Cookie", (String) null);
71+
}
72+
73+
@Test
74+
public void shouldNotCreateEmptySessionCookie() throws Exception {
75+
request()
76+
.dontFollowRedirect()
77+
.get("/466/emptysession")
78+
.execute()
79+
.header("Set-Cookie", (String) null);
80+
}
81+
82+
@Test
83+
public void shouldDestroyAllSessionDataFollowRedirect() throws Exception {
84+
request()
85+
.get("/466/session")
86+
.expect("{foo=bar}");
87+
88+
request()
89+
.get("/466/destroy")
90+
.expect("{}");
91+
}
92+
}

‎jooby/src/main/java/org/jooby/Response.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ default void download(final String filename, final File file) throws Throwable {
312312
* @return This response.
313313
*/
314314
default Response cookie(final String name, final String value) {
315-
return cookie(new Cookie.Definition(name, value).toCookie());
315+
return cookie(new Cookie.Definition(name, value));
316316
}
317317

318318
/**
@@ -321,10 +321,7 @@ default Response cookie(final String name, final String value) {
321321
* @param cookie A cookie definition.
322322
* @return This response.
323323
*/
324-
default Response cookie(final Cookie.Definition cookie) {
325-
requireNonNull(cookie, "A cookie is required.");
326-
return cookie(cookie.toCookie());
327-
}
324+
Response cookie(final Cookie.Definition cookie);
328325

329326
/**
330327
* Adds the specified cookie to the response.

‎jooby/src/main/java/org/jooby/internal/CookieSessionManager.java‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public void requestDone(final Session session) {
9595

9696
@Override
9797
public Definition cookie() {
98-
return cookie;
98+
return new Definition(cookie);
9999
}
100100

101101
private Map<String, String> attributes(final String raw) {
@@ -105,24 +105,25 @@ private Map<String, String> attributes(final String raw) {
105105

106106
private Route.After saveCookie() {
107107
return (req, rsp, result) -> {
108-
Session session = req.session();
109-
Optional<String> value = req.cookie(cookie.name().get()).toOptional();
110-
Map<String, String> initial = value
111-
.map(this::attributes)
112-
.orElse(Collections.emptyMap());
113-
Map<String, String> attributes = session.attributes();
114-
// is dirty?
115-
boolean dirty = !initial.equals(attributes);
116-
log.debug("session dirty: {}", dirty);
117-
if (dirty) {
118-
log.debug("saving session cookie");
119-
String encoded = Cookie.URL_ENCODER.apply(attributes);
120-
String signed = Cookie.Signature.sign(encoded, secret);
121-
rsp.cookie(new Cookie.Definition(cookie).value(signed));
122-
} else if (timeout > 0) {
123-
// touch session
124-
value.ifPresent(raw -> rsp.cookie(new Cookie.Definition(cookie).value(raw)));
125-
}
108+
req.ifSession().ifPresent(session -> {
109+
Optional<String> value = req.cookie(cookie.name().get()).toOptional();
110+
Map<String, String> initial = value
111+
.map(this::attributes)
112+
.orElse(Collections.emptyMap());
113+
Map<String, String> attributes = session.attributes();
114+
// is dirty?
115+
boolean dirty = !initial.equals(attributes);
116+
log.debug("session dirty: {}", dirty);
117+
if (dirty) {
118+
log.debug("saving session cookie");
119+
String encoded = Cookie.URL_ENCODER.apply(attributes);
120+
String signed = Cookie.Signature.sign(encoded, secret);
121+
rsp.cookie(new Cookie.Definition(cookie).value(signed));
122+
} else if (timeout > 0) {
123+
// touch session
124+
value.ifPresent(raw -> rsp.cookie(new Cookie.Definition(cookie).value(raw)));
125+
}
126+
});
126127
return result;
127128
};
128129
}

‎jooby/src/main/java/org/jooby/internal/RequestImpl.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,13 @@ public void done() {
422422
}
423423

424424
private Session setSession(final SessionManager sm, final Response rsp, final Session gsession) {
425-
Session rsession = new RequestScopedSession(sm, rsp, gsession,
426-
() -> this.reqSession = null);
425+
Session rsession = new RequestScopedSession(sm, rsp, gsession, this::destroySession);
427426
reqSession = Optional.of(rsession);
428427
return rsession;
429428
}
430429

430+
private void destroySession() {
431+
this.reqSession = Optional.empty();
432+
}
433+
431434
}

‎jooby/src/main/java/org/jooby/internal/RequestScopedSession.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public void destroy() {
129129
// clear cookie
130130
org.jooby.Cookie.Definition cookie = sm.cookie();
131131
log.debug(" removing cookie: {}", cookie);
132-
rsp.clearCookie(cookie.name().get());
132+
rsp.cookie(cookie.maxAge(0));
133133
// destroy session from storage
134134
sm.destroy(session);
135135

‎jooby/src/main/java/org/jooby/internal/ResponseImpl.java‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737

3838
import org.jooby.Asset;
3939
import org.jooby.Cookie;
40+
import org.jooby.Cookie.Definition;
4041
import org.jooby.Deferred;
4142
import org.jooby.MediaType;
4243
import org.jooby.Mutant;
@@ -141,18 +142,32 @@ public void download(final String filename, final String location) throws Throwa
141142
}
142143

143144
@Override
144-
public Response cookie(final Cookie cookie) {
145-
requireNonNull(cookie, "A cookie is required.");
146-
cookies.put(cookie.name(), cookie);
147-
return this;
145+
public Response clearCookie(final String name) {
146+
requireNonNull(name, "Cookie's name required.");
147+
return cookie(new Cookie.Definition(name).maxAge(0));
148148
}
149149

150150
@Override
151-
public Response clearCookie(final String name) {
152-
requireNonNull(name, "A cookie's name is required.");
153-
if (cookies.remove(name) == null) {
154-
// cookie was set in a previous req, we must send a expire header.
155-
cookies.put(name, new Cookie.Definition(name, "").maxAge(0).toCookie());
151+
public Response cookie(final Definition cookie) {
152+
requireNonNull(cookie, "Cookie required.");
153+
// use default path if none-set
154+
cookie.path(cookie.path().orElse(Route.normalize(req.contextPath() + "/")));
155+
return cookie(cookie.toCookie());
156+
}
157+
158+
@Override
159+
public Response cookie(final Cookie cookie) {
160+
requireNonNull(cookie, "Cookie required.");
161+
String name = cookie.name();
162+
// clear cookie?
163+
if (cookie.maxAge() == 0) {
164+
// clear previously set cookie, otherwise ignore them
165+
if (cookies.remove(name) == null) {
166+
// cookie was set in a previous req, we must send a expire header.
167+
cookies.put(name, cookie);
168+
}
169+
} else {
170+
cookies.put(name, cookie);
156171
}
157172
return this;
158173
}
@@ -370,8 +385,10 @@ public void complete(final Complete handler) {
370385

371386
private void writeCookies() {
372387
if (cookies.size() > 0) {
373-
rsp.header("Set-Cookie",
374-
cookies.values().stream().map(Cookie::encode).collect(Collectors.toList()));
388+
List<String> setCookie = cookies.values().stream()
389+
.map(Cookie::encode)
390+
.collect(Collectors.toList());
391+
rsp.header("Set-Cookie", setCookie);
375392
cookies.clear();
376393
}
377394
}

‎jooby/src/main/java/org/jooby/internal/ServerSessionManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ public void requestDone(final Session session) {
124124

125125
@Override
126126
public Cookie.Definition cookie() {
127-
return template;
127+
return new Cookie.Definition(template);
128128
}
129129

130130
private void createOrUpdate(final SessionImpl session) {

‎jooby/src/test/java/org/jooby/ResponseTest.java‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.util.LinkedList;
1010
import java.util.Optional;
1111

12+
import org.jooby.Cookie.Definition;
1213
import org.jooby.Route.After;
1314
import org.jooby.Route.Complete;
1415
import org.junit.Test;
@@ -25,6 +26,11 @@ public void download(final String filename, final InputStream stream) throws Exc
2526
public void download(final String filename, final String location) throws Exception {
2627
}
2728

29+
@Override
30+
public Response cookie(final Definition cookie) {
31+
throw new UnsupportedOperationException();
32+
}
33+
2834
@Override
2935
public Response cookie(final Cookie cookie) {
3036
throw new UnsupportedOperationException();
@@ -232,31 +238,16 @@ public Response length(final long length) {
232238

233239
@Test
234240
public void cookieWithNameAndValue() throws Exception {
235-
LinkedList<Cookie> dataList = new LinkedList<>();
241+
LinkedList<Cookie.Definition> dataList = new LinkedList<>();
236242
new ResponseMock() {
237243
@Override
238-
public Response cookie(final Cookie cookie) {
244+
public Response cookie(final Cookie.Definition cookie) {
239245
dataList.add(cookie);
240246
return this;
241247
}
242248
}.cookie("name", "value");
243249

244-
assertEquals("name", dataList.getFirst().name());
245-
assertEquals("value", dataList.getFirst().value().get());
246-
}
247-
248-
@Test
249-
public void cookieWith() throws Exception {
250-
LinkedList<Cookie> dataList = new LinkedList<>();
251-
new ResponseMock() {
252-
@Override
253-
public Response cookie(final Cookie cookie) {
254-
dataList.add(cookie);
255-
return this;
256-
}
257-
}.cookie(new Cookie.Definition("name", "value"));
258-
259-
assertEquals("name", dataList.getFirst().name());
250+
assertEquals("name", dataList.getFirst().name().get());
260251
assertEquals("value", dataList.getFirst().value().get());
261252
}
262253

0 commit comments

Comments
 (0)