diff options
author | Bjørn Christian Seime <bjorncs@oath.com> | 2018-09-14 09:18:49 +0200 |
---|---|---|
committer | gjoranv <gv@oath.com> | 2019-01-21 15:09:21 +0100 |
commit | b48b717506c2d40958b5b627863be90aca81cf51 (patch) | |
tree | ec5372c5b5d6ac83221ffb035c2bbfe5d413e086 | |
parent | 178903a8b938738e7fc1db18eea850863f9461ed (diff) |
Fix todos in Cookie
8 files changed, 20 insertions, 168 deletions
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java index a43310aff51..d65d2ecb30d 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java @@ -7,15 +7,11 @@ import org.eclipse.jetty.server.Response; import java.net.HttpCookie; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Set; -import java.util.StringTokenizer; import java.util.concurrent.TimeUnit; -import java.util.logging.Level; -import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -24,22 +20,17 @@ import java.util.stream.StreamSupport; * * Note: RFC 2109 and RFC 2965 is no longer supported. All fields that are not part of RFC 6265 are deprecated. * - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> + * @author Einar M R Rosenvinge * @author bjorncs */ public class Cookie { - private final static Logger log = Logger.getLogger(Cookie.class.getName()); - private final Set<Integer> ports = new HashSet<>(); private String name; private String value; private String domain; private String path; - private String comment; - private String commentUrl; private long maxAgeSeconds = Integer.MIN_VALUE; - private int version; private boolean secure; private boolean httpOnly; private boolean discard; @@ -53,10 +44,7 @@ public class Cookie { value = cookie.value; domain = cookie.domain; path = cookie.path; - comment = cookie.comment; - commentUrl = cookie.commentUrl; maxAgeSeconds = cookie.maxAgeSeconds; - version = cookie.version; secure = cookie.secure; httpOnly = cookie.httpOnly; discard = cookie.discard; @@ -103,38 +91,6 @@ public class Cookie { return this; } - @Deprecated - public String getComment() { - return comment; - } - - @Deprecated - public Cookie setComment(String comment) { - this.comment = comment; - return this; - } - - @Deprecated - public String getCommentURL() { - return getCommentUrl(); - } - - @Deprecated - public Cookie setCommentURL(String commentUrl) { - return setCommentUrl(commentUrl); - } - - @Deprecated - public String getCommentUrl() { - return commentUrl; - } - - @Deprecated - public Cookie setCommentUrl(String commentUrl) { - this.commentUrl = commentUrl; - return this; - } - public int getMaxAge(TimeUnit unit) { return (int)unit.convert(maxAgeSeconds, TimeUnit.SECONDS); } @@ -144,17 +100,6 @@ public class Cookie { return this; } - @Deprecated - public int getVersion() { - return version; - } - - @Deprecated - public Cookie setVersion(int version) { - this.version = version; - return this; - } - public boolean isSecure() { return secure; } @@ -173,29 +118,12 @@ public class Cookie { return this; } - @Deprecated - public boolean isDiscard() { - return discard; - } - - @Deprecated - public Cookie setDiscard(boolean discard) { - this.discard = discard; - return this; - } - - @Deprecated - public Set<Integer> ports() { - return ports; - } - @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; Cookie cookie = (Cookie) o; return maxAgeSeconds == cookie.maxAgeSeconds && - version == cookie.version && secure == cookie.secure && httpOnly == cookie.httpOnly && discard == cookie.discard && @@ -203,14 +131,12 @@ public class Cookie { Objects.equals(name, cookie.name) && Objects.equals(value, cookie.value) && Objects.equals(domain, cookie.domain) && - Objects.equals(path, cookie.path) && - Objects.equals(comment, cookie.comment) && - Objects.equals(commentUrl, cookie.commentUrl); + Objects.equals(path, cookie.path); } @Override public int hashCode() { - return Objects.hash(ports, name, value, domain, path, comment, commentUrl, maxAgeSeconds, version, secure, httpOnly, discard); + return Objects.hash(ports, name, value, domain, path, maxAgeSeconds, secure, httpOnly, discard); } @Override @@ -229,19 +155,12 @@ public class Cookie { return StreamSupport.stream(cookies.spliterator(), false) .map(cookie -> { HttpCookie httpCookie = new HttpCookie(cookie.getName(), cookie.getValue()); - httpCookie.setComment(cookie.getComment()); - httpCookie.setCommentURL(cookie.getCommentURL()); - httpCookie.setDiscard(cookie.isDiscard()); httpCookie.setDomain(cookie.getDomain()); httpCookie.setHttpOnly(cookie.isHttpOnly()); httpCookie.setMaxAge(cookie.getMaxAge(TimeUnit.SECONDS)); httpCookie.setPath(cookie.getPath()); httpCookie.setSecure(cookie.isSecure()); - httpCookie.setVersion(cookie.getVersion()); - String portList = cookie.ports().stream() - .map(Number::toString) - .collect(Collectors.joining(",")); - httpCookie.setPortlist(portList); + httpCookie.setVersion(0); return httpCookie.toString(); }) .collect(Collectors.joining(";")); @@ -255,29 +174,17 @@ public class Cookie { Cookie cookie = new Cookie(); cookie.setName(servletCookie.getName()); cookie.setValue(servletCookie.getValue()); - cookie.setComment(servletCookie.getComment()); cookie.setPath(servletCookie.getPath()); cookie.setDomain(servletCookie.getDomain()); cookie.setMaxAge(servletCookie.getMaxAge(), TimeUnit.SECONDS); cookie.setSecure(servletCookie.getSecure()); - cookie.setVersion(servletCookie.getVersion()); cookie.setHttpOnly(servletCookie.isHttpOnly()); return cookie; }) .collect(Collectors.toList()); } - /** - * @deprecated Use {@link #toSetCookieHeaderAll(Iterable)} instead. - */ - @Deprecated - public static String toSetCookieHeader(Iterable<? extends Cookie> cookies) { - List<String> encodedCookies = toSetCookieHeaderAll(cookies); - return encodedCookies.isEmpty() ? null : encodedCookies.get(0); - } - - // TODO Rename to toSetCookieHeader for Vespa 7 - public static List<String> toSetCookieHeaderAll(Iterable<? extends Cookie> cookies) { + public static List<String> toSetCookieHeaders(Iterable<? extends Cookie> cookies) { // Ugly, bot Jetty does not provide a dedicated cookie parser (will be included in Jetty 10) Response response = new Response(null, null); for (Cookie cookie : cookies) { @@ -293,45 +200,20 @@ public class Cookie { return new ArrayList<>(response.getHeaders("Set-Cookie")); } - // TODO Change return type to Cookie for Vespa 7 - public static List<Cookie> fromSetCookieHeader(String headerVal) { + public static Cookie fromSetCookieHeader(String headerVal) { return HttpCookie.parse(headerVal).stream() .map(httpCookie -> { Cookie cookie = new Cookie(); cookie.setName(httpCookie.getName()); cookie.setValue(httpCookie.getValue()); - cookie.setComment(httpCookie.getComment()); - cookie.setCommentUrl(httpCookie.getCommentURL()); - cookie.setDiscard(httpCookie.getDiscard()); cookie.setDomain(httpCookie.getDomain()); cookie.setHttpOnly(httpCookie.isHttpOnly()); - cookie.setMaxAge((int)httpCookie.getMaxAge(), TimeUnit.SECONDS); + cookie.setMaxAge((int) httpCookie.getMaxAge(), TimeUnit.SECONDS); cookie.setPath(httpCookie.getPath()); cookie.setSecure(httpCookie.getSecure()); - cookie.setVersion(httpCookie.getVersion()); - cookie.ports().addAll(parsePortList(httpCookie.getPortlist())); return cookie; }) - .collect(Collectors.toList()); - } - - - private static List<Integer> parsePortList(String rawPortList) { - if (rawPortList == null) return Collections.emptyList(); - - List<Integer> ports = new ArrayList<>(); - StringTokenizer tokenizer = new StringTokenizer(rawPortList, ","); - while (tokenizer.hasMoreTokens()) { - String rawPort = tokenizer.nextToken().trim(); - if (!rawPort.isEmpty()) { - try { - ports.add(Integer.parseInt(rawPort)); - } catch (NumberFormatException e) { - log.log(Level.FINE, "Unable to parse port: " + rawPort, e); - } - } - } - return ports; + .findFirst().get(); } } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpResponse.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpResponse.java index 8905549a002..f7138ba0e2b 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpResponse.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpResponse.java @@ -69,7 +69,7 @@ public class HttpResponse extends Response implements ServletOrJdiscHttpResponse } List<Cookie> ret = new LinkedList<>(); for (String cookie : cookies) { - ret.addAll(Cookie.fromSetCookieHeader(cookie)); + ret.add(Cookie.fromSetCookieHeader(cookie)); } return ret; } @@ -77,7 +77,7 @@ public class HttpResponse extends Response implements ServletOrJdiscHttpResponse public void encodeSetCookieHeader(List<Cookie> cookies) { headers().remove(HttpHeaders.Names.SET_COOKIE); for (Cookie cookie : cookies) { - headers().add(HttpHeaders.Names.SET_COOKIE, Cookie.toSetCookieHeaderAll(Arrays.asList(cookie))); + headers().add(HttpHeaders.Names.SET_COOKIE, Cookie.toSetCookieHeaders(Arrays.asList(cookie))); } } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapper.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapper.java index 8845f233fab..2b9c650d545 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapper.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapper.java @@ -8,7 +8,7 @@ import java.util.concurrent.TimeUnit; /** * Wrapper of Cookie. * - * @author tejalk + * @author Tejal Knot * */ public class JDiscCookieWrapper { @@ -23,11 +23,6 @@ public class JDiscCookieWrapper { return new JDiscCookieWrapper(cookie); } - @Deprecated - public String getComment() { - return cookie.getComment(); - } - public String getDomain() { return cookie.getDomain(); } @@ -52,16 +47,6 @@ public class JDiscCookieWrapper { return cookie.getValue(); } - @Deprecated - public int getVersion() { - return cookie.getVersion(); - } - - @Deprecated - public void setComment(String purpose) { - cookie.setComment(purpose); - } - public void setDomain(String pattern) { cookie.setDomain(pattern); } @@ -82,11 +67,6 @@ public class JDiscCookieWrapper { cookie.setValue(newValue); } - @Deprecated - public void setVersion(int version) { - cookie.setVersion(version); - } - /** * Return com.yahoo.jdisc.http.Cookie * diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java index 1c8383ae427..b603e7776f1 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java @@ -12,8 +12,6 @@ import java.util.List; /** * Servlet implementation for JDisc filter responses. - * - * @since 5.27 */ class ServletFilterResponse extends DiscFilterResponse { @@ -77,7 +75,7 @@ class ServletFilterResponse extends DiscFilterResponse { @Override public void setCookies(List<Cookie> cookies) { removeHeaders(HttpHeaders.Names.SET_COOKIE); - List<String> setCookieHeaders = Cookie.toSetCookieHeaderAll(cookies); + List<String> setCookieHeaders = Cookie.toSetCookieHeaders(cookies); setCookieHeaders.forEach(cookie -> addHeader(HttpHeaders.Names.SET_COOKIE, cookie)); } } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java index 5825d3f5194..48c8f577de9 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java @@ -18,8 +18,6 @@ import java.util.Map; /** * JDisc wrapper to use a {@link javax.servlet.http.HttpServletResponse} * with JDisc security filters. - * - * @since 5.26 */ public class ServletResponse extends HttpServletResponseWrapper implements ServletOrJdiscHttpResponse { @@ -60,7 +58,7 @@ public class ServletResponse extends HttpServletResponseWrapper implements Servl } List<Cookie> ret = new LinkedList<>(); for (String cookie : cookies) { - ret.addAll(Cookie.fromSetCookieHeader(cookie)); + ret.add(Cookie.fromSetCookieHeader(cookie)); } return ret; } diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java index dab4f91f631..4c651f79666 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java @@ -131,7 +131,6 @@ public class CookieTestCase { } @Test - @SuppressWarnings("deprecation") public void requireThatSetCookieCanBeDecoded() { final Cookie foo = new Cookie(); foo.setName("foo.name"); @@ -141,7 +140,6 @@ public class CookieTestCase { foo.setMaxAge(0, TimeUnit.SECONDS); foo.setSecure(true); foo.setHttpOnly(true); - foo.setVersion(1); assertDecodeSetCookie(foo, "foo.name=foo.value;Max-Age=0;Path=path;Domain=domain;Secure;HTTPOnly;"); final Cookie bar = new Cookie(); @@ -150,7 +148,6 @@ public class CookieTestCase { bar.setPath("path"); bar.setDomain("domain"); bar.setMaxAge(0, TimeUnit.SECONDS); - bar.setVersion(1); assertDecodeSetCookie(bar, "bar.name=bar.value;Max-Age=0;Path=path;Domain=domain;"); } @@ -193,11 +190,13 @@ public class CookieTestCase { } private static void assertEncodeCookie(String expectedResult, List<Cookie> cookies) { - assertThat(Cookie.toCookieHeader(cookies), equalTo(expectedResult)); + String actual = Cookie.toCookieHeader(cookies); + String expectedResult1 = expectedResult; + assertThat(actual, equalTo(expectedResult1)); } private static void assertEncodeSetCookie(List<String> expectedResult, List<Cookie> cookies) { - assertThat(Cookie.toSetCookieHeaderAll(cookies), containsInAnyOrder(expectedResult.toArray())); + assertThat(Cookie.toSetCookieHeaders(cookies), containsInAnyOrder(expectedResult.toArray())); } private static void assertDecodeCookie(List<Cookie> expected, String toDecode) { @@ -205,7 +204,7 @@ public class CookieTestCase { } private static void assertDecodeSetCookie(final Cookie expected, String toDecode) { - assertThat(Cookie.fromSetCookieHeader(toDecode), containsInAnyOrder(expected)); + assertThat(Cookie.fromSetCookieHeader(toDecode), equalTo(expected)); } private static Cookie newCookie(final String name) { diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java index 027d50317b2..d727be020cb 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java @@ -92,7 +92,7 @@ public class HttpResponseTestCase { response.encodeSetCookieHeader(cookies); final List<String> headers = response.headers().get(HttpHeaders.Names.SET_COOKIE); assertEquals(1, headers.size()); - assertEquals(Cookie.toSetCookieHeaderAll(cookies), headers); + assertEquals(Cookie.toSetCookieHeaders(cookies), headers); } @Test @@ -102,7 +102,7 @@ public class HttpResponseTestCase { response.encodeSetCookieHeader(cookies); final List<String> headers = response.headers().get(HttpHeaders.Names.SET_COOKIE); assertEquals(2, headers.size()); - assertEquals(Cookie.toSetCookieHeaderAll(Arrays.asList(new Cookie("foo", "bar"), new Cookie("baz", "cox"))), + assertEquals(Cookie.toSetCookieHeaders(Arrays.asList(new Cookie("foo", "bar"), new Cookie("baz", "cox"))), headers); } diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapperTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapperTest.java index 3439dc172ee..86ca424c90e 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapperTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapperTest.java @@ -7,7 +7,6 @@ import org.testng.annotations.Test; import java.util.concurrent.TimeUnit; -@SuppressWarnings("deprecation") public class JDiscCookieWrapperTest { @Test @@ -15,19 +14,15 @@ public class JDiscCookieWrapperTest { Cookie cookie = new Cookie("name", "value"); JDiscCookieWrapper wrapper = JDiscCookieWrapper.wrap(cookie); - wrapper.setComment("comment"); wrapper.setDomain("yahoo.com"); wrapper.setMaxAge(10); wrapper.setPath("/path"); - wrapper.setVersion(1); Assert.assertEquals(wrapper.getName(), cookie.getName()); Assert.assertEquals(wrapper.getValue(), cookie.getValue()); Assert.assertEquals(wrapper.getDomain(), cookie.getDomain()); - Assert.assertEquals(wrapper.getComment(), cookie.getComment()); Assert.assertEquals(wrapper.getMaxAge(), cookie.getMaxAge(TimeUnit.SECONDS)); Assert.assertEquals(wrapper.getPath(), cookie.getPath()); - Assert.assertEquals(wrapper.getVersion(), cookie.getVersion()); Assert.assertEquals(wrapper.getSecure(), cookie.isSecure()); } |