Skip to content

Commit 9189d11

Browse files
arturobernalgarturobernalg
authored
HTTPCLIENT-2415: Normalize CookieOrigin path for cookie matching (#803)
Strip query / fragment from CookieOrigin path per RFC 6265. Add regression tests for request-targets containing '?', '#'.
1 parent d8d09ed commit 9189d11

File tree

4 files changed

+214
-6
lines changed

4 files changed

+214
-6
lines changed

‎httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestAddCookies.java‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,28 @@ public class RequestAddCookies implements HttpRequestInterceptor {
7676

7777
private static final Logger LOG = LoggerFactory.getLogger(RequestAddCookies.class);
7878

79+
private static String normalizeRequestPath(final String rawPath) {
80+
if (TextUtils.isBlank(rawPath)) {
81+
return "/";
82+
}
83+
int end = rawPath.length();
84+
85+
final int queryIndex = rawPath.indexOf('?');
86+
if (queryIndex >= 0) {
87+
end = queryIndex;
88+
}
89+
final int fragmentIndex = rawPath.indexOf('#');
90+
if (fragmentIndex >= 0 && fragmentIndex < end) {
91+
end = fragmentIndex;
92+
}
93+
if (end == 0) {
94+
return "/";
95+
}
96+
if (end == rawPath.length()) {
97+
return rawPath;
98+
}
99+
return rawPath.substring(0, end);
100+
}
79101
public RequestAddCookies() {
80102
super();
81103
}
@@ -100,7 +122,6 @@ public void process(final HttpRequest request, final EntityDetails entity, final
100122
return;
101123
}
102124

103-
104125
final HttpClientContext clientContext = HttpClientContext.cast(context);
105126
final String exchangeId = clientContext.getExchangeId();
106127

@@ -141,10 +162,9 @@ public void process(final HttpRequest request, final EntityDetails entity, final
141162
}
142163

143164
final URIAuthority authority = request.getAuthority();
144-
String path = request.getPath();
145-
if (TextUtils.isEmpty(path)) {
146-
path = "/";
147-
}
165+
166+
final String path = normalizeRequestPath(request.getPath());
167+
148168
String hostName = authority != null ? authority.getHostName() : null;
149169
if (hostName == null) {
150170
hostName = route.getTargetHost().getHostName();

‎httpclient5/src/test/java/org/apache/hc/client5/http/cookie/TestCookieOrigin.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,11 @@ void testEmptyPath() {
7676
Assertions.assertEquals("/", origin.getPath());
7777
Assertions.assertFalse(origin.isSecure());
7878
}
79+
@Test
80+
void testNormalizePathRejectsNull() {
81+
Assertions.assertThrows(NullPointerException.class, () ->
82+
new CookieOrigin("somehost", 80, null, false));
83+
}
7984

8085
}
8186

‎httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestBasicCookieAttribHandlers.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,5 +399,4 @@ void testBasicHttpOnlyParse() throws Exception {
399399
h.parse(cookie, "anyone");
400400
Assertions.assertTrue(cookie.isHttpOnly());
401401
}
402-
403402
}

‎httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestAddCookies.java‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,62 @@ void testAddCookies() throws Exception {
126126
Assertions.assertFalse(cookieOrigin.isSecure());
127127
}
128128

129+
@Test
130+
void testAddCookiesWithQueryParams() throws Exception {
131+
final BasicCookieStore store = new BasicCookieStore();
132+
final BasicClientCookie cookie = new BasicClientCookie("name", "value");
133+
cookie.setDomain("localhost.local");
134+
cookie.setPath("/stuff");
135+
store.addCookie(cookie);
136+
137+
final HttpRequest request = new BasicHttpRequest("GET", "/stuff?foo=bar");
138+
final HttpRoute route = new HttpRoute(this.target, null, false);
139+
140+
final HttpClientContext context = HttpClientContext.create();
141+
context.setRoute(route);
142+
context.setCookieStore(store);
143+
context.setCookieSpecRegistry(this.cookieSpecRegistry);
144+
145+
final HttpRequestInterceptor interceptor = RequestAddCookies.INSTANCE;
146+
interceptor.process(request, null, context);
147+
148+
final Header[] headers = request.getHeaders("Cookie");
149+
Assertions.assertNotNull(headers);
150+
Assertions.assertEquals(1, headers.length);
151+
Assertions.assertEquals("name=value", headers[0].getValue());
152+
153+
final CookieOrigin cookieOrigin = context.getCookieOrigin();
154+
Assertions.assertEquals("/stuff", cookieOrigin.getPath());
155+
}
156+
157+
@Test
158+
void testAddCookiesWithFragment() throws Exception {
159+
final BasicCookieStore store = new BasicCookieStore();
160+
final BasicClientCookie cookie = new BasicClientCookie("name", "value");
161+
cookie.setDomain("localhost.local");
162+
cookie.setPath("/stuff");
163+
store.addCookie(cookie);
164+
165+
final HttpRequest request = new BasicHttpRequest("GET", "/stuff#section");
166+
final HttpRoute route = new HttpRoute(this.target, null, false);
167+
168+
final HttpClientContext context = HttpClientContext.create();
169+
context.setRoute(route);
170+
context.setCookieStore(store);
171+
context.setCookieSpecRegistry(this.cookieSpecRegistry);
172+
173+
final HttpRequestInterceptor interceptor = RequestAddCookies.INSTANCE;
174+
interceptor.process(request, null, context);
175+
176+
final Header[] headers = request.getHeaders("Cookie");
177+
Assertions.assertNotNull(headers);
178+
Assertions.assertEquals(1, headers.length);
179+
Assertions.assertEquals("name=value", headers[0].getValue());
180+
181+
final CookieOrigin cookieOrigin = context.getCookieOrigin();
182+
Assertions.assertEquals("/stuff", cookieOrigin.getPath());
183+
}
184+
129185
@Test
130186
void testCookiesForConnectRequest() throws Exception {
131187
final HttpRequest request = new BasicHttpRequest("CONNECT", "www.somedomain.com");
@@ -432,5 +488,133 @@ void testSkipAddingCookiesWhenCookieHeaderPresent() throws Exception {
432488
Assertions.assertEquals(1, headers.length);
433489
Assertions.assertEquals("existingCookie=existingValue", headers[0].getValue());
434490
}
491+
@Test
492+
void testAddCookiesWithRequestPathContainingQuery() throws Exception {
493+
final HttpRequest request = new BasicHttpRequest("GET", "/stuff?foo=bar");
494+
495+
final HttpRoute route = new HttpRoute(this.target, null, false);
496+
497+
final CookieStore store = new BasicCookieStore();
498+
final BasicClientCookie cookie = new BasicClientCookie("name", "value");
499+
cookie.setDomain("localhost.local");
500+
cookie.setPath("/stuff");
501+
store.addCookie(cookie);
502+
503+
final HttpClientContext context = HttpClientContext.create();
504+
context.setRoute(route);
505+
context.setCookieStore(store);
506+
context.setCookieSpecRegistry(this.cookieSpecRegistry);
507+
508+
final HttpRequestInterceptor interceptor = RequestAddCookies.INSTANCE;
509+
interceptor.process(request, null, context);
510+
511+
final Header[] headers = request.getHeaders("Cookie");
512+
Assertions.assertNotNull(headers);
513+
Assertions.assertEquals(1, headers.length);
514+
Assertions.assertEquals("name=value", headers[0].getValue());
515+
516+
final CookieOrigin cookieOrigin = context.getCookieOrigin();
517+
Assertions.assertNotNull(cookieOrigin);
518+
Assertions.assertEquals("/stuff", cookieOrigin.getPath());
519+
}
520+
521+
@Test
522+
void testAddCookiesWithRequestPathContainingFragment() throws Exception {
523+
final HttpRequest request = new BasicHttpRequest("GET", "/stuff#frag");
524+
525+
final HttpRoute route = new HttpRoute(this.target, null, false);
526+
527+
final CookieStore store = new BasicCookieStore();
528+
final BasicClientCookie cookie = new BasicClientCookie("name", "value");
529+
cookie.setDomain("localhost.local");
530+
cookie.setPath("/stuff");
531+
store.addCookie(cookie);
532+
533+
final HttpClientContext context = HttpClientContext.create();
534+
context.setRoute(route);
535+
context.setCookieStore(store);
536+
context.setCookieSpecRegistry(this.cookieSpecRegistry);
537+
538+
final HttpRequestInterceptor interceptor = RequestAddCookies.INSTANCE;
539+
interceptor.process(request, null, context);
540+
541+
final Header[] headers = request.getHeaders("Cookie");
542+
Assertions.assertNotNull(headers);
543+
Assertions.assertEquals(1, headers.length);
544+
Assertions.assertEquals("name=value", headers[0].getValue());
545+
546+
final CookieOrigin cookieOrigin = context.getCookieOrigin();
547+
Assertions.assertNotNull(cookieOrigin);
548+
Assertions.assertEquals("/stuff", cookieOrigin.getPath());
549+
}
550+
551+
@Test
552+
void testAddCookiesWithRootPathQuery() throws Exception {
553+
// Should behave as path "/" (query ignored)
554+
final HttpRequest request = new BasicHttpRequest("GET", "/?foo=bar");
555+
556+
final HttpRoute route = new HttpRoute(this.target, null, false);
557+
558+
final HttpClientContext context = HttpClientContext.create();
559+
context.setRoute(route);
560+
context.setCookieStore(this.cookieStore); // setup cookies are path "/"
561+
context.setCookieSpecRegistry(this.cookieSpecRegistry);
562+
563+
final HttpRequestInterceptor interceptor = RequestAddCookies.INSTANCE;
564+
interceptor.process(request, null, context);
565+
566+
final Header[] headers = request.getHeaders("Cookie");
567+
Assertions.assertNotNull(headers);
568+
Assertions.assertEquals(1, headers.length);
569+
Assertions.assertEquals("name1=value1; name2=value2", headers[0].getValue());
570+
571+
final CookieOrigin cookieOrigin = context.getCookieOrigin();
572+
Assertions.assertNotNull(cookieOrigin);
573+
Assertions.assertEquals("/", cookieOrigin.getPath());
574+
}
575+
576+
577+
@Test
578+
void testCookieOriginPathNormalization() throws Exception {
579+
final String[][] cases = new String[][]{
580+
{"", "/"},
581+
{" ", "/"},
582+
{"\t", "/"},
583+
{"?", "/"},
584+
{"#","/"},
585+
{"?foo", "/"},
586+
{"#frag", "/"},
587+
{"/", "/"},
588+
{"/?foo", "/"},
589+
{"/#frag", "/"},
590+
{"/stuff", "/stuff"},
591+
{"/stuff?foo", "/stuff"},
592+
{"/stuff#frag", "/stuff"},
593+
{"/stuff?foo#bar", "/stuff"},
594+
{"/stuff#bar?foo", "/stuff"},
595+
{"/stuff/", "/stuff/"},
596+
{"/stuff/?foo", "/stuff/"},
597+
};
598+
599+
for (final String[] c : cases) {
600+
final String input = c[0];
601+
final String expected = c[1];
602+
603+
final HttpRequest request = new BasicHttpRequest("GET", input);
604+
final HttpRoute route = new HttpRoute(this.target, null, false);
605+
606+
final HttpClientContext context = HttpClientContext.create();
607+
context.setRoute(route);
608+
context.setCookieStore(new BasicCookieStore());
609+
context.setCookieSpecRegistry(this.cookieSpecRegistry);
610+
611+
RequestAddCookies.INSTANCE.process(request, null, context);
612+
613+
final CookieOrigin origin = context.getCookieOrigin();
614+
Assertions.assertNotNull(origin, "input=" + input);
615+
Assertions.assertEquals(expected, origin.getPath(), "input=" + input);
616+
}
617+
}
618+
435619

436620
}

0 commit comments

Comments
 (0)