HTTP: cookie file stores expiration in seconds
A cookie file stores the expiration in seconds since the Linux Epoch,
not in milliseconds. Correct reading and writing cookie files; with
a backwards-compatibility hack to read files that contain a millisecond
timestamp.
Add a test, and fix tests not to rely on the actual current time so
that they will also run successfully after 2030-01-01 noon.
Bug: 571574
Change-Id: If3ba68391e574520701cdee119544eedc42a1ff2
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-simple1.txt b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-simple1.txt
index e06b38c..527893a 100644
--- a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-simple1.txt
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-simple1.txt
@@ -1,2 +1,2 @@
-some-domain1 TRUE /some/path1 FALSE 1893499200000 key1 valueFromSimple1
-some-domain1 TRUE /some/path1 FALSE 1893499200000 key2 valueFromSimple1
\ No newline at end of file
+some-domain1 TRUE /some/path1 FALSE 1893499200 key1 valueFromSimple1
+some-domain1 TRUE /some/path1 FALSE 1893499200 key2 valueFromSimple1
\ No newline at end of file
diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-simple2.txt b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-simple2.txt
index 4bf6723..5ec0606 100644
--- a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-simple2.txt
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-simple2.txt
@@ -1,2 +1,2 @@
-some-domain1 TRUE /some/path1 FALSE 1893499200000 key1 valueFromSimple2
-some-domain1 TRUE /some/path1 FALSE 1893499200000 key3 valueFromSimple2
\ No newline at end of file
+some-domain1 TRUE /some/path1 FALSE 1893499200 key1 valueFromSimple2
+some-domain1 TRUE /some/path1 FALSE 1893499200 key3 valueFromSimple2
\ No newline at end of file
diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-with-empty-and-comment-lines.txt b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-with-empty-and-comment-lines.txt
index a9b8a28..573ee9e 100644
--- a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-with-empty-and-comment-lines.txt
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-with-empty-and-comment-lines.txt
@@ -3,6 +3,6 @@
some-domain1 TRUE /some/path1 FALSE 0 key1 value1
# expires date is 01/01/2030 @ 12:00am (UTC)
-#HttpOnly_.some-domain2 TRUE /some/path2 TRUE 1893499200000 key2 value2
+#HttpOnly_.some-domain2 TRUE /some/path2 TRUE 1893499200 key2 value2
-some-domain3 TRUE /some/path3 FALSE 1893499200000 key3 value3
\ No newline at end of file
+some-domain3 TRUE /some/path3 FALSE 1893499200 key3 value3
\ No newline at end of file
diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-with-milliseconds.txt b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-with-milliseconds.txt
new file mode 100644
index 0000000..940e3b1
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/internal/transport/http/cookies-with-milliseconds.txt
@@ -0,0 +1,2 @@
+some-domain1 TRUE /some/path1 FALSE 1893499200000 key1 valueFromSimple1
+some-domain1 TRUE /some/path1 FALSE 1893499200 key2 valueFromSimple1
\ No newline at end of file
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/http/NetscapeCookieFileTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/http/NetscapeCookieFileTest.java
index 6c8c3ba..b11dd63 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/http/NetscapeCookieFileTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/http/NetscapeCookieFileTest.java
@@ -22,13 +22,13 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
+import java.time.Duration;
import java.time.Instant;
+import java.time.temporal.ChronoUnit;
import java.util.Arrays;
-import java.util.Date;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
-import java.util.regex.Pattern;
import org.eclipse.jgit.internal.storage.file.LockFile;
import org.eclipse.jgit.util.http.HttpCookiesMatcher;
@@ -48,10 +48,14 @@
private URL baseUrl;
/**
- * This is the expiration date that is used in the test cookie files
+ * This is the expiration date that is used in the test cookie files.
*/
- private static long JAN_01_2030_NOON = Instant
- .parse("2030-01-01T12:00:00.000Z").toEpochMilli();
+ private static final Instant TEST_EXPIRY_DATE = Instant
+ .parse("2030-01-01T12:00:00.000Z");
+
+ /** Earlier than TEST_EXPIRY_DATE. */
+ private static final Instant TEST_DATE = TEST_EXPIRY_DATE.minus(180,
+ ChronoUnit.DAYS);
@Before
public void setUp() throws IOException {
@@ -102,14 +106,13 @@
cookie.setPath("/");
cookie.setMaxAge(1000);
cookies.add(cookie);
- Date creationDate = new Date();
try (Writer writer = Files.newBufferedWriter(tmpFile,
StandardCharsets.US_ASCII)) {
- NetscapeCookieFile.write(writer, cookies, baseUrl, creationDate);
+ NetscapeCookieFile.write(writer, cookies, baseUrl, TEST_DATE);
}
String expectedExpiration = String
- .valueOf(creationDate.getTime() + (cookie.getMaxAge() * 1000));
+ .valueOf(TEST_DATE.getEpochSecond() + cookie.getMaxAge());
assertThat(Files.readAllLines(tmpFile, StandardCharsets.US_ASCII),
CoreMatchers
@@ -128,13 +131,12 @@
HttpCookie cookie = new HttpCookie("key2", "value2");
cookie.setMaxAge(1000);
cookies.add(cookie);
- Date creationDate = new Date();
try (Writer writer = Files.newBufferedWriter(tmpFile,
StandardCharsets.US_ASCII)) {
- NetscapeCookieFile.write(writer, cookies, baseUrl, creationDate);
+ NetscapeCookieFile.write(writer, cookies, baseUrl, TEST_DATE);
}
String expectedExpiration = String
- .valueOf(creationDate.getTime() + (cookie.getMaxAge() * 1000));
+ .valueOf(TEST_DATE.getEpochSecond() + cookie.getMaxAge());
assertThat(Files.readAllLines(tmpFile, StandardCharsets.US_ASCII),
CoreMatchers.equalTo(
@@ -161,13 +163,29 @@
}
@Test
+ public void testReadCookieFileWithMilliseconds() throws IOException {
+ try (InputStream input = this.getClass()
+ .getResourceAsStream("cookies-with-milliseconds.txt")) {
+ Files.copy(input, tmpFile, StandardCopyOption.REPLACE_EXISTING);
+ }
+ NetscapeCookieFile cookieFile = new NetscapeCookieFile(tmpFile,
+ TEST_DATE);
+ long expectedMaxAge = Duration.between(TEST_DATE, TEST_EXPIRY_DATE)
+ .getSeconds();
+ for (HttpCookie cookie : cookieFile.getCookies(true)) {
+ assertEquals(expectedMaxAge, cookie.getMaxAge());
+ }
+ }
+
+ @Test
public void testWriteAfterAnotherJgitProcessModifiedTheFile()
throws IOException, InterruptedException {
try (InputStream input = this.getClass()
.getResourceAsStream("cookies-simple1.txt")) {
Files.copy(input, tmpFile, StandardCopyOption.REPLACE_EXISTING);
}
- NetscapeCookieFile cookieFile = new NetscapeCookieFile(tmpFile);
+ NetscapeCookieFile cookieFile = new NetscapeCookieFile(tmpFile,
+ TEST_DATE);
cookieFile.getCookies(true);
// now modify file externally
try (InputStream input = this.getClass()
@@ -177,39 +195,19 @@
// now try to write
cookieFile.write(baseUrl);
- // validate that the external changes are there as well
- // due to rounding errors (conversion from ms to sec to ms)
- // the expiration date might not be exact
List<String> lines = Files.readAllLines(tmpFile,
StandardCharsets.US_ASCII);
assertEquals("Expected 3 lines", 3, lines.size());
- assertStringMatchesPatternWithInexactNumber(lines.get(0),
- "some-domain1\tTRUE\t/some/path1\tFALSE\t(\\d*)\tkey1\tvalueFromSimple2",
- JAN_01_2030_NOON, 1000);
- assertStringMatchesPatternWithInexactNumber(lines.get(1),
- "some-domain1\tTRUE\t/some/path1\tFALSE\t(\\d*)\tkey3\tvalueFromSimple2",
- JAN_01_2030_NOON, 1000);
- assertStringMatchesPatternWithInexactNumber(lines.get(2),
- "some-domain1\tTRUE\t/some/path1\tFALSE\t(\\d*)\tkey2\tvalueFromSimple1",
- JAN_01_2030_NOON, 1000);
- }
-
- @SuppressWarnings("boxing")
- private static final void assertStringMatchesPatternWithInexactNumber(
- String string, String pattern, long expectedNumericValue,
- long delta) {
- java.util.regex.Matcher matcher = Pattern.compile(pattern)
- .matcher(string);
- assertTrue("Given string '" + string + "' does not match '" + pattern
- + "'", matcher.matches());
- // extract numeric value
- Long actualNumericValue = Long.decode(matcher.group(1));
-
- assertTrue(
- "Value is supposed to be close to " + expectedNumericValue
- + " but is " + actualNumericValue + ".",
- Math.abs(expectedNumericValue - actualNumericValue) <= delta);
+ assertEquals(
+ "some-domain1\tTRUE\t/some/path1\tFALSE\t1893499200\tkey1\tvalueFromSimple2",
+ lines.get(0));
+ assertEquals(
+ "some-domain1\tTRUE\t/some/path1\tFALSE\t1893499200\tkey3\tvalueFromSimple2",
+ lines.get(1));
+ assertEquals(
+ "some-domain1\tTRUE\t/some/path1\tFALSE\t1893499200\tkey2\tvalueFromSimple1",
+ lines.get(2));
}
@Test
@@ -229,14 +227,13 @@
cookie.setHttpOnly(true);
cookies.add(cookie);
- Date creationDate = new Date();
-
try (Writer writer = Files.newBufferedWriter(tmpFile,
StandardCharsets.US_ASCII)) {
- NetscapeCookieFile.write(writer, cookies, baseUrl, creationDate);
+ NetscapeCookieFile.write(writer, cookies, baseUrl, TEST_DATE);
}
Set<HttpCookie> actualCookies = new NetscapeCookieFile(tmpFile,
- creationDate).getCookies(true);
+ TEST_DATE)
+ .getCookies(true);
assertThat(actualCookies, HttpCookiesMatcher.containsInOrder(cookies));
}
@@ -246,15 +243,12 @@
.getResourceAsStream("cookies-simple1.txt")) {
Files.copy(input, tmpFile, StandardCopyOption.REPLACE_EXISTING);
}
- // round up to the next second (to prevent rounding errors)
- Date creationDate = new Date(
- (System.currentTimeMillis() / 1000) * 1000);
- Set<HttpCookie> cookies = new NetscapeCookieFile(tmpFile, creationDate)
+ Set<HttpCookie> cookies = new NetscapeCookieFile(tmpFile, TEST_DATE)
.getCookies(true);
Path tmpFile2 = folder.newFile().toPath();
try (Writer writer = Files.newBufferedWriter(tmpFile2,
StandardCharsets.US_ASCII)) {
- NetscapeCookieFile.write(writer, cookies, baseUrl, creationDate);
+ NetscapeCookieFile.write(writer, cookies, baseUrl, TEST_DATE);
}
// compare original file with newly written one, they should not differ
assertEquals(Files.readAllLines(tmpFile), Files.readAllLines(tmpFile2));
@@ -267,13 +261,13 @@
Files.copy(input, tmpFile, StandardCopyOption.REPLACE_EXISTING);
}
- Date creationDate = new Date();
Set<HttpCookie> cookies = new LinkedHashSet<>();
HttpCookie cookie = new HttpCookie("key2", "value2");
cookie.setDomain("some-domain2");
cookie.setPath("/some/path2");
- cookie.setMaxAge((JAN_01_2030_NOON - creationDate.getTime()) / 1000);
+ cookie.setMaxAge(
+ Duration.between(TEST_DATE, TEST_EXPIRY_DATE).getSeconds());
cookie.setSecure(true);
cookie.setHttpOnly(true);
cookies.add(cookie);
@@ -281,11 +275,12 @@
cookie = new HttpCookie("key3", "value3");
cookie.setDomain("some-domain3");
cookie.setPath("/some/path3");
- cookie.setMaxAge((JAN_01_2030_NOON - creationDate.getTime()) / 1000);
+ cookie.setMaxAge(
+ Duration.between(TEST_DATE, TEST_EXPIRY_DATE).getSeconds());
cookies.add(cookie);
- Set<HttpCookie> actualCookies = new NetscapeCookieFile(tmpFile, creationDate)
- .getCookies(true);
+ Set<HttpCookie> actualCookies = new NetscapeCookieFile(tmpFile,
+ TEST_DATE).getCookies(true);
assertThat(actualCookies, HttpCookiesMatcher.containsInOrder(cookies));
}
@@ -296,7 +291,7 @@
Files.copy(input, tmpFile, StandardCopyOption.REPLACE_EXISTING);
}
- new NetscapeCookieFile(tmpFile)
- .getCookies(true);
+ assertTrue(new NetscapeCookieFile(tmpFile, TEST_DATE).getCookies(true)
+ .isEmpty());
}
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/http/NetscapeCookieFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/http/NetscapeCookieFile.java
index 49f26c7..dae7173 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/http/NetscapeCookieFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/http/NetscapeCookieFile.java
@@ -22,11 +22,12 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.text.MessageFormat;
+import java.time.Instant;
import java.util.Arrays;
import java.util.Collection;
-import java.util.Date;
import java.util.LinkedHashSet;
import java.util.Set;
+import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.annotations.Nullable;
@@ -53,6 +54,7 @@
* In general this class is not thread-safe. So any consumer needs to take care
* of synchronization!
*
+ * @see <a href="https://curl.se/docs/http-cookies.html">Cookie file format</a>
* @see <a href="http://www.cookiecentral.com/faq/#3.5">Netscape Cookie File
* Format</a>
* @see <a href=
@@ -92,7 +94,7 @@
private byte[] hash;
- final Date creationDate;
+ private final Instant createdAt;
private Set<HttpCookie> cookies = null;
@@ -104,13 +106,13 @@
* where to find the cookie file
*/
public NetscapeCookieFile(Path path) {
- this(path, new Date());
+ this(path, Instant.now());
}
- NetscapeCookieFile(Path path, Date creationDate) {
+ NetscapeCookieFile(Path path, Instant createdAt) {
this.path = path;
this.snapshot = FileSnapshot.DIRTY;
- this.creationDate = creationDate;
+ this.createdAt = createdAt;
}
/**
@@ -142,7 +144,7 @@
if (cookies == null || refresh) {
try {
byte[] in = getFileContentIfModified();
- Set<HttpCookie> newCookies = parseCookieFile(in, creationDate);
+ Set<HttpCookie> newCookies = parseCookieFile(in, createdAt);
if (cookies != null) {
cookies = mergeCookies(newCookies, cookies);
} else {
@@ -168,9 +170,9 @@
*
* @param input
* the file content to parse
- * @param creationDate
- * the date for the creation of the cookies (used to calculate
- * the maxAge based on the expiration date given within the file)
+ * @param createdAt
+ * cookie creation time; used to calculate the maxAge based on
+ * the expiration date given within the file
* @return the set of parsed cookies from the given file (even expired
* ones). If there is more than one cookie with the same name in
* this file the last one overwrites the first one!
@@ -180,7 +182,7 @@
* if the given file does not have a proper format
*/
private static Set<HttpCookie> parseCookieFile(@NonNull byte[] input,
- @NonNull Date creationDate)
+ @NonNull Instant createdAt)
throws IOException, IllegalArgumentException {
String decoded = RawParseUtils.decode(StandardCharsets.US_ASCII, input);
@@ -190,7 +192,7 @@
new StringReader(decoded))) {
String line;
while ((line = reader.readLine()) != null) {
- HttpCookie cookie = parseLine(line, creationDate);
+ HttpCookie cookie = parseLine(line, createdAt);
if (cookie != null) {
cookies.add(cookie);
}
@@ -200,7 +202,7 @@
}
private static HttpCookie parseLine(@NonNull String line,
- @NonNull Date creationDate) {
+ @NonNull Instant createdAt) {
if (line.isEmpty() || (line.startsWith("#") //$NON-NLS-1$
&& !line.startsWith(HTTP_ONLY_PREAMBLE))) {
return null;
@@ -236,7 +238,12 @@
cookie.setSecure(Boolean.parseBoolean(cookieLineParts[3]));
long expires = Long.parseLong(cookieLineParts[4]);
- long maxAge = (expires - creationDate.getTime()) / 1000;
+ // Older versions stored milliseconds. This heuristic to detect that
+ // will cause trouble in the year 33658. :-)
+ if (cookieLineParts[4].length() == 13) {
+ expires = TimeUnit.MILLISECONDS.toSeconds(expires);
+ }
+ long maxAge = expires - createdAt.getEpochSecond();
if (maxAge <= 0) {
return null; // skip expired cookies
}
@@ -245,7 +252,7 @@
}
/**
- * Read the underying file and return its content but only in case it has
+ * Read the underlying file and return its content but only in case it has
* been modified since the last access.
* <p>
* Internally calculates the hash and maintains {@link FileSnapshot}s to
@@ -333,7 +340,7 @@
path);
// reread new changes if necessary
Set<HttpCookie> cookiesFromFile = NetscapeCookieFile
- .parseCookieFile(cookieFileContent, creationDate);
+ .parseCookieFile(cookieFileContent, createdAt);
this.cookies = mergeCookies(cookiesFromFile, cookies);
}
} catch (FileNotFoundException e) {
@@ -343,7 +350,7 @@
ByteArrayOutputStream output = new ByteArrayOutputStream();
try (Writer writer = new OutputStreamWriter(output,
StandardCharsets.US_ASCII)) {
- write(writer, cookies, url, creationDate);
+ write(writer, cookies, url, createdAt);
}
LockFile lockFile = new LockFile(path.toFile());
for (int retryCount = 0; retryCount < LOCK_ACQUIRE_MAX_RETRY_COUNT; retryCount++) {
@@ -377,24 +384,23 @@
* @param url
* the url for which to write the cookie (to derive the default
* values for certain cookie attributes)
- * @param creationDate
- * the date when the cookie has been created. Important for
- * calculation the cookie expiration time (calculated from
- * cookie's maxAge and this creation time)
+ * @param createdAt
+ * cookie creation time; used to calculate a cookie's expiration
+ * time
* @throws IOException
* if an I/O error occurs
*/
static void write(@NonNull Writer writer,
@NonNull Collection<HttpCookie> cookies, @NonNull URL url,
- @NonNull Date creationDate) throws IOException {
+ @NonNull Instant createdAt) throws IOException {
for (HttpCookie cookie : cookies) {
- writeCookie(writer, cookie, url, creationDate);
+ writeCookie(writer, cookie, url, createdAt);
}
}
private static void writeCookie(@NonNull Writer writer,
@NonNull HttpCookie cookie, @NonNull URL url,
- @NonNull Date creationDate) throws IOException {
+ @NonNull Instant createdAt) throws IOException {
if (cookie.getMaxAge() <= 0) {
return; // skip expired cookies
}
@@ -422,7 +428,7 @@
final String expirationDate;
// whenCreated field is not accessible in HttpCookie
expirationDate = String
- .valueOf(creationDate.getTime() + (cookie.getMaxAge() * 1000));
+ .valueOf(createdAt.getEpochSecond() + cookie.getMaxAge());
writer.write(expirationDate);
writer.write(COLUMN_SEPARATOR);
writer.write(cookie.getName());
