Skip to content

Commit 2c1474c

Browse files
committed
Drop cookie value unescaping, close AsyncHttpClient#858
1 parent 9ffa8ae commit 2c1474c

File tree

9 files changed

+312
-313
lines changed

9 files changed

+312
-313
lines changed

src/main/java/com/ning/http/client/cookie/Cookie.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
public class Cookie {
1616

17-
public static Cookie newValidCookie(String name, String value, String domain, String rawValue, String path, long expires, int maxAge, boolean secure, boolean httpOnly) {
17+
public static Cookie newValidCookie(String name, String value, boolean wrap, String domain, String path, long expires, int maxAge, boolean secure, boolean httpOnly) {
1818

1919
if (name == null) {
2020
throw new NullPointerException("name");
@@ -56,7 +56,7 @@ public static Cookie newValidCookie(String name, String value, String domain, St
5656
domain = validateValue("domain", domain);
5757
path = validateValue("path", path);
5858

59-
return new Cookie(name, value, rawValue, domain, path, expires, maxAge, secure, httpOnly);
59+
return new Cookie(name, value, wrap, domain, path, expires, maxAge, secure, httpOnly);
6060
}
6161

6262
private static String validateValue(String name, String value) {
@@ -84,18 +84,18 @@ private static String validateValue(String name, String value) {
8484

8585
private final String name;
8686
private final String value;
87-
private final String rawValue;
87+
private final boolean wrap;
8888
private final String domain;
8989
private final String path;
9090
private long expires;
9191
private final int maxAge;
9292
private final boolean secure;
9393
private final boolean httpOnly;
9494

95-
public Cookie(String name, String value, String rawValue, String domain, String path, long expires, int maxAge, boolean secure, boolean httpOnly) {
95+
public Cookie(String name, String value, boolean wrap, String domain, String path, long expires, int maxAge, boolean secure, boolean httpOnly) {
9696
this.name = name;
9797
this.value = value;
98-
this.rawValue = rawValue;
98+
this.wrap = wrap;
9999
this.domain = domain;
100100
this.path = path;
101101
this.expires = expires;
@@ -116,8 +116,8 @@ public String getValue() {
116116
return value;
117117
}
118118

119-
public String getRawValue() {
120-
return rawValue;
119+
public boolean isWrap() {
120+
return wrap;
121121
}
122122

123123
public String getPath() {
@@ -145,7 +145,10 @@ public String toString() {
145145
StringBuilder buf = new StringBuilder();
146146
buf.append(name);
147147
buf.append('=');
148-
buf.append(rawValue);
148+
if (wrap)
149+
buf.append('"').append(value).append('"');
150+
else
151+
buf.append(value);
149152
if (domain != null) {
150153
buf.append("; domain=");
151154
buf.append(domain);

src/main/java/com/ning/http/client/cookie/CookieDecoder.java

Lines changed: 177 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -12,23 +12,36 @@
1212
*/
1313
package com.ning.http.client.cookie;
1414

15-
import com.ning.http.util.StringUtils;
15+
import org.slf4j.Logger;
16+
import org.slf4j.LoggerFactory;
17+
18+
import java.nio.CharBuffer;
19+
20+
import static com.ning.http.client.cookie.CookieUtil.*;
1621

1722
public class CookieDecoder {
1823

24+
private static final Logger LOGGER = LoggerFactory.getLogger(CookieDecoder.class);
25+
1926
/**
2027
* Decodes the specified HTTP header value into {@link Cookie}.
2128
*
2229
* @return the decoded {@link Cookie}
2330
*/
2431
public static Cookie decode(String header) {
2532

26-
if (header.length() == 0)
33+
if (header == null) {
34+
throw new NullPointerException("header");
35+
}
36+
37+
final int headerLen = header.length();
38+
39+
if (headerLen == 0) {
2740
return null;
41+
}
2842

29-
KeyValuePairsParser pairsParser = new KeyValuePairsParser();
43+
CookieBuilder cookieBuilder = null;
3044

31-
final int headerLen = header.length();
3245
loop: for (int i = 0;;) {
3346

3447
// Skip spaces and separators.
@@ -49,111 +62,193 @@ public static Cookie decode(String header) {
4962
break;
5063
}
5164

52-
int newNameStart = i;
53-
int newNameEnd = i;
54-
String value;
55-
String rawValue;
65+
int nameBegin = i;
66+
int nameEnd = i;
67+
int valueBegin = -1;
68+
int valueEnd = -1;
5669

57-
if (i == headerLen) {
58-
value = rawValue = null;
59-
} else {
70+
if (i != headerLen) {
6071
keyValLoop: for (;;) {
6172

6273
char curChar = header.charAt(i);
6374
if (curChar == ';') {
6475
// NAME; (no value till ';')
65-
newNameEnd = i;
66-
value = rawValue = null;
76+
nameEnd = i;
77+
valueBegin = valueEnd = -1;
6778
break keyValLoop;
79+
6880
} else if (curChar == '=') {
6981
// NAME=VALUE
70-
newNameEnd = i;
82+
nameEnd = i;
7183
i++;
7284
if (i == headerLen) {
7385
// NAME= (empty value, i.e. nothing after '=')
74-
value = rawValue = "";
86+
valueBegin = valueEnd = 0;
7587
break keyValLoop;
7688
}
7789

78-
int newValueStart = i;
79-
char c = header.charAt(i);
80-
if (c == '"' || c == '\'') {
81-
// NAME="VALUE" or NAME='VALUE'
82-
StringBuilder newValueBuf = StringUtils.stringBuilder();
83-
84-
int rawValueStart = i;
85-
int rawValueEnd = i;
86-
87-
final char q = c;
88-
boolean hadBackslash = false;
89-
i++;
90-
for (;;) {
91-
if (i == headerLen) {
92-
value = newValueBuf.toString();
93-
// only need to compute raw value for cookie
94-
// value which is at most in 2nd position
95-
rawValue = header.substring(rawValueStart, rawValueEnd);
96-
break keyValLoop;
97-
}
98-
if (hadBackslash) {
99-
hadBackslash = false;
100-
c = header.charAt(i++);
101-
rawValueEnd = i;
102-
switch (c) {
103-
case '\\':
104-
case '"':
105-
case '\'':
106-
// Escape last backslash.
107-
newValueBuf.setCharAt(newValueBuf.length() - 1, c);
108-
break;
109-
default:
110-
// Do not escape last backslash.
111-
newValueBuf.append(c);
112-
}
113-
} else {
114-
c = header.charAt(i++);
115-
rawValueEnd = i;
116-
if (c == q) {
117-
value = newValueBuf.toString();
118-
// only need to compute raw value for
119-
// cookie value which is at most in 2nd
120-
// position
121-
rawValue = header.substring(rawValueStart, rawValueEnd);
122-
break keyValLoop;
123-
}
124-
newValueBuf.append(c);
125-
if (c == '\\') {
126-
hadBackslash = true;
127-
}
128-
}
129-
}
130-
} else {
131-
// NAME=VALUE;
132-
int semiPos = header.indexOf(';', i);
133-
if (semiPos > 0) {
134-
value = rawValue = header.substring(newValueStart, semiPos);
135-
i = semiPos;
136-
} else {
137-
value = rawValue = header.substring(newValueStart);
138-
i = headerLen;
139-
}
140-
}
90+
valueBegin = i;
91+
// NAME=VALUE;
92+
int semiPos = header.indexOf(';', i);
93+
valueEnd = i = semiPos > 0 ? semiPos : headerLen;
14194
break keyValLoop;
14295
} else {
14396
i++;
14497
}
14598

14699
if (i == headerLen) {
147100
// NAME (no value till the end of string)
148-
newNameEnd = headerLen;
149-
value = rawValue = null;
101+
nameEnd = headerLen;
102+
valueBegin = valueEnd = -1;
150103
break;
151104
}
152105
}
153106
}
154107

155-
pairsParser.parseKeyValuePair(header, newNameStart, newNameEnd, value, rawValue);
108+
if (valueEnd > 0 && header.charAt(valueEnd - 1) == ',') {
109+
// old multiple cookies separator, skipping it
110+
valueEnd--;
111+
}
112+
113+
if (cookieBuilder == null) {
114+
// cookie name-value pair
115+
if (nameBegin == -1 || nameBegin == nameEnd) {
116+
LOGGER.debug("Skipping cookie with null name");
117+
return null;
118+
}
119+
120+
if (valueBegin == -1) {
121+
LOGGER.debug("Skipping cookie with null value");
122+
return null;
123+
}
124+
125+
CharSequence wrappedValue = CharBuffer.wrap(header, valueBegin, valueEnd);
126+
CharSequence unwrappedValue = unwrapValue(wrappedValue);
127+
if (unwrappedValue == null) {
128+
LOGGER.debug("Skipping cookie because starting quotes are not properly balanced in '{}'", unwrappedValue);
129+
return null;
130+
}
131+
132+
final String name = header.substring(nameBegin, nameEnd);
133+
134+
final boolean wrap = unwrappedValue.length() != valueEnd - valueBegin;
135+
136+
cookieBuilder = new CookieBuilder(name, unwrappedValue.toString(), wrap);
137+
138+
} else {
139+
// cookie attribute
140+
String attrValue = valueBegin == -1 ? null : header.substring(valueBegin, valueEnd);
141+
cookieBuilder.appendAttribute(header, nameBegin, nameEnd, attrValue);
142+
}
143+
}
144+
return cookieBuilder.cookie();
145+
}
146+
147+
private static class CookieBuilder {
148+
149+
private static final String PATH = "Path";
150+
151+
private static final String EXPIRES = "Expires";
152+
153+
private static final String MAX_AGE = "Max-Age";
154+
155+
private static final String DOMAIN = "Domain";
156+
157+
private static final String SECURE = "Secure";
158+
159+
private static final String HTTPONLY = "HTTPOnly";
160+
161+
private final String name;
162+
private final String value;
163+
private final boolean wrap;
164+
private String domain;
165+
private String path;
166+
private int maxAge = Integer.MIN_VALUE;
167+
private String expires;
168+
private boolean secure;
169+
private boolean httpOnly;
170+
171+
public CookieBuilder(String name, String value, boolean wrap) {
172+
this.name = name;
173+
this.value = value;
174+
this.wrap = wrap;
175+
}
176+
177+
public Cookie cookie() {
178+
return new Cookie(name, value, wrap, domain, path, computeExpires(expires), maxAge, secure, httpOnly);
179+
}
180+
181+
/**
182+
* Parse and store a key-value pair. First one is considered to be the
183+
* cookie name/value. Unknown attribute names are silently discarded.
184+
*
185+
* @param header
186+
* the HTTP header
187+
* @param keyStart
188+
* where the key starts in the header
189+
* @param keyEnd
190+
* where the key ends in the header
191+
* @param value
192+
* the decoded value
193+
*/
194+
public void appendAttribute(String header, int keyStart, int keyEnd, String value) {
195+
setCookieAttribute(header, keyStart, keyEnd, value);
196+
}
197+
198+
private void setCookieAttribute(String header, int keyStart, int keyEnd, String value) {
199+
200+
int length = keyEnd - keyStart;
201+
202+
if (length == 4) {
203+
parse4(header, keyStart, value);
204+
} else if (length == 6) {
205+
parse6(header, keyStart, value);
206+
} else if (length == 7) {
207+
parse7(header, keyStart, value);
208+
} else if (length == 8) {
209+
parse8(header, keyStart, value);
210+
}
211+
}
212+
213+
private void parse4(String header, int nameStart, String value) {
214+
if (header.regionMatches(true, nameStart, PATH, 0, 4)) {
215+
path = value;
216+
}
217+
}
218+
219+
private void parse6(String header, int nameStart, String value) {
220+
if (header.regionMatches(true, nameStart, DOMAIN, 0, 5)) {
221+
domain = value.length() > 0 ? value.toString() : null;
222+
} else if (header.regionMatches(true, nameStart, SECURE, 0, 5)) {
223+
secure = true;
224+
}
225+
}
226+
227+
private void setExpire(String value) {
228+
expires = value;
229+
}
230+
231+
private void setMaxAge(String value) {
232+
try {
233+
maxAge = Math.max(Integer.valueOf(value), 0);
234+
} catch (NumberFormatException e1) {
235+
// ignore failure to parse -> treat as session cookie
236+
}
237+
}
238+
239+
private void parse7(String header, int nameStart, String value) {
240+
if (header.regionMatches(true, nameStart, EXPIRES, 0, 7)) {
241+
setExpire(value);
242+
} else if (header.regionMatches(true, nameStart, MAX_AGE, 0, 7)) {
243+
setMaxAge(value);
244+
}
245+
}
246+
247+
private void parse8(String header, int nameStart, String value) {
248+
249+
if (header.regionMatches(true, nameStart, HTTPONLY, 0, 8)) {
250+
httpOnly = true;
251+
}
156252
}
157-
return pairsParser.cookie();
158253
}
159254
}

0 commit comments

Comments
 (0)