Skip to content

Commit

Permalink
Improves handling of cookies in server responses. A cookie's identity…
Browse files Browse the repository at this point in the history
… is defined by its name, domain and path, not just its name. A new method in ServerResponseHeaders enables clearing a cookie by passing all three values (a complete cookie). The equals method in SetCookie has now been overridden as well. Some new tests.
  • Loading branch information
spericas committed Feb 7, 2025
1 parent dec95f3 commit 24a0501
Show file tree
Hide file tree
Showing 7 changed files with 286 additions and 23 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
* Copyright (c) 2022, 2025 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -106,12 +106,24 @@ default ServerResponseHeaders addCookie(String name, String value) {

/**
* Clears a cookie by adding a {@code Set-Cookie} header with an expiration date in the past.
* It is recommended to use {@link #clearCookie(SetCookie)} instead for better handling
* of Path and Domain.
*
* @param name name of the cookie.
* @return this instance
*/
ServerResponseHeaders clearCookie(String name);

/**
* Clears a cookie by adding a {@code Set-Cookie} header with an expiration date in the past.
*
* @param setCookie the cookie.
* @return this instance
*/
default ServerResponseHeaders clearCookie(SetCookie setCookie) {
return clearCookie(setCookie.name());
}

/**
* Sets the value of {@link HeaderNames#LAST_MODIFIED} header.
* <p>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
* Copyright (c) 2022, 2025 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,6 +19,7 @@
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.List;
import java.util.function.Predicate;

import io.helidon.common.LazyValue;

Expand All @@ -39,22 +40,35 @@ public ServerResponseHeaders addCookie(SetCookie cookie) {
return this;
}

@Override
public ServerResponseHeaders clearCookie(SetCookie cookie) {
clearCookie(cookie, cookie::equals);
return this;
}

@Override
public ServerResponseHeaders clearCookie(String name) {
SetCookie expiredCookie = SetCookie.builder(name, "deleted")
.path("/")
clearCookie(SetCookie.builder(name, "").build(), c -> c.name().equals(name));
return this;
}

private void clearCookie(SetCookie cookie, Predicate<SetCookie> predicate) {
// expiredCookie same as cookie but with different expiration
SetCookie expiredCookie = SetCookie.builder(cookie)
.expires(START_OF_YEAR_1970.get())
.build();

// update or add new header?
if (contains(HeaderNames.SET_COOKIE)) {
remove(HeaderNames.SET_COOKIE, it -> {
List<String> currentValues = it.allValues();
String[] newValues = new String[currentValues.size()];
boolean found = false;
for (int i = 0; i < currentValues.size(); i++) {
String currentValue = currentValues.get(i);
if (SetCookie.parse(currentValue).name().equals(name)) {
newValues[i] = expiredCookie.text();
SetCookie currentCookie = SetCookie.parse(currentValue);
if (predicate.test(currentCookie)) {
newValues[i] = expiredCookie.text(); // replace with expired
found = true;
} else {
newValues[i] = currentValue;
Expand All @@ -63,15 +77,13 @@ public ServerResponseHeaders clearCookie(String name) {
if (!found) {
String[] values = new String[newValues.length + 1];
System.arraycopy(newValues, 0, values, 0, newValues.length);
values[values.length - 1] = expiredCookie.text();
values[values.length - 1] = expiredCookie.text(); // replace with expired
newValues = values;
}

set(HeaderValues.create(HeaderNames.SET_COOKIE, newValues));
});
} else {
addCookie(expiredCookie);
}
return this;
}
}
47 changes: 44 additions & 3 deletions http/http/src/main/java/io/helidon/http/SetCookie.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2024 Oracle and/or its affiliates.
* Copyright (c) 2018, 2025 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -67,6 +67,16 @@ public static Builder builder(String name, String value) {
return new Builder(name, value);
}

/**
* Creates a new fluent API builder using another cookie.
*
* @param setCookie the other cookie
* @return a new fluent API builder
*/
public static Builder builder(SetCookie setCookie) {
return new Builder(setCookie);
}

/**
* Parses new instance of {@link SetCookie} from the String representation.
*
Expand All @@ -78,7 +88,7 @@ public static SetCookie parse(String setCookie) {
String nameAndValue = cookieParts[0];
int equalsIndex = nameAndValue.indexOf('=');
String name = nameAndValue.substring(0, equalsIndex);
String value = nameAndValue.length() == equalsIndex ? null : nameAndValue.substring(equalsIndex + 1);
String value = nameAndValue.substring(equalsIndex + 1);
Builder builder = builder(name, value);

for (int i = 1; i < cookieParts.length; i++) {
Expand All @@ -88,7 +98,7 @@ public static SetCookie parse(String setCookie) {
String partValue;
if (equalsIndex > -1) {
partName = cookiePart.substring(0, equalsIndex);
partValue = cookiePart.length() == equalsIndex ? null : cookiePart.substring(equalsIndex + 1);
partValue = cookiePart.substring(equalsIndex + 1);
} else {
partName = cookiePart;
partValue = null;
Expand Down Expand Up @@ -277,6 +287,24 @@ public String toString() {
return result.toString();
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof SetCookie setCookie)) {
return false;
}
return Objects.equals(name, setCookie.name)
&& Objects.equals(domain, setCookie.domain)
&& Objects.equals(path, setCookie.path);
}

@Override
public int hashCode() {
return Objects.hash(name, domain, path);
}

private static void hasNoValue(String partName, String partValue) {
if (partValue != null) {
throw new IllegalArgumentException("Set-Cookie parameter " + partName + " has to have no value!");
Expand Down Expand Up @@ -349,6 +377,19 @@ private Builder(String name, String value) {
this.value = value;
}

private Builder(SetCookie other) {
Objects.requireNonNull(other);
this.name = other.name;
this.value = other.value;
this.expires = other.expires;
this.maxAge = other.maxAge;
this.domain = other.domain;
this.path = other.path;
this.secure = other.secure;
this.httpOnly = other.httpOnly;
this.sameSite = other.sameSite;
}

@Override
public SetCookie build() {
return new SetCookie(this);
Expand Down
65 changes: 54 additions & 11 deletions http/http/src/test/java/io/helidon/http/SetCookieTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2024 Oracle and/or its affiliates.
* Copyright (c) 2020, 2025 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,6 +16,7 @@
package io.helidon.http;

import java.time.Duration;
import java.time.Instant;

import org.junit.jupiter.api.Test;

Expand All @@ -29,17 +30,18 @@
*/
public class SetCookieTest {

private static final String TEMPLATE = "some-cookie=some-cookie-value; "
+ "Expires=Thu, 22 Oct 2015 07:28:00 GMT; "
+ "Max-Age=2592000; "
+ "Domain=domain.value; "
+ "Path=/; "
+ "Secure; "
+ "HttpOnly; "
+ "SameSite=Lax";

@Test
public void testSetCookiesFromString() {
String template = "some-cookie=some-cookie-value; "
+ "Expires=Thu, 22 Oct 2015 07:28:00 GMT; "
+ "Max-Age=2592000; "
+ "Domain=domain.value; "
+ "Path=/; "
+ "Secure; "
+ "HttpOnly; "
+ "SameSite=Lax";
SetCookie setCookie = SetCookie.parse(template);
SetCookie setCookie = SetCookie.parse(TEMPLATE);

assertThat(setCookie.name(), is("some-cookie"));
assertThat(setCookie.value(), is("some-cookie-value"));
Expand All @@ -51,7 +53,7 @@ public void testSetCookiesFromString() {
assertThat(setCookie.httpOnly(), is(true));
assertThat(setCookie.sameSite(), optionalValue(is(SetCookie.SameSite.LAX)));

assertThat("Generate same cookie value", setCookie.toString(), is(template));
assertThat("Generate same cookie value", setCookie.toString(), is(TEMPLATE));
}

@Test
Expand All @@ -63,4 +65,45 @@ public void testSetCookiesInvalidValue() {
assertThat(ex.getMessage(), is("Unexpected Set-Cookie part: Invalid"));
}

@Test
public void testEmptyValue() {
SetCookie setCookie = SetCookie.parse("some-cookie=");
assertThat(setCookie.value(), is(""));
}

@Test
public void testEquals() {
SetCookie setCookie1 = SetCookie.parse(TEMPLATE);
SetCookie setCookie2 = SetCookie.builder("some-cookie", "").build();
SetCookie setCookie3 = SetCookie.builder("some-cookie", "")
.path("/")
.domain("domain.value")
.expires(Instant.now()) // ignored in equals
.build();

assertThat("They match name, path and domain",
setCookie1.equals(setCookie3), is(true));
assertThat("They match name, path and domain",
setCookie1.hashCode(), is(setCookie3.hashCode()));
assertThat("They do not match path or domain",
setCookie1.equals(setCookie2), is(false));
}

@Test
public void testCookieBuilder() {
SetCookie setCookie1 = SetCookie.parse(TEMPLATE);
SetCookie setCookie2 = SetCookie.builder(setCookie1).build(); // from setCookie1

assertThat(setCookie1.equals(setCookie2), is(true));
assertThat(setCookie1.hashCode(), is(setCookie2.hashCode()));
assertThat(setCookie2.name(), is("some-cookie"));
assertThat(setCookie2.value(), is("some-cookie-value"));
assertThat(setCookie2.expires(), optionalValue(is(DateTime.parse("Thu, 22 Oct 2015 07:28:00 GMT"))));
assertThat(setCookie2.maxAge(), optionalValue(is(Duration.ofSeconds(2592000))));
assertThat(setCookie2.domain(), optionalValue(is("domain.value")));
assertThat(setCookie2.path(), optionalValue(is("/")));
assertThat(setCookie2.secure(), is(true));
assertThat(setCookie2.httpOnly(), is(true));
assertThat(setCookie2.sameSite(), optionalValue(is(SetCookie.SameSite.LAX)));
}
}
52 changes: 52 additions & 0 deletions http/tests/cookie/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright (c) 2025 Oracle and/or its affiliates.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="http://maven.apache.org/POM/4.0.0"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>io.helidon.http.tests</groupId>
<artifactId>helidon-http-tests-project</artifactId>
<version>4.2.0-SNAPSHOT</version>
</parent>

<artifactId>helidon-http-tests-cookie</artifactId>
<name>Helidon HTTP Tests Cookie</name>

<dependencies>
<dependency>
<groupId>io.helidon.webserver.testing.junit5</groupId>
<artifactId>helidon-webserver-testing-junit5</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-all</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.helidon.logging</groupId>
<artifactId>helidon-logging-jul</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Loading

0 comments on commit 24a0501

Please sign in to comment.