From 67fd9dd1d9863a7856925eda5bc2811b2d1dd9ac Mon Sep 17 00:00:00 2001 From: Veniamin Fernandes Date: Mon, 23 Jan 2023 22:58:42 +0200 Subject: [PATCH 1/4] Add CountDownLatch-es into CPPluginConnectionImplTest Wait for all threads to start and pause on the CountDownLatch. Then send start signal to all threads and call the method under test. This makes invocation more concurrent and forces all threads to start at the same place. Signed-off-by: Veniamin Fernandes --- .../fcp/FCPPluginConnectionImplTest.java | 78 ++++++++++++------- 1 file changed, 51 insertions(+), 27 deletions(-) diff --git a/test/freenet/clients/fcp/FCPPluginConnectionImplTest.java b/test/freenet/clients/fcp/FCPPluginConnectionImplTest.java index e8b42a979a8..a1d12781995 100644 --- a/test/freenet/clients/fcp/FCPPluginConnectionImplTest.java +++ b/test/freenet/clients/fcp/FCPPluginConnectionImplTest.java @@ -6,6 +6,7 @@ import static org.junit.Assert.*; import java.io.IOException; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -30,7 +31,7 @@ public final class FCPPluginConnectionImplTest { * checking whether it is empty after all send threads have terminated.
*/ @Test - public final void testSendSynchronousThreadSafety() throws InterruptedException { + public void testSendSynchronousThreadSafety() throws InterruptedException { // JUnit ignores failures in threads other than the threads which it runs tests from. // Thus we pass failures out with this boolean. // NOTICE: We also use JUnit assert*() / fail() even though they won't work in threads @@ -45,31 +46,37 @@ public final void testSendSynchronousThreadSafety() throws InterruptedException // This is by design: Plugins are supposed to be unloadable and the FCPPluginConnectionImpl // must not keep them pinned in memory after unload. final ServerSideFCPMessageHandler server = new ServerSideFCPMessageHandler() { - @Override public FCPPluginMessage handlePluginFCPMessage( - final FCPPluginConnection connection, final FCPPluginMessage message) { - - final FCPPluginMessage reply = FCPPluginMessage.constructSuccessReply(message); - reply.params.putSingle("replyToThread", message.params.get("thread")); - return reply; - } - }; - + @Override + public FCPPluginMessage handlePluginFCPMessage( + final FCPPluginConnection connection, + final FCPPluginMessage message + ) { + final FCPPluginMessage reply = FCPPluginMessage.constructSuccessReply(message); + reply.params.putSingle("replyToThread", message.params.get("thread")); + return reply; + } + }; + final ClientSideFCPMessageHandler client = new ClientSideFCPMessageHandler() { - @Override public FCPPluginMessage handlePluginFCPMessage( - final FCPPluginConnection connection, final FCPPluginMessage message) { - - failure.set(true); - fail("This test is about sendSynchronous() so the reply messages should not " - + "hit the client message handler"); - throw new UnsupportedOperationException(); - } - }; + @Override + public FCPPluginMessage handlePluginFCPMessage( + final FCPPluginConnection connection, + final FCPPluginMessage message + ) { + failure.set(true); + fail( + "This test is about sendSynchronous() so the reply messages should not hit the client message handler" + ); + throw new UnsupportedOperationException(); + } + }; - final FCPPluginConnectionImpl connection = FCPPluginConnectionImpl.constructForUnitTest( - server, client); + final FCPPluginConnectionImpl connection = FCPPluginConnectionImpl.constructForUnitTest(server, client); final int threadCount = 100; final Thread[] threads = new Thread[threadCount]; + final CountDownLatch allThreadsStarted = new CountDownLatch(threadCount); + final CountDownLatch concurrentStart = new CountDownLatch(1); for(int i=0; i < threadCount; ++i) { final String threadIndex = Integer.toString(i); @@ -83,6 +90,7 @@ public final void testSendSynchronousThreadSafety() throws InterruptedException @Override public void run() { try { + awaitConcurrentStart(); final FCPPluginMessage reply = connection.sendSynchronous( SendDirection.ToServer, message, TimeUnit.SECONDS.toNanos(10)); @@ -98,6 +106,15 @@ public final void testSendSynchronousThreadSafety() throws InterruptedException fail("InterruptedException " + e); } } + + private void awaitConcurrentStart() throws InterruptedException { + allThreadsStarted.countDown(); + boolean waitSuccess = concurrentStart.await(1, TimeUnit.MINUTES); + if (!waitSuccess) { + failure.set(true); + fail("Start timeout in thread " + threadIndex); + } + } }); threads[i] = thread; @@ -106,14 +123,21 @@ public final void testSendSynchronousThreadSafety() throws InterruptedException // Start them in a separate loop, not in the loop where we construct them, to ensure that // they are all started at the same time, execute in parallel, and thus have maximal // probability of race conditions. - for(int i=0; i < threadCount; ++i) + for (int i = 0; i < threadCount; ++i) { threads[i].start(); - - for(int i=0; i < threadCount; ++i) + } + + boolean waitSuccess = allThreadsStarted.await(1, TimeUnit.MINUTES); + if (!waitSuccess) { + fail("Start timeout in the main test method"); + } + concurrentStart.countDown(); + + for (int i = 0; i < threadCount; ++i) { threads[i].join(); - - assertEquals("JUnit failures cannot be passed out of threads, please check stdout/stderr.", - false, failure.get()); + } + + assertFalse("JUnit failures cannot be passed out of threads, please check stdout/stderr.", failure.get()); assertEquals("FCPPluginConnectionImpl sendSynchronous() map should not leak", 0, connection.getSendSynchronousCount()); From 83fc04fba1e34ef694edf3a874665bca5a9bf707 Mon Sep 17 00:00:00 2001 From: Veniamin Fernandes Date: Sat, 11 Feb 2023 18:32:02 +0200 Subject: [PATCH 2/4] improve tests in freenet.clients package Adds improvements to assertions and code formatting Signed-off-by: Veniamin Fernandes --- .../fcp/FCPPluginMessageEncodeDecodeTest.java | 22 ++++++++----------- .../clients/http/FilterCSSIdentifierTest.java | 2 +- .../utils/UriFilterProxyHeaderParserTest.java | 19 ++++++++++------ 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/test/freenet/clients/fcp/FCPPluginMessageEncodeDecodeTest.java b/test/freenet/clients/fcp/FCPPluginMessageEncodeDecodeTest.java index 6714dce2cb5..6618e812405 100644 --- a/test/freenet/clients/fcp/FCPPluginMessageEncodeDecodeTest.java +++ b/test/freenet/clients/fcp/FCPPluginMessageEncodeDecodeTest.java @@ -5,12 +5,11 @@ import static org.junit.Assert.*; -import java.io.IOException; import java.util.ArrayList; +import java.util.List; import org.junit.Test; -import freenet.node.FSParseException; import freenet.support.SimpleFieldSet; /** @@ -37,8 +36,8 @@ public final class FCPPluginMessageEncodeDecodeTest { * decoding is then tested using {@link #testEncodeDecode(FCPPluginMessage)}. */ @Test - public final void testEncodeDecode() throws MessageInvalidException, IOException, FSParseException { - ArrayList messages = new ArrayList(); + public void testEncodeDecode() throws MessageInvalidException { + List messages = new ArrayList<>(); // Non-reply messages. Can either have a SimpleFieldSet, or a Bucket, or both. We don't use // Buckets because the parsing code for those is higher level code, so we only have to use @@ -87,11 +86,9 @@ public final void testEncodeDecode() throws MessageInvalidException, IOException /** * @see FCPPluginMessageEncodeDecodeTest Explained at class-level JavaDoc of this class. */ - private final void testEncodeDecode(FCPPluginMessage message) - throws MessageInvalidException, IOException, FSParseException { + private void testEncodeDecode(FCPPluginMessage message) throws MessageInvalidException { - SimpleFieldSet encodedMessage - = new FCPPluginServerMessage("testPlugin", message).getFieldSet(); + SimpleFieldSet encodedMessage = new FCPPluginServerMessage("testPlugin", message).getFieldSet(); // The params have a different prefix in FCPPluginServerMessage and FCPPluginClientMessage. // So we have to rename them by removing the sub-SimpleFieldSet with the old prefix and @@ -102,24 +99,23 @@ private final void testEncodeDecode(FCPPluginMessage message) encodedMessage.put(FCPPluginClientMessage.PARAM_PREFIX, params); } - FCPPluginMessage decodedMessage - = new FCPPluginClientMessage(encodedMessage).constructFCPPluginMessage(); + FCPPluginMessage decodedMessage = new FCPPluginClientMessage(encodedMessage).constructFCPPluginMessage(); // Permissions are set by the FCPPluginConnectionImpl when the message is actually delivered - assertEquals(null, decodedMessage.permissions); + assertNull(decodedMessage.permissions); assertEquals(message.identifier, decodedMessage.identifier); // SimpleFieldSet offers no equals(). But its designed to be human readable, so we can // just encode them into Strings and compare those. if(message.params == null) { - assertEquals(null, decodedMessage.params); + assertNull(decodedMessage.params); } else { assertEquals(message.params.toOrderedString(), decodedMessage.params.toOrderedString()); } if(message.data == null) { - assertEquals(null, decodedMessage.data); + assertNull(decodedMessage.data); } else { // Not implemented yet because the parsing of the data is higher level FCP code; i.e. // it is not implemented in FCPPluginClientMessage, but one of its parent classes. diff --git a/test/freenet/clients/http/FilterCSSIdentifierTest.java b/test/freenet/clients/http/FilterCSSIdentifierTest.java index 2370fb064b1..10f3a12bd49 100644 --- a/test/freenet/clients/http/FilterCSSIdentifierTest.java +++ b/test/freenet/clients/http/FilterCSSIdentifierTest.java @@ -14,7 +14,7 @@ public class FilterCSSIdentifierTest { @Test public void testKnownValid() { - String identifiers[] = { "sample_key-1", "-_", "-k_d", "_testing-key" }; + String[] identifiers = { "sample_key-1", "-_", "-k_d", "_testing-key" }; for (String identifier : identifiers) { assertEquals(identifier, PageMaker.filterCSSIdentifier(identifier)); diff --git a/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java b/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java index bac35e4ca44..0532ef9bd87 100644 --- a/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java +++ b/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java @@ -1,5 +1,6 @@ package freenet.clients.http.utils; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import org.junit.Test; @@ -281,15 +282,16 @@ private void testUriPrefixMatchesExpected( String uriScheme, String uriHost, MultiValueTable headers, - String resultUriPrefix) throws Exception { + String resultUriPrefix + ) throws Exception { String schemeHostAndPort = UriFilterProxyHeaderParser.parse( fakePortOption(fProxyPort), fakeBindToOption(fProxyBindTo), uriScheme, uriHost, - headers) - .toString(); - assertTrue( + headers + ).toString(); + assertEquals( String.format( "schemeHostAndPort %s does not match expected %s; portConfig=\"%s\", bindTo=\"%s\", uriScheme=\"%s\", uriHost=\"%s\", headers=%s, expected=\"%s\"", schemeHostAndPort, @@ -299,8 +301,11 @@ private void testUriPrefixMatchesExpected( uriScheme, uriHost, headers, - resultUriPrefix), - schemeHostAndPort.equals(resultUriPrefix)); + resultUriPrefix + ), + schemeHostAndPort, + resultUriPrefix + ); } @@ -336,7 +341,7 @@ private StringOption fakePortOption(String value) return option; } - private class DummyStringCallback extends StringCallback { + private static class DummyStringCallback extends StringCallback { @Override public String get() { From 8b10e3972b6952b99757cdf0f712c68e9c54be49 Mon Sep 17 00:00:00 2001 From: Veniamin Fernandes Date: Sat, 11 Feb 2023 18:42:35 +0200 Subject: [PATCH 3/4] improve HTTP date and time parsing in TimeUtil Remove commented and broken parsing. Add date and time parsing, which can parse various timestamps defined in multiple HTTP standards. Main algorithm is defined in https://www.ietf.org/rfc/rfc6265.html#section-5.1.1 Also use newer classes from java.time package. Signed-off-by: Veniamin Fernandes --- src/freenet/client/filter/HTMLFilter.java | 17 +- .../clients/http/ImageCreatorToadlet.java | 9 +- src/freenet/support/TimeUtil.java | 260 +++++++++++++++++- test/freenet/support/TimeUtilTest.java | 175 +++++++++--- 4 files changed, 391 insertions(+), 70 deletions(-) diff --git a/src/freenet/client/filter/HTMLFilter.java b/src/freenet/client/filter/HTMLFilter.java index 1372b571cd2..88f83570b16 100644 --- a/src/freenet/client/filter/HTMLFilter.java +++ b/src/freenet/client/filter/HTMLFilter.java @@ -17,7 +17,7 @@ import java.io.Writer; import java.nio.charset.Charset; import java.nio.charset.MalformedInputException; -import java.text.ParseException; +import java.time.format.DateTimeParseException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -31,14 +31,9 @@ import java.util.Stack; import java.util.StringTokenizer; -import freenet.clients.http.ToadletContextImpl; import freenet.l10n.NodeL10n; -import freenet.support.HTMLDecoder; -import freenet.support.HTMLEncoder; -import freenet.support.Logger; +import freenet.support.*; import freenet.support.Logger.LogLevel; -import freenet.support.URLDecoder; -import freenet.support.URLEncodedFormatException; import freenet.support.io.NullWriter; public class HTMLFilter implements ContentDataFilter, CharsetExtractor { @@ -3008,13 +3003,13 @@ Map sanitizeHash(Map h, } else if ((http_equiv != null) && (name == null)) { if (http_equiv.equalsIgnoreCase("Expires")) { try { - ToadletContextImpl.parseHTTPDate(content); - hn.put("http-equiv", http_equiv); - hn.put("content", content); - } catch (ParseException e) { + TimeUtil.parseHttpDateTime(content); + } catch (DateTimeParseException e) { // Delete it. return null; } + hn.put("http-equiv", http_equiv); + hn.put("content", content); } else if ( http_equiv.equalsIgnoreCase("Content-Script-Type")) { // We don't support script at this time. diff --git a/src/freenet/clients/http/ImageCreatorToadlet.java b/src/freenet/clients/http/ImageCreatorToadlet.java index 92ccc7b3bdc..1897d32de39 100644 --- a/src/freenet/clients/http/ImageCreatorToadlet.java +++ b/src/freenet/clients/http/ImageCreatorToadlet.java @@ -10,12 +10,14 @@ import java.io.OutputStream; import java.net.URI; import java.sql.Date; -import java.text.ParseException; +import java.time.ZonedDateTime; +import java.time.format.DateTimeParseException; import javax.imageio.ImageIO; import freenet.client.HighLevelSimpleClient; import freenet.support.MultiValueTable; +import freenet.support.TimeUtil; import freenet.support.api.Bucket; import freenet.support.api.HTTPRequest; @@ -49,12 +51,13 @@ public void handleMethodGET(URI uri, HTTPRequest req, ToadletContext ctx) throws if (ctx.getHeaders().containsKey("if-modified-since")) { try { // If the received date is equal to the last modification of this class, then it doesn't need regeneration - if (ToadletContextImpl.parseHTTPDate(ctx.getHeaders().get("if-modified-since")).compareTo(LAST_MODIFIED) == 0) { + ZonedDateTime zonedDateTime = TimeUtil.parseHttpDateTime(ctx.getHeaders().get("if-modified-since")); + if (zonedDateTime.toInstant().equals(LAST_MODIFIED.toInstant())) { // So we just send the NOT_MODIFIED response, and skip the generation ctx.sendReplyHeadersStatic(304, "Not Modified", null, "image/png", 0, LAST_MODIFIED); needsGeneration = false; } - } catch (ParseException pe) { + } catch (DateTimeParseException pe) { // If something goes wrong, we regenerate } } diff --git a/src/freenet/support/TimeUtil.java b/src/freenet/support/TimeUtil.java index 92695950369..1c0a64fb9e2 100644 --- a/src/freenet/support/TimeUtil.java +++ b/src/freenet/support/TimeUtil.java @@ -26,7 +26,14 @@ the License, or (at your option) any later version. import java.text.DecimalFormat; import java.text.SimpleDateFormat; +import java.time.*; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeFormatterBuilder; +import java.time.format.DateTimeParseException; +import java.time.temporal.ChronoField; import java.util.*; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * Time formatting utility. @@ -36,7 +43,75 @@ public class TimeUtil { public static final TimeZone TZ_UTC = TimeZone.getTimeZone("UTC"); - /** + // https://www.ietf.org/rfc/rfc6265.html#section-5.1.1 + private static final Pattern DATE_DELIMETERS = Pattern.compile("[\\x09\\x20-\\x2F\\x3B-\\x40\\x5B-\\x60\\x7B-\\x7E]+"); + private static final Pattern TIME_PATTERN = Pattern.compile("(\\d{1,2}):(\\d{1,2}):(\\d{1,2})"); + private static final Pattern DAY_OF_MONTH_PATTERN = Pattern.compile("\\d{1,2}"); + private static final Pattern YEAR_PATTERN = Pattern.compile("\\d{2,4}"); + + private static final List DATE_TIME_FORMATTERS = Collections.unmodifiableList( + Arrays.asList( + DateTimeFormatter.RFC_1123_DATE_TIME, + // Same as RFC1123 but with timezone name + DateTimeFormatter.ofPattern("EEE, dd MMM yyyy HH:mm:ss zzz"), + + // Monday, 07 Nov 1994 08:49:37 GMT + DateTimeFormatter.ofPattern("EEEE, dd MMM yyyy HH:mm:ss zzz"), + + // Sunday, 06-Nov-94 08:49:37 GMT ; RFC 850, obsoleted by RFC 1036 + // Current century two digit years + DateTimeFormatter.ofPattern("EEEE, dd'-'MMM'-'uu HH:mm:ss zzz"), + + // two digit years between 1900 - 2000 + new DateTimeFormatterBuilder() + .appendPattern("EEEE, dd'-'MMM'-'") + .appendValueReduced(ChronoField.YEAR, 2, 2, 1900) + .appendPattern(" HH:mm:ss zzz") + .toFormatter(), + + DateTimeFormatter.ISO_DATE_TIME, + DateTimeFormatter.ISO_LOCAL_DATE_TIME, + DateTimeFormatter.ISO_ZONED_DATE_TIME, + DateTimeFormatter.ISO_OFFSET_DATE, + DateTimeFormatter.ISO_INSTANT, + DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"), + DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:sszzz") + + ) + ); + + private static final List MONTHS = Collections.unmodifiableList( + Arrays.asList("jan", "feb", "mar", "apr", "may", "jun", "jul", "aug", "sep", "oct", "nov", "dec") + ); + + private static final List MONTH_FORMATTERS = Collections.unmodifiableList( + Arrays.asList( + DateTimeFormatter.ofPattern("MMMM", Locale.US), + DateTimeFormatter.ofPattern("MMMM", Locale.getDefault()) + ) + ); + + private static final List DAY_OF_WEEK_FORMATTER = Collections.unmodifiableList( + Arrays.asList( + DateTimeFormatter.ofPattern("EEE", Locale.US), + DateTimeFormatter.ofPattern("EEE", Locale.getDefault()), + DateTimeFormatter.ofPattern("EEEE", Locale.US), + DateTimeFormatter.ofPattern("EEEE", Locale.getDefault()) + ) + ); + + private static final List TZ_FORMATTERS = Collections.unmodifiableList( + Arrays.asList( + new DateTimeFormatterBuilder().appendOffset("+HH:mm", "Z").toFormatter(), + new DateTimeFormatterBuilder().appendOffset("+HHmm", "Z").toFormatter(), + new DateTimeFormatterBuilder().appendOffset("+HH:MM:ss", "Z").toFormatter(), + new DateTimeFormatterBuilder().appendOffset("+HHMMss", "Z").toFormatter(), + new DateTimeFormatterBuilder().appendZoneOrOffsetId().toFormatter() + ) + ); + + + /** * It converts a given time interval into a * week/day/hour/second.milliseconds string. * @param timeInterval interval to convert, millis @@ -187,17 +262,178 @@ public static String makeHTTPDate(long time) { sdf.setTimeZone(TZ_UTC); return sdf.format(new Date(time)); } - -// FIXME: For me it returns a parsed time with 2 hours difference, so it seems to parse localtime. WHY? - -// public static Date parseHTTPDate(String date) throws ParseException { -// SimpleDateFormat sdf = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss 'GMT'",Locale.US); -// sdf.setTimeZone(TZ_UTC); -// return sdf.parse(date); -// } - - - /** + + /** + * Parses text datetime defined in multiple standards (RFC822, RFC1036, RFC1123, ISO8601) + * + * @param dateTimeText datetime text, must have textual month or be an ISO8601 timestamp. + * @return date time with time zone + * @throws DateTimeParseException if cannot parse datetime + */ + public static ZonedDateTime parseHttpDateTime(String dateTimeText) { + for (DateTimeFormatter df : DATE_TIME_FORMATTERS) { + try { + return ZonedDateTime.parse(dateTimeText, df); + } catch (DateTimeParseException ignored) { + // continue with next pattern + } + try { + return LocalDateTime.parse(dateTimeText, df).atZone(ZoneOffset.UTC); + } catch (DateTimeParseException ignored) { + // continue with next pattern + } + } + + boolean foundTime = false; + boolean foundDayOfWeek = false; + boolean foundDayOfMonth = false; + boolean foundMonth = false; + boolean foundYear = false; + + int hour = -1; + int min = -1; + int second = -1; + + ZoneId zoneId = ZoneOffset.UTC; + + int dayOfWeek = -1; + int dayOfMonth = -1; + int month = -1; + int year = -1; + + // This algorithm is defined in https://www.ietf.org/rfc/rfc6265.html#section-5.1.1 + // It is extended with timezone parsing + + String[] tokens = DATE_DELIMETERS.split(dateTimeText); + for (String token : tokens) { + Matcher timeMatcher = TIME_PATTERN.matcher(token); + if (!foundTime && timeMatcher.matches()) { + hour = Integer.parseInt(timeMatcher.group(1)); + min = Integer.parseInt(timeMatcher.group(2)); + second = Integer.parseInt(timeMatcher.group(3)); + foundTime = true; + continue; + } + if (!foundDayOfMonth && DAY_OF_MONTH_PATTERN.matcher(token).matches()) { + dayOfMonth = Integer.parseInt(token); + foundDayOfMonth = true; + continue; + } + if (!foundMonth) { + month = parseMonth(token); + if (month > 0) { + foundMonth = true; + continue; + } + } + if (!foundYear && YEAR_PATTERN.matcher(token).matches()) { + year = Integer.parseInt(token); + foundYear = true; + } + + if (!foundDayOfWeek) { + dayOfWeek = parseDayOfWeek(token); + if (dayOfWeek > 0) { + foundDayOfWeek = true; + } + } + + ZoneId newZoneId = parseZoneId(dateTimeText, foundTime, foundYear, token); + if (newZoneId != null) { + zoneId = newZoneId; + } + } + + if (foundTime && foundDayOfMonth && foundMonth && foundYear) { + if (year >= 70 && year <= 99) { + year += 1900; + } + if (year >= 0 && year <= 69) { + year += 2000; + } + ZonedDateTime dateTime; + try { + dateTime = ZonedDateTime.of( + year, + month, + dayOfMonth, + hour, + min, + second, + 0, + zoneId + ); + } catch (DateTimeException dte) { + throw new DateTimeParseException("Cannot parse datetime", dateTimeText, 0, dte); + } + if (foundDayOfWeek && dayOfWeek != dateTime.getDayOfWeek().getValue()) { + throw new DateTimeParseException("Invalid day of week", dateTimeText, -0); + } + return dateTime; + } + + throw new DateTimeParseException("Cannot parse datetime", dateTimeText, 0); + } + + private static int parseMonth(String token) { + int month = MONTHS.indexOf(token.toLowerCase(Locale.ROOT)) + 1; + if (month > 0) { + return month; + } + for (DateTimeFormatter monthFormatter : MONTH_FORMATTERS) { + try { + return monthFormatter.parse(token).get(ChronoField.MONTH_OF_YEAR); + } catch (DateTimeParseException ignored) { + } + } + return -1; + } + + private static int parseDayOfWeek(String token) { + for (DateTimeFormatter dateTimeFormatter : DAY_OF_WEEK_FORMATTER) { + try { + return dateTimeFormatter.parse(token).get(ChronoField.DAY_OF_WEEK); + } catch (DateTimeParseException ignored) { + } + } + return -1; + } + + private static ZoneId parseZoneId(String dateTimeText, boolean foundTime, boolean foundYear, String token) { + for (DateTimeFormatter tzFormatter : TZ_FORMATTERS) { + try { + return ZoneId.from(tzFormatter.parse(token)); + } catch (DateTimeException ignored) { + } + } + try { + return ZoneId.of(token); + } catch (DateTimeException ignored2) { + } + try { + return ZoneId.of(token, ZoneId.SHORT_IDS); + } catch (DateTimeException ignored3) { + } + if (foundYear && foundTime && token.matches("(\\d{6})|(\\d{4})|(\\d{2}:\\d{2}(:\\d{2})?)")) { + int idx = dateTimeText.indexOf(token); + if (idx > 0) { + char c = dateTimeText.charAt(idx - 1); + if (c == '+' || c == '-') { + String offset = c + token; + for (DateTimeFormatter tzFormatter : TZ_FORMATTERS) { + try { + return ZoneId.from(tzFormatter.parse(offset)); + } catch (DateTimeException ignored) { + } + } + } + } + } + return null; + } + + + /** * @return Returns the passed date with the same year/month/day but with the time set to 00:00:00.000 */ public static Date setTimeToZero(final Date date) { diff --git a/test/freenet/support/TimeUtilTest.java b/test/freenet/support/TimeUtilTest.java index a5febf1fee1..f05a8a2d403 100644 --- a/test/freenet/support/TimeUtilTest.java +++ b/test/freenet/support/TimeUtilTest.java @@ -19,10 +19,13 @@ import static org.junit.Assert.*; -import java.util.Date; -import java.util.GregorianCalendar; -import java.util.Locale; -import java.util.TimeZone; +import java.text.ParseException; +import java.time.Duration; +import java.time.ZoneId; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.time.format.DateTimeParseException; +import java.util.*; import org.junit.Before; import org.junit.Test; @@ -35,7 +38,7 @@ public class TimeUtilTest { //1w+1d+1h+1m+1s+1ms - private long oneForTermLong = 694861001; + private final long oneForTermLong = 694861001; @Before public void setUp() throws Exception { @@ -49,8 +52,10 @@ public void setUp() throws Exception { @Test public void testFormatTime_LongIntBoolean_MaxValue() { String expectedForMaxLongValue = "15250284452w3d7h12m55.807s"; - assertEquals(TimeUtil.formatTime(Long.MAX_VALUE,6,true), - expectedForMaxLongValue); + assertEquals( + expectedForMaxLongValue, + TimeUtil.formatTime(Long.MAX_VALUE,6,true) + ); } /** @@ -60,8 +65,10 @@ public void testFormatTime_LongIntBoolean_MaxValue() { @Test public void testFormatTime_LongInt() { String expectedForMaxLongValue = "15250284452w3d7h12m55s"; - assertEquals(TimeUtil.formatTime(Long.MAX_VALUE,6), - expectedForMaxLongValue); + assertEquals( + expectedForMaxLongValue, + TimeUtil.formatTime(Long.MAX_VALUE,6) + ); } /** @@ -72,8 +79,10 @@ public void testFormatTime_LongInt() { public void testFormatTime_Long() { //it uses two terms by default String expectedForMaxLongValue = "15250284452w3d"; - assertEquals(TimeUtil.formatTime(Long.MAX_VALUE), - expectedForMaxLongValue); + assertEquals( + expectedForMaxLongValue, + TimeUtil.formatTime(Long.MAX_VALUE) + ); } /** @@ -84,7 +93,6 @@ public void testFormatTime_Long() { */ @Test public void testFormatTime_KnownValues() { - Long methodLong; String[][] valAndExpected = { //one week {"604800000","1w"}, @@ -97,10 +105,12 @@ public void testFormatTime_KnownValues() { //one second {"1000","1s"} }; - for(int i = 0; i < valAndExpected.length; i++) { - methodLong = Long.valueOf(valAndExpected[i][0]); - assertEquals(TimeUtil.formatTime(methodLong.longValue()), - valAndExpected[i][1]); } + for (String[] pair : valAndExpected) { + assertEquals( + pair[1], + TimeUtil.formatTime(Long.parseLong(pair[0])) + ); + } } /** @@ -112,24 +122,27 @@ public void testFormatTime_KnownValues() { @Test public void testFormatTime_LongIntBoolean_maxTerms() { String[] valAndExpected = { - //0 terms - "", - //1 term - "1w", - //2 terms - "1w1d", - //3 terms - "1w1d1h", - //4 terms - "1w1d1h1m", - //5 terms - "1w1d1h1m1s", - //6 terms - "1w1d1h1m1.001s" + //0 terms + "", + //1 term + "1w", + //2 terms + "1w1d", + //3 terms + "1w1d1h", + //4 terms + "1w1d1h1m", + //5 terms + "1w1d1h1m1s", + //6 terms + "1w1d1h1m1.001s" }; - for(int i = 0; i < valAndExpected.length; i++) - assertEquals(TimeUtil.formatTime(oneForTermLong,i,true), - valAndExpected[i]); + for (int maxTerms = 0; maxTerms < valAndExpected.length; maxTerms++) { + assertEquals( + valAndExpected[maxTerms], + TimeUtil.formatTime(oneForTermLong, maxTerms, true) + ); + } } /** @@ -141,8 +154,8 @@ public void testFormatTime_LongIntBoolean_maxTerms() { @Test public void testFormatTime_LongIntBoolean_milliseconds() { long methodValue = 1; //1ms - assertEquals(TimeUtil.formatTime(methodValue,6,false),"0s"); - assertEquals(TimeUtil.formatTime(methodValue,6,true),"0.001s"); + assertEquals("0s", TimeUtil.formatTime(methodValue,6,false)); + assertEquals("0.001s", TimeUtil.formatTime(methodValue,6,true)); } /** @@ -153,11 +166,11 @@ public void testFormatTime_LongIntBoolean_milliseconds() { */ @Test public void testFormatTime_LongIntBoolean_tooManyTerms() { - try { - TimeUtil.formatTime(oneForTermLong,7); - fail("Expected IllegalArgumentException not thrown"); } - catch (IllegalArgumentException anException) { - assertNotNull(anException); } + assertThrows( + "Expected exception was not thrown for invalid maxTerms parameter", + IllegalArgumentException.class, + () -> TimeUtil.formatTime(oneForTermLong,7) + ); } /** Tests {@link TimeUtil#setTimeToZero(Date)} */ @@ -218,10 +231,84 @@ public void testToMillis_empty() { @Test public void testToMillis_unknownFormat() { - try { - TimeUtil.toMillis("15250284452w3q7h12m55.807s"); - } catch (NumberFormatException e) { - assertNotNull(e); + assertThrows( + "Expected exception was not thrown for invalid time interval parameter", + NumberFormatException.class, + () -> TimeUtil.toMillis("15250284452w3q7h12m55.807s") + ); + } + + @Test + public void parseHttpDateTime() { + Object[][] dateTimeSamples = { + {"Sun, 06 Nov 1994 08:49:37 GMT", ZonedDateTime.of(1994, 11, 6, 8, 49, 37, 0, ZoneOffset.UTC)}, + {"Sun, 06 Nov 1994 08:49:37 UTC", ZonedDateTime.of(1994, 11, 6, 8, 49, 37, 0, ZoneOffset.UTC)}, + {"Sun, 06 Nov 1994 08:49:37 ABC", ZonedDateTime.of(1994, 11, 6, 8, 49, 37, 0, ZoneOffset.UTC)}, + {"Some text: Sun, 06 Nov 1994 08:49:37 ", ZonedDateTime.of(1994, 11, 6, 8, 49, 37, 0, ZoneOffset.UTC)}, + {"Sun, 06 Nov 1994 08:49:37", ZonedDateTime.of(1994, 11, 6, 8, 49, 37, 0, ZoneOffset.UTC)}, + {"Sun, 1994 Nov 6 08:49:37 GMT", ZonedDateTime.of(1994, 11, 6, 8, 49, 37, 0, ZoneOffset.UTC)}, + {"Sunday, 06-Nov-94 08:49:37 GMT", ZonedDateTime.of(1994, 11, 6, 8, 49, 37, 0, ZoneOffset.UTC)}, + {"Monday, 30-Jan-23 08:49:37 GMT", ZonedDateTime.of(2023, 1, 30, 8, 49, 37, 0, ZoneOffset.UTC)}, + {"Monday, 07 Nov 1994 08:49:37 GMT", ZonedDateTime.of(1994, 11, 7, 8, 49, 37, 0, ZoneOffset.UTC)}, + {"Tue Nov 8 08:49:37 1994", ZonedDateTime.of(1994, 11, 8, 8, 49, 37, 0, ZoneOffset.UTC)}, + {"Wed, 21.Oct.2015 07:28:00 GMT", ZonedDateTime.of(2015, 10, 21, 7, 28, 0, 0, ZoneOffset.UTC)}, + {"Thu, 01 Jan 1970 00:00:00 UTC", ZonedDateTime.of(1970, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC)}, + {"Fri, 15-Jan-2021 22:23:01 GMT", ZonedDateTime.of(2021, 1, 15, 22, 23, 1, 0, ZoneOffset.UTC)}, + {"Saturday, 15/January/2022 09:55:01 PST", ZonedDateTime.of(2022, 1, 15, 9, 55, 1, 0, ZoneId.of("PST", ZoneId.SHORT_IDS))}, + {"Sunday, 22 August 99 06:30:07", ZonedDateTime.of(1999, 8, 22, 6, 30, 7, 0, ZoneOffset.UTC)}, + {"2008-08-08T08:08:08", ZonedDateTime.of(2008, 8, 8, 8, 8, 8, 0, ZoneOffset.UTC)}, + {"2009-09-09T09:09:09+00:00", ZonedDateTime.of(2009, 9, 9, 9, 9, 9, 0, ZoneOffset.UTC)}, + {"2010-10-10T10:10:10.719922211-04:00", ZonedDateTime.of(2010, 10, 10, 10, 10, 10, 719922211, ZoneOffset.ofHours(-4))}, + {"2010-10-10T10:10:10.719922211", ZonedDateTime.of(2010, 10, 10, 10, 10, 10, 719922211, ZoneOffset.UTC)}, + {"2011-11-11T11:11:11.123+02:30", ZonedDateTime.of(2011, 11, 11, 11, 11, 11, 123000000, ZoneOffset.ofHoursMinutes(2, 30))}, + {"2011-12-03T10:15:30Z", ZonedDateTime.of(2011, 12, 3, 10, 15, 30, 0, ZoneOffset.UTC)}, + {"2011-12-03 10:15:30Z", ZonedDateTime.of(2011, 12, 3, 10, 15, 30, 0, ZoneOffset.UTC)}, + {"2011-12-03 10:15:30", ZonedDateTime.of(2011, 12, 3, 10, 15, 30, 0, ZoneOffset.UTC)}, + {"2011-12-03 10:15:30+01:00", ZonedDateTime.of(2011, 12, 3, 10, 15, 30, 0, ZoneOffset.ofHours(1))}, + {"2011-12-03 10:15:30-03:00", ZonedDateTime.of(2011, 12, 3, 10, 15, 30, 0, ZoneOffset.ofHours(-3))}, + {"Fri, 2013-DEC-13 13:13:13 GMT", ZonedDateTime.of(2013, 12, 13, 13, 13, 13, 0, ZoneOffset.UTC)}, + {"Tue Apr 12 09:45:14 2016", ZonedDateTime.of(2016, 4, 12, 9, 45, 14, 0, ZoneOffset.UTC)}, + {"Tue Aug 19 1975 23:15:30 GMT+0100 (Western European Summer Time)", ZonedDateTime.of(1975, 8, 19, 23, 15, 30, 0, ZoneOffset.ofHours(1))}, + {"Tue Aug 19 1975 23:15:30 +0100 (Western European Summer Time)", ZonedDateTime.of(1975, 8, 19, 23, 15, 30, 0, ZoneOffset.ofHours(1))}, + {"Tue Aug 19 +1975 23:15:30 +0100 (Western European Summer Time)", ZonedDateTime.of(1975, 8, 19, 23, 15, 30, 0, ZoneOffset.ofHours(1))}, + {"Tue Aug-19 -1975 23:15:30 -0100 (Western European Summer Time)", ZonedDateTime.of(1975, 8, 19, 23, 15, 30, 0, ZoneOffset.ofHours(-1))}, + {"Tue Aug-19-1975 23:15:30 -0100 (Western European Summer Time)", ZonedDateTime.of(1975, 8, 19, 23, 15, 30, 0, ZoneOffset.ofHours(-1))}, + {"Tue+Aug+19+1975+23:15:30+GMT+01:00 (Western European Summer Time)", ZonedDateTime.of(1975, 8, 19, 23, 15, 30, 0, ZoneOffset.ofHours(1))}, + {"Tue+Aug+19+1975+23:15:30+GMT-01:00 (Western European Summer Time)", ZonedDateTime.of(1975, 8, 19, 23, 15, 30, 0, ZoneOffset.ofHours(-1))}, + {"Tue Aug-19-1975 23:15:30 GMT+0100 (Western European Summer Time)", ZonedDateTime.of(1975, 8, 19, 23, 15, 30, 0, ZoneOffset.ofHours(1))}, + {"Tue Aug-19-1975 23:15:30 GMT+023015 (Western European Summer Time)", ZonedDateTime.of(1975, 8, 19, 23, 15, 30, 0, ZoneOffset.ofHoursMinutesSeconds(2, 30, 15))}, + {"Tue Aug-19-1975 23:15:30 GMT+02:30:15 (Western European Summer Time)", ZonedDateTime.of(1975, 8, 19, 23, 15, 30, 0, ZoneOffset.ofHoursMinutesSeconds(2, 30, 15))}, + {"Wednesday, Aug 20 1975 12:00:01 GMT+08:15", ZonedDateTime.of(1975, 8, 20, 12, 0, 1, 0, ZoneOffset.ofHoursMinutes(8, 15))} + }; + + for (Object[] sample: dateTimeSamples) { + String dateTimeText = (String) sample[0]; + ZonedDateTime validDateTime = (ZonedDateTime) sample[1]; + assertEquals( + "Result is not equal for input datetime text: '" + dateTimeText + "'", + validDateTime.toInstant(), + TimeUtil.parseHttpDateTime(dateTimeText).toInstant() + ); + } + + List invalidDateTimeSamples = Arrays.asList( + // invalid day + "Saturday, 22 August 99 06:30:07", + // cannot detect month or day + "Sun, 1994 11 06 Nov 08:49:37 GMT", + "08/22/2006 06:30", + "08/22/2006 06:30 AM", + "08/22/2006 6:30", + "08/22/2006 08:49:37", + "2006 08 22 08:49:37", + "30 Feb 1970 00:00:00 UTC" + ); + for (String dateTimeText: invalidDateTimeSamples) { + assertThrows( + "Parse method should throw exception for invalid input datetime text: '" + dateTimeText + "'", + DateTimeParseException.class, + () -> TimeUtil.parseHttpDateTime(dateTimeText) + ); } } } From c88622380da72a8765ae4a7e845de3335d910bf4 Mon Sep 17 00:00:00 2001 From: Veniamin Fernandes Date: Sat, 11 Feb 2023 19:07:43 +0200 Subject: [PATCH 4/4] improve Cookie And ReceivedCookie classes Improved unit tests for Cookie And ReceivedCookie classes. Split ReceivedCookie class out of Cookie class, and removed unused parts. Set-Cookie2 and Cookie2 format is no longer used in modern browsers. Newer specification RFC6225 defines received cookie as key value pairs only. ReceivedCookie uses only cookie name and value, and others parts were not used and are removed now. Signed-off-by: Veniamin Fernandes --- src/freenet/clients/http/Cookie.java | 620 +++++++------- src/freenet/clients/http/ReceivedCookie.java | 451 ++++------ src/freenet/clients/http/SessionManager.java | 782 +++++++++--------- .../clients/http/ToadletContextImpl.java | 211 +++-- test/freenet/clients/http/CookieTest.java | 371 +++++++-- .../clients/http/ReceivedCookieTest.java | 139 +++- 6 files changed, 1368 insertions(+), 1206 deletions(-) diff --git a/src/freenet/clients/http/Cookie.java b/src/freenet/clients/http/Cookie.java index 02c25d22c85..484c689fc56 100644 --- a/src/freenet/clients/http/Cookie.java +++ b/src/freenet/clients/http/Cookie.java @@ -5,327 +5,315 @@ import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.Charset; -import java.util.Arrays; -import java.util.Date; -import java.util.HashSet; +import java.time.Instant; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.util.*; -import freenet.support.CurrentTimeUTC; -import freenet.support.TimeUtil; /** * @author xor (xor@freenetproject.org) */ public class Cookie { - /** - * FIXME: Where was this taken from? - */ - private static final HashSet invalidValueCharacters = - new HashSet(Arrays.asList(new Character[] { '(', ')', '[', ']', '{', '}', '=', ',', '\"', '/', '\\', '?', '@', ':', ';' })); - - /** - * Taken from this discussion: - CTL = (octets 0 - 31) and DEL (127)> - CHAR = - token = 1* - separators = "(" | ")" | "<" | ">" | "@" | "," | ";" | ":" | "\" | <"> | "/" | "[" | "]" | "?" | "=" | "{" | "}" | SP | HT - so, anything from 32-126 that isn't in that list of seperators is valid. - TheSeeker: where did you copy that from? - http://www.ietf.org/rfc/rfc2616.txt - */ - public static final HashSet httpSeparatorCharacters = - new HashSet(Arrays.asList(new Character[] { '(', ')', '<', '>', '@', ',', ';', ':', '\\', '\"', '/', '[', ']', '?', '=', '{', '}', ' ', '\t' })); - - private static final Charset usasciiCharset = Charset.forName("US-ASCII"); - - - protected int version; - - protected URI domain; - - protected URI path; - - protected String name; - - protected String value; - - protected Date expirationDate; - - protected boolean discard; - - - /** - * Constructor for creating cookies which are to be used in {@link ToadletContext.setCookie}. - * - * @param myPath The path of the cookie. Must be absolute. - * @param myName The name of the cookie. Must be latin letters and numbers only and is converted to lowercase. - * @param myValue The value of the cookie. Must not contain any linebreaks. May be null, null means empty value. - * @throws URISyntaxException - */ - public Cookie(URI myPath, String myName, String myValue, Date myExpirationDate) { - version = 1; - - domain = null; - path = validatePath(myPath); - name = validateName(myName); - value = myValue != null ? validateValue(myValue) : ""; - expirationDate = validateExpirationDate(myExpirationDate); - discard = true; // Freenet cookies shall always be discarded when the browser is closed; - } - - protected Cookie() { - - } - - /** - * Returns true if two Cookies have equal domain, path and name. Does not check the value! - */ - @Override - public boolean equals(Object obj) { - if(obj == this) - return true; - - if(!(obj instanceof Cookie)) - return false; - - Cookie other = (Cookie)obj; - - // RFC2965: Two cookies are equal if name and domain are equal with case-insensitive comparison and path is equal with case-sensitive comparison. - // We don't have to do anything about the case here though because getName() / getDomain() returns lowercase and getPath() returns the original path. - - URI myDomain = getDomain(); URI otherDomain = other.getDomain(); - - if(myDomain != null) { - if(otherDomain == null || !otherDomain.toString().equals(myDomain.toString())) - return false; - } else if(otherDomain != null) - return false; - - - if(!getPath().toString().equals(other.getPath().toString())) - return false; - - if(!getName().equals(other.getName())) - return false; - - return true; - } - - @Override - public int hashCode() { - return domain.hashCode() + path.hashCode() + name.hashCode(); - } - - public static URI validateDomain(String domainString) throws URISyntaxException { - return validateDomain(new URI(domainString.toLowerCase())); - } - - public static URI validateDomain(URI domain) { - String scheme = domain.getScheme().toLowerCase(); - - if(!"http".equals(scheme) && !"https".equals(scheme)) - throw new IllegalArgumentException("Illegal cookie domain, must be http or https: " + domain); - - String path = domain.getPath(); - - if(!"".equals(path) && !"/".equals(path)) - throw new IllegalArgumentException("Illegal cookie domain, contains a path: " + domain); - - return domain; - } - - public static URI validatePath(String stringPath) throws URISyntaxException { - return validatePath(new URI(stringPath)); - } - - public static URI validatePath(URI path) { - // FIXME: Be more restrictive. - - if(path.isAbsolute()) - throw new IllegalArgumentException("Illegal cookie path, must be relative: " + path); - - if(path.toString().startsWith("/") == false) - throw new IllegalArgumentException("Illegal cookie path, must start with /: " + path); - - // RFC2965: Path is case sensitive! - - return path; - } - - - /** - * Validates the name of a cookie against a mixture of RFC2965, RFC2616 (from IETF.org) and personal choice :| - * The personal choice is mostly that it uses isISOControl for determining control characters instead of the - * list in the RFC - therefore, more characters are considered as control characters. - * So effectively this function is more restrictive than the RFCs. - * TODO: Read the RFCs in depth and make this function fully compatible. - */ - public static String validateName(String name) { - if("".equals(name)) - throw new IllegalArgumentException("Name is empty."); - - if(!isUSASCII(name)) - throw new IllegalArgumentException("Invalid name, contains non-US-ASCII characters: " + name); - - name = name.trim().toLowerCase(); // RFC2965: Name is case insensitive - - /* - CTL = (octets 0 - 31) and DEL (127)> - CHAR = - token = 1* - separators = "(" | ")" | "<" | ">" | "@" | "," | ";" | ":" | "\" | <"> | "/" | "[" | "]" | "?" | "=" | "{" | "}" | SP | HT - so, anything from 32-126 that isn't in that list of seperators is valid. - TheSeeker: where did you copy that from? - http://www.ietf.org/rfc/rfc2616.txt - The following grammar uses the notation, and tokens DIGIT (decimal digits), token (informally, a sequence of non-special, non-white space characters), and http_URL from the HTTP/1.1 specification [RFC2616] to describe their syntax. - quoted-string = ( <"> *(qdtext | quoted-pair ) <"> ) - qdtext = > - TEXT = - LWS = [CRLF] 1*( SP | HT ) - OCTET = - so, if it's quoted, it can be anything 32-126 and 128-255 (except a quote char, obviously) - */ - - for(Character c : name.toCharArray()) { - if(Character.isWhitespace(c)) - throw new IllegalArgumentException("Invalid name, contains whitespace: " + name); - - // From isISOControl javadoc: A character is considered to be an ISO control character if its in the range [0,31] or [127,159] - if(Character.isISOControl(c)) - throw new IllegalArgumentException("Invalid name, contains control characters."); - - if(httpSeparatorCharacters.contains(c)) - throw new IllegalArgumentException("Invalid name, contains one of the explicitely disallowed characters: " + name); - } - - if( name.startsWith("$") - || "comment".equals(name) - || "discard".equals(name) - || "domain".equals(name) - || "expires".equals(name) - || "max-age".equals(name) - || "path".equals(name) - || "secure".equals(name) - || "version".equals(name) - ) - throw new IllegalArgumentException("Name is reserved: " + name); - - return name; - } - - private static boolean isUSASCII(String name) { - for(int i=0;i 126) - return false; - } - return true; - } - - /** - * Validates the value of a cookie against a mixture of RFC2965, RFC2616 (from IETF.org) and personal choice :| - * The personal choice is mostly that it uses isISOControl for determining control characters instead of the - * list in the RFC - therefore, more characters are considered as control characters. - * So effectively this function is more restrictive than the RFCs. - * TODO: Read the RFCs in depth and make this function fully compatible. - */ - public static String validateValue(String value) { - if(!isUSASCII(value)) - throw new IllegalArgumentException("Invalid value, contains non-US-ASCII characters: " + value); - - value = value.trim(); - - /* - CTL = (octets 0 - 31) and DEL (127)> - CHAR = - token = 1* - separators = "(" | ")" | "<" | ">" | "@" | "," | ";" | ":" | "\" | <"> | "/" | "[" | "]" | "?" | "=" | "{" | "}" | SP | HT - so, anything from 32-126 that isn't in that list of seperators is valid. - TheSeeker: where did you copy that from? - http://www.ietf.org/rfc/rfc2616.txt - The following grammar uses the notation, and tokens DIGIT (decimal digits), token (informally, a sequence of non-special, non-white space characters), and http_URL from the HTTP/1.1 specification [RFC2616] to describe their syntax. - quoted-string = ( <"> *(qdtext | quoted-pair ) <"> ) - qdtext = > - TEXT = - LWS = [CRLF] 1*( SP | HT ) - OCTET = - so, if it's quoted, it can be anything 32-126 and 128-255 (except a quote char, obviously) - */ - - for(Character c : value.toCharArray()) { - // We allow whitespace in the value because quotation is allowed and supported by the parser in ReceivedCookie - - if(Character.isISOControl(c)) - throw new IllegalArgumentException("Invalid value, contains control characters."); - - // TODO: The source of the invalid value characters list is not mentioned in its javadoc - it has to be re-validated - if(invalidValueCharacters.contains(c)) - throw new IllegalArgumentException("Invalid value, contains one of the explicitely disallowed characters: " + value); - } - - return value; - } - - public static Date validateExpirationDate(Date expirationDate) { - if(CurrentTimeUTC.get().after(expirationDate)) - throw new IllegalArgumentException("Illegal expiration date, is in past: " + expirationDate); - - return expirationDate; - } - - public URI getDomain() { - return domain; - } - - public URI getPath() { - return path; - } - - public String getName() { - return name; - } - - public String getValue() { - return value; - } - -// TODO: This is broken in ReceivedCookie so it is also commented out here. See ReceivedCookie for the explanation. -// public Date getExpirationDate() { -// return expirationDate; -// } - - - /** - * Encodes the content of this cookie to the HTTP header value representation as of RFC2965. - * Does not quote any of the values as Firefox 4 does not support quotation and the latest IETF draft for cookies does not contain - * quotation: http://tools.ietf.org/html/draft-ietf-httpstate-cookie-23 - * The key "set-cookie2:" is not prefixed to the returned String! - */ - protected String encodeToHeaderValue() { - StringBuilder sb = new StringBuilder(512); // TODO: As soon as something else besides Freetalk uses cookies, adjust this value. - - // RFC2965: Name MUST be first. - sb.append(name); sb.append("="); sb.append(value); sb.append(';'); - - sb.append("version="); sb.append(version); sb.append(';'); - - if(domain != null) { - sb.append("domain="); sb.append(domain); sb.append(';'); - } - - sb.append("path="); sb.append(path); sb.append(';'); - - sb.append("expires="); sb.append(TimeUtil.makeHTTPDate(expirationDate.getTime())); sb.append(';'); - - if(discard) { - sb.append("discard="); sb.append(discard); sb.append(';'); - } - - return sb.toString(); - } + /** + * Cookie token separators from https://www.ietf.org/rfc/rfc2616.html#section-2.2 + */ + static final Set COOKIE_TOKEN_SEPARATOR_CHARACTERS = Collections.unmodifiableSet( + new HashSet<>( + Arrays.asList( + '(', ')', '<', '>', '@', ',', ';', ':', '\\', '\"', '/', '[', ']', '?', '=', '{', '}', ' ', '\t' + ) + ) + ); + + /** + * Cookie value forbidden chars excluding ASCII control characters and space. + * From https://www.ietf.org/rfc/rfc6265.html#section-4.1.1 + */ + static final Set COOKIE_VALUE_FORBIDDEN_CHARS = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList('\"', ',', ';', '\\')) + ); + + /** + * Reserved cookie attribute names as for https://www.ietf.org/rfc/rfc2965.html#section-3.2 + * (in lower case) + */ + private static final Set RESERVED_ATTRIBUTE_NAMES = Collections.unmodifiableSet( + new HashSet<>( + Arrays.asList( + "comment", "commenturl", "discard", "domain", "expires", "max-age", "path", "port", "secure", "version" + ) + ) + ); + + + protected int version; + + protected URI domain; + + protected URI path; + + protected String name; + + protected String value; + + protected Instant expirationTime; + + protected boolean discard; + + + /** + * Constructor for creating cookies which are to be used in {@link ToadletContext.setCookie}. + * + * @param myPath The path of the cookie. Must be absolute. + * @param myName The name of the cookie. Must be latin letters and numbers only and is converted to lowercase. + * @param myValue The value of the cookie. Must not contain any linebreaks. May be null, null means empty value. + */ + public Cookie(URI myPath, String myName, String myValue, Instant myExpirationTime) { + version = 1; + domain = null; + path = validatePath(myPath); + name = validateName(myName); + value = myValue != null ? validateValue(myValue) : ""; + expirationTime = validateExpirationDate(myExpirationTime); + discard = true; // Freenet cookies shall always be discarded when the browser is closed; + } + + /** + * Returns true if two Cookies have equal domain, path and name. Does not check the value! + */ + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + + if (!(obj instanceof Cookie)) { + return false; + } + + Cookie other = (Cookie) obj; + + // RFC2965: Two cookies are equal if name and domain are equal with case-insensitive comparison and path is equal with case-sensitive comparison. + // We don't have to do anything about the case here though because getName() / getDomain() returns lowercase and getPath() returns the original path. + + URI myDomain = getDomain(); + URI otherDomain = other.getDomain(); + + if (myDomain != null) { + if (otherDomain == null || !otherDomain.toString().equals(myDomain.toString())) { + return false; + } + } else if (otherDomain != null) { + return false; + } + + if (!getPath().toString().equals(other.getPath().toString())) { + return false; + } + + return getName().equals(other.getName()); + } + + @Override + public int hashCode() { + return Objects.hash(this.domain, this.path, this.name); + } + + protected static URI validateDomain(String domainString) throws URISyntaxException { + return validateDomain(new URI(domainString.toLowerCase())); + } + + protected static URI validateDomain(URI domain) { + String scheme = domain.getScheme().toLowerCase(); + + if (!"http".equals(scheme) && !"https".equals(scheme)) { + throw new IllegalArgumentException("Illegal cookie domain, must be http or https: " + domain); + } + + String path = domain.getPath(); + + if (!"".equals(path) && !"/".equals(path)) { + throw new IllegalArgumentException("Illegal cookie domain, contains a path: " + domain); + } + + return domain; + } + + protected static URI validatePath(URI path) { + // FIXME: Be more restrictive. + + if (path == null) { + throw new IllegalArgumentException("Illegal cookie path, must be present but actual value is null"); + } + + if (path.isAbsolute()) { + throw new IllegalArgumentException("Illegal cookie path, must be relative: " + path); + } + + if (!path.toString().startsWith("/")) { + throw new IllegalArgumentException("Illegal cookie path, must start with /: " + path); + } + + // RFC2965: Path is case sensitive! + + return path; + } + + + /** + * Validates the name of a cookie against a mixture of RFC2965, RFC2616 and RFC6265 (from IETF.org) + * and personal choice :| + * The personal choice is mostly that it uses isISOControl for determining control characters instead of the + * list in the RFC - therefore, more characters are considered as control characters. + * So effectively this function is more restrictive than the RFCs. + * TODO: Read the RFCs in depth and make this function fully compatible. + */ + protected static String validateName(String name) { + if (name == null) { + throw new IllegalArgumentException("Name is null."); + } + + if (name.isEmpty()) { + throw new IllegalArgumentException("Name is empty."); + } + + if (!isUSASCII(name)) { + throw new IllegalArgumentException("Invalid name, contains non-US-ASCII characters: " + name); + } + + name = name.trim().toLowerCase(); // RFC2965: Name is case-insensitive + + for (Character c : name.toCharArray()) { + if (Character.isWhitespace(c)) { + throw new IllegalArgumentException("Invalid name, contains whitespace: " + name); + } + + // From isISOControl javadoc: A character is considered to be an ISO control character if its in the range [0,31] or [127,159] + if (Character.isISOControl(c)) { + throw new IllegalArgumentException("Invalid name, contains control characters."); + } + + if (COOKIE_TOKEN_SEPARATOR_CHARACTERS.contains(c)) { + throw new IllegalArgumentException("Invalid name, contains one of the explicitely disallowed characters: " + name); + } + } + + if (name.startsWith("$") || RESERVED_ATTRIBUTE_NAMES.contains(name)) { + throw new IllegalArgumentException("Name is reserved: " + name); + } + + return name; + } + + private static boolean isUSASCII(String name) { + for (int i = 0; i < name.length(); i++) { + char c = name.charAt(i); + // Java chars are unicode. Unicode is a superset of US-ASCII. + if (c < 32 || c > 126) { + return false; + } + } + return true; + } + + /** + * Validates the value of a cookie against a mixture of RFC2965, RFC2616 and RFC6265 (from IETF.org) + * and personal choice :| + * The personal choice is mostly that it uses isISOControl for determining control characters instead of the + * list in the RFC - therefore, more characters are considered as control characters. + * So effectively this function is more restrictive than the RFCs. + * TODO: Read the RFCs in depth and make this function fully compatible. + */ + protected static String validateValue(String value) { + if (!isUSASCII(value)) { + throw new IllegalArgumentException("Invalid value, contains non-US-ASCII characters: " + value); + } + + value = value.trim(); + + for (Character c : value.toCharArray()) { + // We allow whitespace in the value because quotation is allowed and supported by the parser in ReceivedCookie + + if (Character.isISOControl(c)) { + throw new IllegalArgumentException("Invalid value, contains control characters."); + } + + if (COOKIE_VALUE_FORBIDDEN_CHARS.contains(c)) { + throw new IllegalArgumentException("Invalid value, contains one of the explicitly disallowed characters: " + value); + } + } + + return value; + } + + protected static Instant validateExpirationDate(Instant expirationTime) { + if (expirationTime == null) { + throw new IllegalArgumentException("Illegal expiration time, value is null"); + } + return expirationTime; + } + + public URI getDomain() { + return domain; + } + + public URI getPath() { + return path; + } + + public String getName() { + return name; + } + + public String getValue() { + return value; + } + + public Instant getExpirationTime() { + return expirationTime; + } + + /** + * Encodes the content of this cookie to the HTTP header value representation as of RFC2965. + * Does not quote any of the values as Firefox 4 does not support quotation and the latest IETF draft for cookies does not contain + * quotation: http://tools.ietf.org/html/draft-ietf-httpstate-cookie-23 + * The key "set-cookie2:" is not prefixed to the returned String! + */ + protected String encodeToHeaderValue() { + StringBuilder sb = new StringBuilder(512); // TODO: As soon as something else besides Freetalk uses cookies, adjust this value. + + // RFC2965: Name MUST be first. + sb.append(name); + sb.append("="); + sb.append(value); + sb.append(';'); + + sb.append("version="); + sb.append(version); + sb.append(';'); + + if (domain != null) { + sb.append("domain="); + sb.append(domain); + sb.append(';'); + } + + sb.append("path="); + sb.append(path); + sb.append(';'); + + sb.append("expires="); + sb.append(DateTimeFormatter.RFC_1123_DATE_TIME.format(expirationTime.atOffset(ZoneOffset.UTC))); + sb.append(';'); + + if (discard) { + sb.append("discard="); + sb.append(discard); + sb.append(';'); + } + + return sb.toString(); + } } diff --git a/src/freenet/clients/http/ReceivedCookie.java b/src/freenet/clients/http/ReceivedCookie.java index 1749c621e44..1b914cdaa9c 100644 --- a/src/freenet/clients/http/ReceivedCookie.java +++ b/src/freenet/clients/http/ReceivedCookie.java @@ -3,286 +3,199 @@ * http://www.gnu.org/ for further details of the GPL. */ package freenet.clients.http; -import java.net.URI; -import java.net.URISyntaxException; import java.text.ParseException; import java.util.ArrayList; -import java.util.Hashtable; - -import freenet.support.Logger; +import java.util.Collections; +import java.util.List; +import java.util.Objects; /** * A cookie which the server has received from the client. - * - * This class is not thread-safe! - * + * * @author xor (xor@freenetproject.org) */ -public final class ReceivedCookie extends Cookie { - - private static volatile boolean logMINOR; - private static volatile boolean logDEBUG; - - static { - Logger.registerClass(ReceivedCookie.class); - } - - private String notValidatedName; - - private Hashtable content; - - - /** - * Constructor for creating cookies from parsed key-value pairs. - * - * Does not validate the names or values of the keys, each attribute is validated at the first call to it's getter method. - * Therefore, no CPU time is wasted if the client sends cookies which we do not use. - */ - private ReceivedCookie(String myName, Hashtable myContent) { - // We do not validate the input here, we only parse it if someone actually tries to access this cookie. - notValidatedName = myName; - content = myContent; - - // We do NOT parse the version even though RFC2965 requires it because Firefox (3.0.14) does not give us a version. - - version = 1; - //version = Integer.parseInt(content.get("$version")); - - //if(version != 1) - // throw new IllegalArgumentException("Invalid version: " + version); - } - - /** - * Parses the value of a "Cookie:" HTTP header and returns a list of received cookies which it contained. - * - A single "Cookie:" header is allowed to contain multiple cookies. Further, a HTTP request can contain multiple "Cookie" keys!. - * - * @param httpHeader The value of a "Cookie:" header (i.e. the prefix "Cookie:" must not be contained in this parameter!) - * @return A list of {@link ReceivedCookie} objects. The validity of their name/value pairs is not deeply checked, their getName() / getValue() might throw! - * @throws ParseException If the general formatting of the cookie is wrong. - */ - protected static ArrayList parseHeader(String httpHeader) throws ParseException { - - if(logMINOR) - Logger.minor(ReceivedCookie.class, "Received HTTP cookie header:" + httpHeader); - - char[] header = httpHeader.toCharArray(); - - String currentCookieName = null; - Hashtable currentCookieContent = new Hashtable(16); - - ArrayList cookies = new ArrayList(4); // TODO: Adjust to the usual amount of cookies which fred uses + 1 - - // We do manual parsing instead of using regular expressions for two reasons: - // 1. The value of cookies can be quoted, therefore it is a context-free language and not a regular language - we cannot express it with a regexp! - // 2. Its very fast :) - - // Set to true if a broken browser (Konqueror) specifies a cookie where the name is NOT the first attribute. - try { - for(int i = 0; i < header.length;) { - // Skip leading whitespace of key, we must do a header.length check because there might be no more key, so we continue; - if(Character.isWhitespace(header[i])) { - ++i; - continue; - } - - String key; - String value = null; - - // Parse key - { - int keyBeginIndex = i; - - while(i < header.length && header[i] != '=' && header[i] != ';') - ++i; - - int keyEndIndex = i; - - if(keyEndIndex >= header.length || header[keyEndIndex] == ';') - value = ""; - - while(Character.isWhitespace(header[keyEndIndex-1])) // Remove trailing whitespace - --keyEndIndex; - - key = new String(header, keyBeginIndex, keyEndIndex - keyBeginIndex).toLowerCase(); - - if(key.length() == 0) - throw new ParseException("Invalid cookie: Contains an empty key: " + httpHeader, i); - - // We're done parsing the key, continue to the next character. - ++i; - } - - // Parse value (empty values are allowed). - if(value == null && i < header.length) { - while(Character.isWhitespace(header[i])) // Skip leading whitespace - ++i; - - int valueBeginIndex; - char valueEndChar; - - if(header[i] == '\"') { // Value is quoted - valueEndChar = '\"'; - valueBeginIndex = ++i; - - while(header[i] != valueEndChar) +public final class ReceivedCookie { + + private static final int ONE_MB_OF_ASCII_TEXT = 1048576; + + private final String name; + private final String value; + + + /** + * Constructor for creating cookies from parsed key-value pairs. + *

+ * Does not validate the names or values of the keys, each attribute is validated at the first call to it's getter method. + * Therefore, no CPU time is wasted if the client sends cookies which we do not use. + */ + private ReceivedCookie(String myName, String myValue) { + if (myName == null) { + throw new IllegalArgumentException("Cookie name is null"); + } + if (myName.isEmpty()) { + throw new IllegalArgumentException("Cookie name is empty"); + } + name = myName; + value = myValue; + } + + /** + * Parses the value of a "Cookie:" HTTP header and returns a list of received cookies which it contained. + * - A single "Cookie:" header is allowed to contain multiple cookies. Further, a HTTP request can contain multiple "Cookie" keys!. + * + * @param httpHeader The value of a "Cookie:" header (i.e. the prefix "Cookie:" must not be contained in this parameter!) + * @return A list of {@link ReceivedCookie} objects. The validity of their name/value pairs is not deeply checked, their getName() / getValue() might throw! + * @throws ParseException If the general formatting of the cookie is wrong. + */ + static List parseHeader(String httpHeader) throws ParseException { + if (httpHeader == null || httpHeader.isEmpty()) { + return Collections.emptyList(); + } + if (httpHeader.length() > ONE_MB_OF_ASCII_TEXT) { + throw new IllegalArgumentException("Cookie value is too long. Length: " + httpHeader.length()); + } + char[] header = httpHeader.toCharArray(); + List cookies = new ArrayList<>(4); // TODO: Adjust to the usual amount of cookies which fred uses + 1 + + // We do manual parsing instead of using regular expressions for two reasons: + // 1. The value of cookies can be quoted, therefore it is a context-free language and not a regular language - we cannot express it with a regexp! + // 2. Its very fast :) + + String key = null; + String value = null; + try { + for (int i = 0; i < header.length; ) { + // Skip leading whitespace of key, we must do a header.length check because there might be no more key, so we continue; + if (Character.isWhitespace(header[i])) { + ++i; + continue; + } + + // Parse key + { + int keyBeginIndex = i; + + while (i < header.length && header[i] != '=' && header[i] != ';') { ++i; - - } else { - valueEndChar = ';'; - valueBeginIndex = i; - - while(i < header.length && header[i] != valueEndChar) + } + + int keyEndIndex = i; + + // Remove trailing whitespace + while (Character.isWhitespace(header[keyEndIndex - 1])) { + --keyEndIndex; + } + + key = new String(header, keyBeginIndex, keyEndIndex - keyBeginIndex); + + if (key.length() == 0) { + throw new ParseException("Invalid cookie: Contains an empty key: " + httpHeader, i); + } + + // We're done parsing the key, continue to the next character. + ++i; + } + + // Parse value (empty values are allowed). + if (i < header.length) { + // Skip leading whitespace + while (Character.isWhitespace(header[i])) { ++i; - } - - - int valueEndIndex = i; - - while(valueEndIndex > valueBeginIndex && Character.isWhitespace(header[valueEndIndex-1])) // Remove trailing whitespace - --valueEndIndex; - - value = new String(header, valueBeginIndex, valueEndIndex - valueBeginIndex); - - // We're done parsing the value, continue to the next character - ++i; - - // Skip whitespace between end of quotation and the semicolon following the quotation. - if(valueEndChar == '\"') { - while(i < header.length && header[i] != ';') { - if(!Character.isWhitespace(header[i])) - throw new ParseException("Invalid cookie: Missing terminating semicolon after value quotation: " + httpHeader, i); - + } + + int valueBeginIndex; + char valueEndChar; + + if (header[i] == '\"') { // Value is quoted + valueEndChar = '\"'; + valueBeginIndex = ++i; + + while (header[i] != valueEndChar) { + ++i; + } + + } else { + valueEndChar = ';'; + valueBeginIndex = i; + + while (i < header.length && header[i] != valueEndChar) { + ++i; + } + } + + + int valueEndIndex = i; + + // Remove trailing whitespace + while (valueEndIndex > valueBeginIndex && Character.isWhitespace(header[valueEndIndex - 1])) { + --valueEndIndex; + } + + value = new String(header, valueBeginIndex, valueEndIndex - valueBeginIndex); + + // We're done parsing the value, continue to the next character + ++i; + + // Skip whitespace between end of quotation and the semicolon following the quotation. + if (valueEndChar == '\"') { + while (i < header.length && header[i] != ';') { + if (!Character.isWhitespace(header[i])) { + throw new ParseException("Invalid cookie: Missing terminating semicolon after value quotation: " + httpHeader, i); + } + ++i; + } + + // We found the semicolon, skip it ++i; } - - // We found the semicolon, skip it - ++i; - } - - } - else - value = ""; - - // RFC2965: Name MUST be first. Anything key besides the name of the cookie begins with $. The next cookie begins if a key occurs which is not - // prefixed with $. - - if(currentCookieName == null) { // We have not found the name yet, the first key/value pair must be the name and the value of the cookie. - if(key.charAt(0) == '$') { - // We cannot throw because Konqueror (4.2.2) is broken and specifies $version as the first attribute. - //throw new IllegalArgumentException("Invalid cookie: Name is not the first attribute: " + httpHeader); - - currentCookieContent.put(key, value); - } else { - currentCookieName = key; - currentCookieContent.put(currentCookieName, value); - } - } else { - if(key.charAt(0) == '$') - currentCookieContent.put(key, value); - else {// We finished parsing of the current cookie, a new one starts here. - //if(singleCookie) - // throw new ParseException("Invalid cookie header: Multiple cookies specified but " - // + " the name of the first cookie was not the first attribute: " + httpHeader, i); - - cookies.add(new ReceivedCookie(currentCookieName, currentCookieContent)); // Store the previous cookie. - - currentCookieName = key; - currentCookieContent = new Hashtable(16); - currentCookieContent.put(currentCookieName, value); - } - } - } - } - catch(ArrayIndexOutOfBoundsException e) { - ParseException p = new ParseException("Index out of bounds (" + e.getMessage() + ") for cookie " + httpHeader, 0); - p.setStackTrace(e.getStackTrace()); - throw p; - } - - // Store the last cookie (the loop only stores the current cookie when a new one starts). - if(currentCookieName != null) - cookies.add(new ReceivedCookie(currentCookieName, currentCookieContent)); - - return cookies; - } - - - /** - * @throws IllegalArgumentException If the validation of the name fails. - */ - @Override - public String getName() { - if(name == null) { - name = validateName(notValidatedName); - notValidatedName = null; - } - - return name; - } - - /** - * @throws IllegalArgumentException If the validation of the domain fails. - */ - @Override - public URI getDomain() { - if(domain == null) { - try { - String domainString = content.get("$domain"); - if(domainString == null) - return null; - - domain = validateDomain(domainString); - } catch (URISyntaxException e) { - throw new IllegalArgumentException(e); - } + } + + cookies.add(new ReceivedCookie(key, value)); + key = null; + value = null; + } + } catch (ArrayIndexOutOfBoundsException e) { + ParseException p = new ParseException("Index out of bounds (" + e.getMessage() + ") for cookie " + httpHeader, 0); + p.setStackTrace(e.getStackTrace()); + throw p; + } + + // Store the last cookie (the loop only stores the current cookie when a new one starts). + if (key != null) { + cookies.add(new ReceivedCookie(key, value)); + } + + return cookies; + } + + /** + * @return cookie name + */ + public String getName() { + return name; + } + + /** + * @return cookie value, may be {@code null} or empty + */ + public String getValue() { + return value; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; } - - return domain; - } - - /** - * @throws IllegalArgumentException If the validation of the path fails. - */ - @Override - public URI getPath() { - if(path == null) { - try { - path = validatePath(content.get("$path")); - } catch (URISyntaxException e) { - throw new IllegalArgumentException(e); - } + if (o == null || getClass() != o.getClass()) { + return false; } - - return path; - } - - /** - * @throws IllegalArgumentException If the validation of the name fails. - */ - @Override - public String getValue() { - if(value == null) - value = validateValue(content.get(getName())); - - return value; - } - -// TODO: This is broken because TimeUtil.parseHTTPDate() does not work. -// public Date getExpirationDate() { -// if(expirationDate == null) { -// try { -// expirationDate = validateExpirationDate(TimeUtil.parseHTTPDate(content.get("$expires"))); -// } catch (ParseException e) { -// throw new IllegalArgumentException(e); -// } -// } -// -// return expirationDate; -// } - - @Override - protected String encodeToHeaderValue() { - throw new UnsupportedOperationException("ReceivedCookie objects cannot be encoded to a HTTP header value, use Cookie objects!"); - } - + ReceivedCookie that = (ReceivedCookie) o; + return name.equals(that.name) && Objects.equals(value, that.value); + } + + @Override + public int hashCode() { + return Objects.hash(name, value); + } } diff --git a/src/freenet/clients/http/SessionManager.java b/src/freenet/clients/http/SessionManager.java index 78029af6c15..e492a42a243 100644 --- a/src/freenet/clients/http/SessionManager.java +++ b/src/freenet/clients/http/SessionManager.java @@ -8,6 +8,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.text.ParseException; +import java.time.Instant; import java.util.Date; import java.util.Enumeration; import java.util.HashMap; @@ -23,407 +24,418 @@ /** * A basic session manager for cookie-based HTTP session. - * It allows its parent web interface to associate a "UserID" (a string) with a session ID. - * + * It allows its parent web interface to associate a "UserID" (a string) with a session ID. + *

* Formal definition of a SessionManager: * A 1:1 mapping of SessionID to UserID. Queries by SessionID and UserID run in O(1). * The Session ID primary key consists of: Cookie path + Cookie name + random "actual" session ID * The user ID is received from the client application. - * + *

* Therefore, when multiple client applications want to store sessions, each one is supposed * to create its own SessionManager because user IDs might overlap. - * + *

* The sessions of each application then get their {@link Session} by using a different * cookie path OR a different cookie namespace, depending on which constructor you use. - * + *

* Paths are used when client applications do NOT share the same path on the server. * For example "/Chat" would cause the browser to only send back the cookie if the user is - * browsing "/Chat/", not for "/". - * BUT usually we want the menu contents of client applications to be in the logged-in state even if - * the user is NOT browsing the client application web interface right now, therefore the "/" path + * browsing "/Chat/", not for "/". + * BUT usually we want the menu contents of client applications to be in the logged-in state even if + * the user is NOT browsing the client application web interface right now, therefore the "/" path * must be used in most cases. * If client application cookies shall be received from all paths on the server, the client * application should use the constructor which requires a cookie namespace. - * - * The usage of a namespace gurantees that Sessions of different client applications do not overlap. - * + *

+ * The usage of a namespace guarantees that Sessions of different client applications do not overlap. + * * @author xor (xor@freenetproject.org) */ public final class SessionManager { - /** - * The amount of milliseconds after which a session is deleted due to expiration. - */ - public static final long MAX_SESSION_IDLE_TIME = HOURS.toMillis(1); - - public static final String SESSION_COOKIE_NAME = "SessionID"; - - private final URI mCookiePath; - private final String mCookieNamespace; - private final String mCookieName; - - /** - * Constructs a new session manager for use with the given cookie path. - * Cookies are only sent back if the user is browsing the domain within that path. - * - * @param myCookiePath The path in which the cookies should be valid. - */ - public SessionManager(URI myCookiePath) { - if(myCookiePath.isAbsolute()) - throw new IllegalArgumentException("Illegal cookie path, must be relative: " + myCookiePath); - - if(myCookiePath.toString().startsWith("/") == false) - throw new IllegalArgumentException("Illegal cookie path, must start with /: " + myCookiePath); - - // FIXME: The new constructor was written at 2010-11-15. Uncomment the following safety check after we gave plugins some time to migrate - // if(myCookiePath.getPath().equals("/")) - // throw new IllegalArgumentException("Illegal cookie path '/'. You should use the constructor which allows the specification" + - // "of a namespace for using the global path."); - - - // TODO: Add further checks. - - //mCookieDomain = myCookieDomain; - mCookiePath = myCookiePath; - mCookieNamespace = ""; - mCookieName = SESSION_COOKIE_NAME; - } - - /** - * Constructs a new session manager for use with the "/" cookie path - * - * @param myCookieNamespace The name of the client application which uses this cookie. Must not be empty. Must be latin letters and numbers only. - */ - public SessionManager(String myCookieNamespace) { - if(myCookieNamespace.length() == 0) - throw new IllegalArgumentException("You must specify a cookie namespace or use the constructor " + - "which allows specification of a cookie path."); - - if(!StringValidityChecker.isLatinLettersAndNumbersOnly(myCookieNamespace)) - throw new IllegalArgumentException("The cookie namespace must be latin letters and numbers only."); - - //mCookieDomain = myCookieDomain; - try { - mCookiePath = new URI("/"); - } catch (URISyntaxException e) { - throw new RuntimeException(e); - } - mCookieNamespace = myCookieNamespace; - mCookieName = myCookieNamespace + SESSION_COOKIE_NAME; - } - - - public static final class Session { - - private final UUID mID; - private final String mUserID; - private final Map mAttributes = new HashMap(); - - private long mExpiresAtTime; - - private Session(String myUserID, long currentTime) { - mID = UUID.randomUUID(); - mUserID = myUserID; - mExpiresAtTime = currentTime + SessionManager.MAX_SESSION_IDLE_TIME; - } - - @Override - public boolean equals(Object obj) { - if(obj == null) return false; - if(!(obj instanceof Session)) return false; - Session other = ((Session)obj); - return other.getID().equals(mID); - } - - @Override - public int hashCode() { - return mID.hashCode(); - } - - public UUID getID() { - return mID; - } - - public String getUserID() { - return mUserID; - } - - private long getExpirationTime() { - return mExpiresAtTime; - } - - private boolean isExpired(long time) { - return time >= mExpiresAtTime; - } - - private void updateExpiresAtTime(long currentTime) { - mExpiresAtTime = currentTime + SessionManager.MAX_SESSION_IDLE_TIME; - } - - /** - * Returns whether this session contains an attribute with the given - * name. - * - * @param name - * The name of the attribute to check for - * @return {@code true} if this session contains an attribute with the - * given name, {@code false} otherwise - */ - public boolean hasAttribute(String name) { - return mAttributes.containsKey(name); - } - - /** - * Returns the value of the attribute with the given name. If there is - * no attribute with the given name, {@code null} is returned. - * - * @param name - * The name of the attribute whose value to get - * @return The value of the attribute, or {@code null} - */ - public Object getAttribute(String name) { - return mAttributes.get(name); - } - - /** - * Sets the value of the attribute with the given name. - * - * @param name - * The name of the attribute whose value to set - * @param value - * The new value of the attribute - */ - public void setAttribute(String name, Object value) { - mAttributes.put(name, value); - } - - /** - * Removes the attribute with the given name. Nothing will happen if - * there is no attribute with the given name. - * - * @param name - * The name of the attribute to remove - */ - public void removeAttribute(String name) { - mAttributes.remove(name); - } - - /** - * Returns the names of all currently existing attributes. - * - * @return The names of all attributes - */ - public Set getAttributeNames() { - return mAttributes.keySet(); - } - - } - - private final LRUMap mSessionsByID = new LRUMap(); - private final Hashtable mSessionsByUserID = new Hashtable(); - - - /** - * Returns the cookie path as specified in the constructor. - * Returns "/" if the constructor which only requires a namespace was used. - */ - public URI getCookiePath() { - return mCookiePath; - } - - - /** - * Returns the namespace as specified in the constructor. - * Returns an empty string if the constructor which requires a cookie path only was used. - */ - public String getCookieNamespace() { - return mCookieNamespace; - } - - - /** - * Creates a new session for the given user ID. - * - * If a session for the given user ID already exists, it is deleted. It is not re-used to ensure that parallel logins with the same user account from - * different computers do not work. - * - * @param context The ToadletContext in which the session cookie shall be stored. - */ - public synchronized Session createSession(String userID, ToadletContext context) { - // We must synchronize around the fetching of the time and mSessionsByID.push() because mSessionsByID is no sorting data structure: It's a plain - // LRUMap so to ensure that it stays sorted the operation "getTime(); push();" must be atomic. - long time = CurrentTimeUTC.getInMillis(); - - removeExpiredSessions(time); - - deleteSessionByUserID(userID); - - Session session = new Session(userID, time); - mSessionsByID.push(session.getID(), session); - mSessionsByUserID.put(session.getUserID(), session); - - setSessionCookie(session, context); - - return session; - } - - /** - * Returns true if the given {@link ToadletContext} contains a session cookie for a valid (existing and not expired) session. - * - * In opposite to {@link getSessionUserID}, this function does NOT extend the validity of the session. - * Therefore, this function can be considered as a way of peeking for a session, to decide which Toadlet links should be visible. - */ - public synchronized boolean sessionExists(ToadletContext context) { - UUID sessionID = getSessionID(context); - - if(sessionID == null) - return false; - - removeExpiredSessions(CurrentTimeUTC.getInMillis()); - - return mSessionsByID.containsKey(sessionID); - } - - /** - * Retrieves the session ID from the session cookie in the given {@link ToadletContext}, checks if it contains a valid (existing and not expired) session - * and if yes, returns the {@link Session}. - * - * If the session was valid, then its validity is extended by {@link MAX_SESSION_IDLE_TIME}. - * - * If the session did not exist or is not valid anymore, null is returned. - */ - public synchronized Session useSession(ToadletContext context) { - UUID sessionID = getSessionID(context); - if(sessionID == null) - return null; - - // We must synchronize around the fetching of the time and mSessionsByID.push() because mSessionsByID is no sorting data structure: It's a plain - // LRUMap so to ensure that it stays sorted the operation "getTime(); push();" must be atomic. - long time = CurrentTimeUTC.getInMillis(); - - removeExpiredSessions(time); - - Session session = mSessionsByID.get(sessionID); - - if(session == null) - return null; - - - session.updateExpiresAtTime(time); - mSessionsByID.push(session.getID(), session); - - setSessionCookie(session, context); - - return session; - } - - /** - * Retrieves the session ID from the session cookie in the given {@link ToadletContext}, checks if it contains a valid (existing and not expired) session - * and if yes, deletes the session. - * - * @return True if the session was deleted, false if there was no session cookie or no session. - */ - public boolean deleteSession(ToadletContext context) { - UUID sessionID = getSessionID(context); - if(sessionID == null) - return false; - - return deleteSession(sessionID); - } - - /** - * @return Returns the session ID stored in the cookies of the HTTP headers of the given {@link ToadletContext}. Returns null if there is no session ID stored. - */ - private UUID getSessionID(ToadletContext context) { - if(context == null) - return null; - - try { - ReceivedCookie sessionCookie = context.getCookie(null, mCookiePath, mCookieName); - - return sessionCookie == null ? null : UUID.fromString(sessionCookie.getValue()); - } catch(ParseException e) { - Logger.error(this, "Getting session cookie failed", e); - return null; - } catch(IllegalArgumentException e) { - Logger.error(this, "Getting the value of the session cookie failed", e); - return null; - } - } - - /** - * Stores a session cookie for the given session in the given {@link ToadletContext}'s HTTP headers. - * @param session - * @param context - */ - private void setSessionCookie(Session session, ToadletContext context) { - context.setCookie(new Cookie(mCookiePath, mCookieName, session.getID().toString(), new Date(session.getExpirationTime()))); - } - - /** - * Deletes the session with the given ID. - * - * @return True if a session with the given ID existed. - */ - private synchronized boolean deleteSession(UUID sessionID) { - Session session = mSessionsByID.get(sessionID); - - if(session == null) - return false; - - mSessionsByID.removeKey(sessionID); - mSessionsByUserID.remove(session.getUserID()); - return true; - } - - /** - * Deletes the session associated with the given user ID. - * - * @return True if a session with the given ID existed. - */ - private synchronized boolean deleteSessionByUserID(String userID) { - Session session = mSessionsByUserID.remove(userID); - if(session == null) - return false; - - mSessionsByID.removeKey(session.getID()); - return true; - } - - /** - * Garbage-collects any expired sessions. Must be called before client-inteface functions do anything which relies on the existence a session, - * that is: creating sessions, using sessions or checking whether sessions exist. - * - * FIXME: Before putting the session manager into fred, write a thread which periodically garbage collects old sessions - currently, sessions - * will only be garbage collected if any client continues using the SessiomManager - * - * @param time The current time. - */ - private synchronized void removeExpiredSessions(long time) { - for(Session session = mSessionsByID.peekValue(); session != null && session.isExpired(time); session = mSessionsByID.peekValue()) { - mSessionsByID.popValue(); - mSessionsByUserID.remove(session.getUserID()); - } - - // FIXME: Execute every few hours only. - verifySessionsByUserIDTable(); - } - - /** - * Debug function which checks whether the sessions by user ID table does not contain any sessions which do not exist anymore; - */ - private synchronized void verifySessionsByUserIDTable() { - - Enumeration sessions = mSessionsByUserID.elements(); - while(sessions.hasMoreElements()) { - Session session = sessions.nextElement(); - - if(mSessionsByID.containsKey(session.getID()) == false) { - Logger.error(this, "Sessions by user ID hashtable contains deleted session, removing it: " + session); - - mSessionsByUserID.remove(session.getUserID()); - } - } - } + /** + * The amount of milliseconds after which a session is deleted due to expiration. + */ + public static final long MAX_SESSION_IDLE_TIME = HOURS.toMillis(1); + + public static final String SESSION_COOKIE_NAME = "SessionID"; + + private final LRUMap mSessionsByID = new LRUMap<>(); + private final Map mSessionsByUserID = new HashMap<>(); + + private final URI mCookiePath; + private final String mCookieNamespace; + private final String mCookieName; + + /** + * Constructs a new session manager for use with the given cookie path. + * Cookies are only sent back if the user is browsing the domain within that path. + * + * @param myCookiePath The path in which the cookies should be valid. + */ + public SessionManager(URI myCookiePath) { + if (myCookiePath.isAbsolute()) { + throw new IllegalArgumentException("Illegal cookie path, must be relative: " + myCookiePath); + } + + if (!myCookiePath.toString().startsWith("/")) { + throw new IllegalArgumentException("Illegal cookie path, must start with /: " + myCookiePath); + } + + // FIXME: The new constructor was written at 2010-11-15. Uncomment the following safety check after we gave plugins some time to migrate + // if(myCookiePath.getPath().equals("/")) + // throw new IllegalArgumentException("Illegal cookie path '/'. You should use the constructor which allows the specification" + + // "of a namespace for using the global path."); + + + // TODO: Add further checks. + + //mCookieDomain = myCookieDomain; + mCookiePath = myCookiePath; + mCookieNamespace = ""; + mCookieName = SESSION_COOKIE_NAME; + } + + /** + * Constructs a new session manager for use with the "/" cookie path + * + * @param myCookieNamespace The name of the client application which uses this cookie. Must not be empty. Must be latin letters and numbers only. + */ + public SessionManager(String myCookieNamespace) { + if (myCookieNamespace.length() == 0) { + throw new IllegalArgumentException("You must specify a cookie namespace or use the constructor " + + "which allows specification of a cookie path."); + } + + if (!StringValidityChecker.isLatinLettersAndNumbersOnly(myCookieNamespace)) { + throw new IllegalArgumentException("The cookie namespace must be latin letters and numbers only."); + } + + try { + mCookiePath = new URI("/"); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + mCookieNamespace = myCookieNamespace; + mCookieName = myCookieNamespace + SESSION_COOKIE_NAME; + } + + + public static final class Session { + + private final UUID mID; + private final String mUserID; + private final Map mAttributes = new HashMap<>(); + + private long mExpiresAtTime; + + private Session(String myUserID, long currentTime) { + mID = UUID.randomUUID(); + mUserID = myUserID; + mExpiresAtTime = currentTime + SessionManager.MAX_SESSION_IDLE_TIME; + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (!(obj instanceof Session)) { + return false; + } + Session other = ((Session) obj); + return other.getID().equals(mID); + } + + @Override + public int hashCode() { + return mID.hashCode(); + } + + public UUID getID() { + return mID; + } + + public String getUserID() { + return mUserID; + } + + private long getExpirationTime() { + return mExpiresAtTime; + } + + private boolean isExpired(long time) { + return time >= mExpiresAtTime; + } + + private void updateExpiresAtTime(long currentTime) { + mExpiresAtTime = currentTime + SessionManager.MAX_SESSION_IDLE_TIME; + } + + /** + * Returns whether this session contains an attribute with the given + * name. + * + * @param name The name of the attribute to check for + * @return {@code true} if this session contains an attribute with the + * given name, {@code false} otherwise + */ + public boolean hasAttribute(String name) { + return mAttributes.containsKey(name); + } + + /** + * Returns the value of the attribute with the given name. If there is + * no attribute with the given name, {@code null} is returned. + * + * @param name The name of the attribute whose value to get + * @return The value of the attribute, or {@code null} + */ + public Object getAttribute(String name) { + return mAttributes.get(name); + } + + /** + * Sets the value of the attribute with the given name. + * + * @param name The name of the attribute whose value to set + * @param value The new value of the attribute + */ + public void setAttribute(String name, Object value) { + mAttributes.put(name, value); + } + + /** + * Removes the attribute with the given name. Nothing will happen if + * there is no attribute with the given name. + * + * @param name The name of the attribute to remove + */ + public void removeAttribute(String name) { + mAttributes.remove(name); + } + + /** + * Returns the names of all currently existing attributes. + * + * @return The names of all attributes + */ + public Set getAttributeNames() { + return mAttributes.keySet(); + } + + } + + + /** + * Returns the cookie path as specified in the constructor. + * Returns "/" if the constructor which only requires a namespace was used. + */ + public URI getCookiePath() { + return mCookiePath; + } + + + /** + * Returns the namespace as specified in the constructor. + * Returns an empty string if the constructor which requires a cookie path only was used. + */ + public String getCookieNamespace() { + return mCookieNamespace; + } + + + /** + * Creates a new session for the given user ID. + *

+ * If a session for the given user ID already exists, it is deleted. It is not re-used to ensure that parallel logins with the same user account from + * different computers do not work. + * + * @param context The ToadletContext in which the session cookie shall be stored. + */ + public synchronized Session createSession(String userID, ToadletContext context) { + // We must synchronize around the fetching of the time and mSessionsByID.push() because mSessionsByID is no sorting data structure: It's a plain + // LRUMap so to ensure that it stays sorted the operation "getTime(); push();" must be atomic. + long time = CurrentTimeUTC.getInMillis(); + + removeExpiredSessions(time); + + deleteSessionByUserID(userID); + + Session session = new Session(userID, time); + mSessionsByID.push(session.getID(), session); + mSessionsByUserID.put(session.getUserID(), session); + + setSessionCookie(session, context); + + return session; + } + + /** + * Returns true if the given {@link ToadletContext} contains a session cookie for a valid (existing and not expired) session. + *

+ * In opposite to {@link #useSession}, this function does NOT extend the validity of the session. + * Therefore, this function can be considered as a way of peeking for a session, to decide which Toadlet links should be visible. + */ + public synchronized boolean sessionExists(ToadletContext context) { + UUID sessionID = getSessionID(context); + + if (sessionID == null) { + return false; + } + + removeExpiredSessions(CurrentTimeUTC.getInMillis()); + + return mSessionsByID.containsKey(sessionID); + } + + /** + * Retrieves the session ID from the session cookie in the given {@link ToadletContext}, checks if it contains a valid (existing and not expired) session + * and if yes, returns the {@link Session}. + *

+ * If the session was valid, then its validity is extended by {@link #MAX_SESSION_IDLE_TIME}. + *

+ * If the session did not exist or is not valid anymore, null is returned. + */ + public synchronized Session useSession(ToadletContext context) { + UUID sessionID = getSessionID(context); + if (sessionID == null) { + return null; + } + + // We must synchronize around the fetching of the time and mSessionsByID.push() because mSessionsByID is no sorting data structure: It's a plain + // LRUMap so to ensure that it stays sorted the operation "getTime(); push();" must be atomic. + long time = CurrentTimeUTC.getInMillis(); + + removeExpiredSessions(time); + + Session session = mSessionsByID.get(sessionID); + if (session == null) { + return null; + } + + session.updateExpiresAtTime(time); + mSessionsByID.push(session.getID(), session); + + setSessionCookie(session, context); + + return session; + } + + /** + * Retrieves the session ID from the session cookie in the given {@link ToadletContext}, checks if it contains a valid (existing and not expired) session + * and if yes, deletes the session. + * + * @return True if the session was deleted, false if there was no session cookie or no session. + */ + public boolean deleteSession(ToadletContext context) { + UUID sessionID = getSessionID(context); + if (sessionID == null) { + return false; + } + + return deleteSession(sessionID); + } + + /** + * @return Returns the session ID stored in the cookies of the HTTP headers of the given {@link ToadletContext}. Returns null if there is no session ID stored. + */ + private UUID getSessionID(ToadletContext context) { + if (context == null) { + return null; + } + try { + ReceivedCookie sessionCookie = context.getCookie(null, mCookiePath, mCookieName); + if (sessionCookie == null) { + return null; + } + return UUID.fromString(sessionCookie.getValue()); + } catch (ParseException e) { + Logger.error(this, "Getting session cookie failed", e); + return null; + } catch (IllegalArgumentException e) { + Logger.error(this, "Getting the value of the session cookie failed", e); + return null; + } + } + + /** + * Stores a session cookie for the given session in the given {@link ToadletContext}'s HTTP headers. + * + * @param session + * @param context + */ + private void setSessionCookie(Session session, ToadletContext context) { + context.setCookie( + new Cookie( + mCookiePath, + mCookieName, + session.getID().toString(), + Instant.ofEpochMilli(session.getExpirationTime()) + ) + ); + } + + /** + * Deletes the session with the given ID. + * + * @return True if a session with the given ID existed. + */ + private synchronized boolean deleteSession(UUID sessionID) { + Session session = mSessionsByID.get(sessionID); + if (session == null) { + return false; + } + + mSessionsByID.removeKey(sessionID); + mSessionsByUserID.remove(session.getUserID()); + return true; + } + + /** + * Deletes the session associated with the given user ID. + * + * @return True if a session with the given ID existed. + */ + private synchronized boolean deleteSessionByUserID(String userID) { + Session session = mSessionsByUserID.remove(userID); + if (session == null) { + return false; + } + + mSessionsByID.removeKey(session.getID()); + return true; + } + + /** + * Garbage-collects any expired sessions. Must be called before client-interface functions do anything which relies on the existence a session, + * that is: creating sessions, using sessions or checking whether sessions exist. + *

+ * FIXME: Before putting the session manager into fred, write a thread which periodically garbage collects old sessions - currently, sessions + * will only be garbage collected if any client continues using the SessionManager + * + * @param time The current time. + */ + private synchronized void removeExpiredSessions(long time) { + for (Session session = mSessionsByID.peekValue(); session != null && session.isExpired(time); session = mSessionsByID.peekValue()) { + mSessionsByID.popValue(); + mSessionsByUserID.remove(session.getUserID()); + } + + // FIXME: Execute every few hours only. + verifySessionsByUserIDTable(); + } + + /** + * Debug function which checks whether the sessions by user ID table does not contain any sessions which do not exist anymore; + */ + private synchronized void verifySessionsByUserIDTable() { + for (Session session : mSessionsByUserID.values()) { + if (!mSessionsByID.containsKey(session.getID())) { + Logger.error(this, "Sessions by user ID hashtable contains deleted session, removing it: " + session); + + mSessionsByUserID.remove(session.getUserID()); + } + } + } } diff --git a/src/freenet/clients/http/ToadletContextImpl.java b/src/freenet/clients/http/ToadletContextImpl.java index f3518a72fe0..4739d97de21 100644 --- a/src/freenet/clients/http/ToadletContextImpl.java +++ b/src/freenet/clients/http/ToadletContextImpl.java @@ -12,16 +12,10 @@ import java.net.Socket; import java.net.URI; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.text.ParseException; -import java.text.SimpleDateFormat; -import java.util.ArrayList; -import java.util.Date; -import java.util.Enumeration; -import java.util.List; -import java.util.Locale; -import java.util.StringJoiner; -import java.util.TimeZone; +import java.util.*; import freenet.clients.http.FProxyFetchInProgress.REFILTER_POLICY; import freenet.clients.http.annotation.AllowData; @@ -54,7 +48,7 @@ */ public class ToadletContextImpl implements ToadletContext { - private static final Class HANDLE_PARAMETERS[] = new Class[] { + private static final Class[] HANDLE_PARAMETERS = new Class[] { URI.class, HTTPRequest.class, ToadletContext.class }; @@ -81,7 +75,7 @@ public class ToadletContextImpl implements ToadletContext { /** The unique id of the request*/ private final String uniqueId; - private URI uri; + private final URI uri; private static volatile boolean logMINOR; private static volatile boolean logDEBUG; @@ -101,7 +95,7 @@ public void shouldUpdate(){ private boolean closed; private boolean shouldDisconnect; - public ToadletContextImpl(Socket sock, MultiValueTable headers, BucketFactory bf, PageMaker pageMaker, ToadletContainer container, UserAlertManager userAlertManager, BookmarkManager bookmarkManager, URI uri, long uniqueID) throws IOException { + public ToadletContextImpl(Socket sock, MultiValueTable headers, BucketFactory bf, PageMaker pageMaker, ToadletContainer container, UserAlertManager userAlertManager, BookmarkManager bookmarkManager, URI uri) throws IOException { this.headers = headers; this.cookies = null; this.replyCookies = null; @@ -117,16 +111,18 @@ public ToadletContextImpl(Socket sock, MultiValueTable headers, B this.userAlertManager = userAlertManager; this.bookmarkManager = bookmarkManager; //Generate an unique id - uniqueId=String.valueOf(Math.random()); + this.uniqueId = String.valueOf(Math.random()); } private void close() { closed = true; } - private void sendMethodNotAllowed(String method, boolean shouldDisconnect) throws ToadletContextClosedException, IOException { - if(closed) throw new ToadletContextClosedException(); - MultiValueTable mvt = new MultiValueTable(); + private void sendMethodNotAllowed(boolean shouldDisconnect) throws ToadletContextClosedException, IOException { + if(closed) { + throw new ToadletContextClosedException(); + } + MultiValueTable mvt = new MultiValueTable<>(); mvt.put("Allow", "GET, PUT"); sendError(sockOutputStream, 405, "Method Not Allowed", l10n("methodNotAllowed"), shouldDisconnect, mvt); } @@ -159,8 +155,10 @@ private static void sendError(OutputStream os, int code, String httpReason, Stri * @throws IOException If we could not send the error message. */ private static void sendHTMLError(OutputStream os, int code, String httpReason, String htmlMessage, boolean disconnect, MultiValueTable mvt) throws IOException { - if(mvt == null) mvt = new MultiValueTable(); - byte[] messageBytes = htmlMessage.getBytes("UTF-8"); + if(mvt == null) { + mvt = new MultiValueTable<>(); + } + byte[] messageBytes = htmlMessage.getBytes(StandardCharsets.UTF_8); sendReplyHeaders(os, code, httpReason, mvt, "text/html; charset=UTF-8", messageBytes.length, null, disconnect, false, false); os.write(messageBytes); } @@ -175,7 +173,7 @@ private static void sendURIParseError(OutputStream os, boolean shouldDisconnect, PrintWriter pw = new PrintWriter(sw); e.printStackTrace(pw); pw.close(); - String message = ""+l10n("uriParseErrorTitle")+"

"+HTMLEncoder.encode(e.getMessage())+"

\n"+sw.toString();
+		String message = ""+l10n("uriParseErrorTitle")+"
"+HTMLEncoder.encode(e.getMessage())+"
\n" + sw;
 		sendHTMLError(os, 400, "Bad Request", message, shouldDisconnect, null);
 	}
 
@@ -203,14 +201,14 @@ public void sendReplyHeadersStatic(int replyCode, String replyDescription, Multi
 	
 	@Override
 	public void sendReplyHeadersFProxy(int replyCode, String replyDescription, MultiValueTable mvt, String mimeType, long contentLength) throws ToadletContextClosedException, IOException {
-	    boolean enableJavascript = false;
-	    if(container.isFProxyWebPushingEnabled() && container.isFProxyJavascriptEnabled())
-	        enableJavascript = true;
-	    sendReplyHeaders(replyCode, replyDescription, mvt, mimeType, contentLength, null, false, true, enableJavascript);
+	    boolean enableJavascript = container.isFProxyWebPushingEnabled() && container.isFProxyJavascriptEnabled();
+		sendReplyHeaders(replyCode, replyDescription, mvt, mimeType, contentLength, null, false, true, enableJavascript);
 	}
 	
 	private void sendReplyHeaders(int replyCode, String replyDescription, MultiValueTable mvt, String mimeType, long contentLength, Date mTime, boolean isOutlinkConfirmationPage, boolean allowFrames, boolean enableJavascript) throws ToadletContextClosedException, IOException {
-		if(closed) throw new ToadletContextClosedException();
+		if(closed) {
+			throw new ToadletContextClosedException();
+		}
 		if(firstReplySendingException != null) {
 			throw new IllegalStateException("Already sent headers!", firstReplySendingException);
 		}
@@ -218,7 +216,7 @@ private void sendReplyHeaders(int replyCode, String replyDescription, MultiValue
 		
 		if(replyCookies != null) {
 			if (mvt == null) {
-				mvt = new MultiValueTable();
+				mvt = new MultiValueTable<>();
 			}
 			
 			// We do NOT use "set-cookie2" even though we should according though RFC2965 - Firefox 3.0.14 ignores it for me!
@@ -226,8 +224,9 @@ private void sendReplyHeaders(int replyCode, String replyDescription, MultiValue
 			for(Cookie cookie : replyCookies) {
 				final String cookieHeader = cookie.encodeToHeaderValue();
 				mvt.put("set-cookie", cookieHeader);
-				if(logMINOR)
+				if(logMINOR) {
 					Logger.minor(this, "set-cookie: " + cookieHeader);
+				}
 			}
 		}
 		sendReplyHeaders(sockOutputStream, replyCode, replyDescription, mvt, mimeType, contentLength, mTime, shouldDisconnect, enableJavascript, allowFrames);
@@ -253,7 +252,7 @@ public boolean checkFormPassword(HTTPRequest request)
 	public boolean checkFormPassword(HTTPRequest request, String redirectTo)
 			throws ToadletContextClosedException, IOException {
 		if (!hasFormPassword(request)) {
-			MultiValueTable headers = new MultiValueTable();
+			MultiValueTable headers = new MultiValueTable<>();
 			headers.put("Location", redirectTo);
 			sendReplyHeaders(302, "Found", headers, null, 0);
 			return false;
@@ -276,15 +275,18 @@ public boolean checkFullAccess(Toadlet toadlet) throws ToadletContextClosedExcep
     }
 	
 	@Override
-	public boolean hasFormPassword(HTTPRequest request) throws IOException {
+	public boolean hasFormPassword(HTTPRequest request) {
 		String pass = request.getPartAsStringFailsafe("formPassword", 32);
-		byte[] inputBytes = pass.getBytes("UTF-8");
-		byte[] compareBytes = getFormPassword().getBytes("UTF-8");
+		byte[] inputBytes = pass.getBytes(StandardCharsets.UTF_8);
+		byte[] compareBytes = getFormPassword().getBytes(StandardCharsets.UTF_8);
 		if(!MessageDigest.isEqual(inputBytes, compareBytes)) {
-			if (logMINOR)
+			if (logMINOR) {
 				Logger.minor(this, "Bad formPassword: " + pass);
+			}
 			return false;
-		} else return true;
+		} else {
+			return true;
+		}
 	}
 	
 	@Override
@@ -303,18 +305,23 @@ public MultiValueTable getHeaders() {
 	}
 	
 	private void parseCookies() throws ParseException {
-		if(cookies != null)
+		if(cookies != null) {
 			return;
+		}
 		
 		int cookieAmount = headers.countAll("cookie");
 		
-		if(cookieAmount == 0)
+		if(cookieAmount == 0) {
 			return;
+		}
 		
-		cookies = new ArrayList(cookieAmount + 1);
+		cookies = new ArrayList<>(cookieAmount + 1);
 		
 		for(String cookieHeader : headers.iterateAll("cookie")) {
-			ArrayList parsedCookies = ReceivedCookie.parseHeader(cookieHeader);
+			if(logMINOR) {
+				Logger.minor(this, "Received HTTP cookie header:" + cookieHeader);
+			}
+			List parsedCookies = ReceivedCookie.parseHeader(cookieHeader);
 			cookies.addAll(parsedCookies);
 		}
 	}
@@ -328,31 +335,15 @@ public ReceivedCookie getCookie(URI domain, URI path, String name) throws ParseE
 		
 		name = name.toLowerCase();
 		
-		//String stringDomain = domain==null ? null : domain.toString().toLowerCase();
-		//String stringPath = path.toString();
-		
+
 		// RFC2965: Two cookies are equal if name and domain are equal with case-insensitive comparison and path is equal with case-sensitive comparison.
 		//getName() / getDomain() returns lowercase and getPath() returns the original path.
 		
 		// UNFORTUNATELY firefox will ONLY give us the name and the value of the cookie, so we ignore everything else.
 		
 		for(ReceivedCookie cookie : cookies) {
-			try {
-			//if(stringDomain != null) {
-			//	URI cookieDomain = cookie.getDomain();
-			//	
-			//	if(cookieDomain==null || !stringDomain.equals(cookieDomain.toString()))
-			//		continue;
-			//}
-			//
-			//if(cookie.getPath().toString().equals(stringPath) && cookie.getName().equals(name))
-			//	return cookie;
-				
-				if(cookie.getName().equals(name))
-					return cookie;
-			}
-			catch(RuntimeException e) {
-				Logger.error(this, "Error in cookie", e);
+			if(cookie.getName().equalsIgnoreCase(name)) {
+				return cookie;
 			}
 		}
 		
@@ -361,8 +352,9 @@ public ReceivedCookie getCookie(URI domain, URI path, String name) throws ParseE
 	
 	@Override
 	public void setCookie(Cookie newCookie) {
-		if(replyCookies == null)
-			replyCookies = new ArrayList(4);
+		if(replyCookies == null) {
+			replyCookies = new ArrayList<>(4);
+		}
 		
 		replyCookies.add(newCookie);
 	}
@@ -370,31 +362,31 @@ public void setCookie(Cookie newCookie) {
 	static void sendReplyHeaders(OutputStream sockOutputStream, int replyCode, String replyDescription, MultiValueTable mvt, String mimeType, long contentLength, Date mTime, boolean disconnect, boolean allowScripts, boolean allowFrames) throws IOException {
 		
 		// Construct headers
-		if(mvt == null)
-			mvt = new MultiValueTable();
-		if(mimeType != null)
-			if(mimeType.equalsIgnoreCase("text/html")){
-				mvt.put("content-type", mimeType+"; charset=UTF-8");
-			}else{
+		if(mvt == null) {
+			mvt = new MultiValueTable<>();
+		}
+		if(mimeType != null) {
+			if (mimeType.equalsIgnoreCase("text/html")) {
+				mvt.put("content-type", mimeType + "; charset=UTF-8");
+			} else {
 				mvt.put("content-type", mimeType);
 			}
-		if(contentLength >= 0)
+		}
+		if(contentLength >= 0) {
 			mvt.put("content-length", Long.toString(contentLength));
-
-		boolean allowCaching; // For privacy reasons, only static
-							  // content may be cached
-		if (mTime == null) {
-			allowCaching = false;
-		} else {
-			allowCaching = true;
 		}
+
+		// For privacy reasons, only static
+		// content may be cached
+		boolean allowCaching = mTime != null;
+
 		String expiresTime;
 		String cacheControl;
 		if (allowCaching) {
 			// use an expiry time of 30 day from now, about the frequency of Freenet releases
 			// Expires is needed for older browsers
 			expiresTime = TimeUtil.makeHTTPDate(System.currentTimeMillis() + DAYS.toMillis(30));
-			cacheControl = "public, max-age=" + String.valueOf(3600 * 24 * 30);
+			cacheControl = "public, max-age=" + 3600 * 24 * 30;
 		} else {
 			expiresTime = "Thu, 01 Jan 1970 00:00:00 GMT";
 			// no-cache for Internet Explorer, no-store for Firefox
@@ -414,10 +406,11 @@ static void sendReplyHeaders(OutputStream sockOutputStream, int replyCode, Strin
 		
 		mvt.put("last-modified", lastModString);
 		mvt.put("date", nowString);
-		if(disconnect)
+		if(disconnect) {
 			mvt.put("connection", "close");
-		else
+		} else {
 			mvt.put("connection", "keep-alive");
+		}
 		String contentSecurityPolicy = generateCSP(allowScripts, allowFrames);
 		mvt.put("content-security-policy", contentSecurityPolicy);
 		mvt.put("x-content-security-policy", contentSecurityPolicy);
@@ -429,12 +422,12 @@ static void sendReplyHeaders(OutputStream sockOutputStream, int replyCode, Strin
 		buf.append(' ');
 		buf.append(replyDescription);
 		buf.append("\r\n");
-		for(Enumeration e = mvt.keys();e.hasMoreElements();) {
+		for (Enumeration e = mvt.keys(); e.hasMoreElements(); ) {
 			String key = e.nextElement();
 			Object[] list = mvt.getArray(key);
 			key = fixKey(key);
-			for(int i=0;i headers = new MultiValueTable();
+				MultiValueTable headers = new MultiValueTable<>();
 				
 				while(true) {
 					String line = lis.readLine(32768, 128, false); // ISO-8859 or US-ASCII, not UTF-8
@@ -569,7 +546,7 @@ public static void handle(Socket sock, ToadletContainer container, PageMaker pag
 				boolean allowPost = container.allowPosts();
 				BucketFactory bf = container.getBucketFactory();
 				
-				ToadletContextImpl ctx = new ToadletContextImpl(sock, headers, bf, pageMaker, container, userAlertManager, bookmarkManager, uri, container.generateUniqueID());
+				ToadletContextImpl ctx = new ToadletContextImpl(sock, headers, bf, pageMaker, container, userAlertManager, bookmarkManager, uri);
 				ctx.shouldDisconnect = disconnect;
 				
 				/*
@@ -614,7 +591,7 @@ public static void handle(Socket sock, ToadletContainer container, PageMaker pag
 					} else {
 						FileUtil.skipFully(is, len);
 						if (method.equals("POST")) {
-							ctx.sendMethodNotAllowed("POST", true);
+							ctx.sendMethodNotAllowed(true);
 						} else {
 							sendError(sock.getOutputStream(), 403, "Forbidden", "Content not allowed in this configuration", true, null);
 						}
@@ -657,7 +634,7 @@ public static void handle(Socket sock, ToadletContainer container, PageMaker pag
 						// if the Toadlet does not support the method, we don't need to parse the data
 						// also due this pre check a 'NoSuchMethodException' should never appear
 						if (!(t.findSupportedMethods().contains(method))) {
-							ctx.sendMethodNotAllowed(method, ctx.shouldDisconnect);
+							ctx.sendMethodNotAllowed(ctx.shouldDisconnect);
 							break;
 						}
 
@@ -717,8 +694,8 @@ public static void handle(Socket sock, ToadletContainer container, PageMaker pag
 				PrintWriter pw = new PrintWriter(sw);
 				t.printStackTrace(pw);
 				pw.flush();
-				msg = msg + sw.toString() + "
";
-				byte[] messageBytes = msg.getBytes("UTF-8");
+				msg = msg + sw + "
"; + byte[] messageBytes = msg.getBytes(StandardCharsets.UTF_8); sendReplyHeaders(sock.getOutputStream(), 500, "Internal failure", null, "text/html; charset=UTF-8", messageBytes.length, null, true, false, false); sock.getOutputStream().write(messageBytes); } catch (IOException e1) { @@ -761,7 +738,7 @@ private static void callToadletMethod(Toadlet t, String method, URI uri, HTTPReq } } ctx.setActiveToadlet(t); - Object arglist[] = new Object[] {uri, req, ctx}; + Object[] arglist = new Object[] {uri, req, ctx}; m.invoke(t, arglist); } catch (InvocationTargetException ite) { throw ite.getCause(); @@ -792,11 +769,7 @@ private static boolean shouldDisconnectAfterHandled(boolean isHTTP10, MultiValue if(connection.equalsIgnoreCase("keep-alive")) return false; } - if(isHTTP10 == true) - return true; - else - // HTTP 1.1 - return false; + return isHTTP10; // or else HTTP 1.1 } @Override diff --git a/test/freenet/clients/http/CookieTest.java b/test/freenet/clients/http/CookieTest.java index 636cbd2e680..95cf7250464 100644 --- a/test/freenet/clients/http/CookieTest.java +++ b/test/freenet/clients/http/CookieTest.java @@ -3,105 +3,310 @@ * http://www.gnu.org/ for further details of the GPL. */ package freenet.clients.http; +import static freenet.clients.http.Cookie.COOKIE_TOKEN_SEPARATOR_CHARACTERS; +import static freenet.clients.http.Cookie.COOKIE_VALUE_FORBIDDEN_CHARS; import static org.junit.Assert.*; import java.net.URI; import java.net.URISyntaxException; -import java.util.Date; +import java.time.Instant; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Locale; +import java.util.Set; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; -import freenet.support.CurrentTimeUTC; +import org.junit.function.ThrowingRunnable; public class CookieTest { - - static final String VALID_PATH = "/Freetalk"; - static final String VALID_NAME = "SessionID"; - static final String VALID_VALUE = "abCd12345"; - + + private static final String VALID_PATH = "/Freetalk"; + private static final String VALID_NAME = "SessionID"; + private static final String VALID_VALUE = "abCd12345"; + + + /** + * https://www.ietf.org/rfc/rfc2965.html#section-3.2 + */ + private static final Set RESERVED_COOKIE_MEMBERS = Collections.unmodifiableSet( + new HashSet<>( + Arrays.asList( + "Comment", + "CommentURL", + "Discard", + "Domain", + "Max-Age", + "Path", + "Port", + "Secure", + "Version" + ) + ) + ); + + /** + * Sample control characters + */ + private static final Set SOME_CONTROL_CHARACTERS = Collections.unmodifiableSet( + new HashSet<>( + Arrays.asList( + (char) 127, // DEL + (char) 27, // ESC + (char) 8, // backspace + '\n', + '\r', + '\t', + (char) 0 + ) + ) + ); + + /** + * https://www.ietf.org/rfc/rfc2396.html#section-2.2 + */ + private static final Set URI_RESERVED_CHARACTERS = Collections.unmodifiableSet( + new HashSet<>( + Arrays.asList( + ';', '/', '?', ':', ':', '@', '&', '=', '+', '$', ',' + ) + ) + ); + /** + * https://www.ietf.org/rfc/rfc2396.html#section-2.3 + */ + private static final Set URI_UNRESERVED_CHARACTERS = Collections.unmodifiableSet( + new HashSet<>( + Arrays.asList( + 'a', 'A', '7', '-', '_', '.', '!', '~', '*', '\'', '(', ')' + ) + ) + ); + + private static final Set VALID_TOKENS = Collections.unmodifiableSet( + new HashSet<>( + Arrays.asList( + "my-name", + "-name", + "name-", + "my_name", + "_name", + "name_", + "my+name", + "+name", + "name+", + "my.name", + ".name", + "name.", + " name " + ) + ) + ); + URI validPath; - Date validExpiresDate; + Instant validExpiresTime; Cookie cookie; @Before public void setUp() throws Exception { validPath = new URI(VALID_PATH); - validExpiresDate = new Date(CurrentTimeUTC.getInMillis()+60*60*1000); - cookie = new Cookie(validPath, VALID_NAME, VALID_VALUE, validExpiresDate); + validExpiresTime = Instant.now().plusMillis(+60*60*1000); + cookie = new Cookie(validPath, VALID_NAME, VALID_VALUE, validExpiresTime); } @Test - public void testCookieURIStringStringDate() throws URISyntaxException { - try { - new Cookie(null, VALID_NAME, VALID_VALUE, validExpiresDate); - fail("Constructor allows path to be null"); - } catch(RuntimeException e) {} - - try { - new Cookie(new URI(""), VALID_NAME, VALID_VALUE, validExpiresDate); - fail("Constructor allows path to be empty"); + public void testCookieURIParameter() throws Exception { + assertThrowsIllegalArgumentException( + "Constructor allows path to be null", + () -> new Cookie(null, VALID_NAME, VALID_VALUE, validExpiresTime) + ); + assertThrowsIllegalArgumentException( + "Constructor allows path to be empty", + () -> new Cookie(new URI(""), VALID_NAME, VALID_VALUE, validExpiresTime) + ); + + for (Character c : URI_RESERVED_CHARACTERS) { + new Cookie(new URI("/my" + c + "/value"), VALID_NAME, VALID_VALUE, validExpiresTime); } - catch(RuntimeException e) {} - - // TODO: Test for invalid characters in path. - - try { - new Cookie(validPath, null, VALID_VALUE, validExpiresDate); - fail("Constructor allows name to be null"); - } catch(RuntimeException e) {} - - try { - new Cookie(validPath, "", VALID_VALUE, validExpiresDate); - fail("Constructor allows name to be empty"); - } catch(RuntimeException e) {} - - try { - new Cookie(validPath, "test;", VALID_VALUE, validExpiresDate); - fail("Constructor allows invalid characters in name"); - } catch(RuntimeException e) {} - - // TODO: Test for more invalid characters in name + + for (Character c : URI_UNRESERVED_CHARACTERS) { + new Cookie(new URI("/my" + c + "/value"), VALID_NAME, VALID_VALUE, validExpiresTime); + } + + assertThrowsIllegalArgumentException( + "Constructor allows path not starting with /", + () -> new Cookie(new URI("my/path"), VALID_NAME, VALID_VALUE, validExpiresTime) + ); + + assertThrowsIllegalArgumentException( + "Constructor allows path containing full URI", + () -> new Cookie(new URI("http://example.com/my/path"), VALID_NAME, VALID_VALUE, validExpiresTime) + ); + + for (String s : Arrays.asList("free%20net", "net%09free")) { + new Cookie(new URI("/" + s), VALID_NAME, VALID_VALUE, validExpiresTime); + } + } + + @Test + public void testCookieNameParameter() { + assertThrowsIllegalArgumentException( + "Constructor allows name to be null", + () -> cookieForName(null) + ); + + assertThrowsIllegalArgumentException( + "Constructor allows name to be empty", + () -> cookieForName("") + ); + + assertThrowsIllegalArgumentException( + "Constructor allows invalid characters in name", + () -> cookieForName("test;") + ); + + for (String reservedName: RESERVED_COOKIE_MEMBERS) { + assertInvalidCookieAttributeName(reservedName); + assertInvalidCookieAttributeName(reservedName.toLowerCase(Locale.ROOT)); + assertInvalidCookieAttributeName(reservedName.toUpperCase(Locale.ROOT)); + + for (int i = 0; i < reservedName.length(); i++) { + char[] chars = reservedName.toCharArray(); + char c = chars[i]; + chars[i] = Character.toString(c).toLowerCase(Locale.ROOT).charAt(0); + + String nameWithLowerChar = new String(chars); + assertInvalidCookieAttributeName(nameWithLowerChar); + + chars[i] = Character.toString(c).toUpperCase(Locale.ROOT).charAt(0); + String nameWithUpperChar = new String(chars); + assertInvalidCookieAttributeName(nameWithUpperChar); + } + } + + for (Character c : COOKIE_TOKEN_SEPARATOR_CHARACTERS) { + assertInvalidCookieAttributeName("separator" + c + "name"); + } + for (Character c : COOKIE_VALUE_FORBIDDEN_CHARS) { + assertInvalidCookieAttributeName("separator" + c + "name"); + } + for (Character c : SOME_CONTROL_CHARACTERS) { + assertInvalidCookieAttributeName("control" + c + "name"); + } + // US-ASCII only + assertInvalidCookieAttributeName("cöölName"); + + for (String validToken : VALID_TOKENS) { + cookieForName(validToken); + } + } + + private Cookie cookieForName(String name) { + return new Cookie(validPath, name, VALID_VALUE, validExpiresTime); + } + + private void assertInvalidCookieAttributeName(String name) { + assertThrowsIllegalArgumentException( + "Constructor allows invalid cookie attribute name: '" + name + "'", + () -> cookieForName(name) + ); + } + + @Test + public void testCookieValueParameter() { // Empty values are allowed; - new Cookie(validPath, VALID_NAME, null, validExpiresDate).getValue(); - new Cookie(validPath, VALID_NAME, "", validExpiresDate); - - try { - new Cookie(validPath, VALID_NAME, "\"", validExpiresDate); - fail("Constructor allows invalid characters in value"); - } catch(RuntimeException e) {} - - try { - new Cookie(validPath, VALID_NAME, VALID_VALUE + "ä", validExpiresDate); - fail("Constructor allows non-US-ASCII characters in value"); - } catch(RuntimeException e) {} - - // TODO: Test for more invalid characters in value; - - try { - new Cookie(validPath, VALID_NAME, VALID_VALUE, new Date(CurrentTimeUTC.getInMillis()-1)); - fail("Constructor allows construction of expired cookies."); - } catch(RuntimeException e) {} + assertEquals("", cookieForValue(null).getValue()); + assertEquals("", cookieForValue("").getValue()); + + assertThrowsIllegalArgumentException( + "Constructor allows invalid characters in value", + () -> cookieForValue("\"") + ); + + assertThrowsIllegalArgumentException( + "Constructor allows non-US-ASCII characters in value", + () -> cookieForValue(VALID_VALUE + "ä") + ); + + for (String cookieMember : RESERVED_COOKIE_MEMBERS) { + // allow reserved words as values + cookieForValue(cookieMember); + } + + for (Character c : COOKIE_VALUE_FORBIDDEN_CHARS) { + assertInvalidCookieAttributeValue("separator" + c + "value"); + } + for (Character c : SOME_CONTROL_CHARACTERS) { + assertInvalidCookieAttributeValue("control" + c + "value"); + } + // US-ASCII only + assertInvalidCookieAttributeValue("cöölValue"); + + for (String validToken : VALID_TOKENS) { + new Cookie(validPath, validToken, VALID_VALUE, validExpiresTime); + } + for (String validName : Arrays.asList( + "my name", + " my name " + )) { + cookieForValue(validName); + } + } + + private Cookie cookieForValue(String myValue) { + return new Cookie(validPath, VALID_NAME, myValue, validExpiresTime); + } + + private void assertInvalidCookieAttributeValue(String value) { + assertThrowsIllegalArgumentException( + "Constructor allows invalid cookie attribute value: '" + value + "'", + () -> cookieForValue(value) + ); + } + + @Test + public void testCookieDateParameter() { + assertThrowsIllegalArgumentException( + "Constructor allows construction with null date.", + () -> new Cookie(validPath, VALID_NAME, VALID_VALUE, null) + ); + + new Cookie(validPath, VALID_NAME, VALID_VALUE, Instant.now().minusMillis(-1)); } @Test public void testEqualsObject() throws URISyntaxException { assertEquals(cookie, cookie); - assertEquals(cookie, new Cookie(validPath, VALID_NAME, VALID_VALUE, new Date(CurrentTimeUTC.getInMillis()+60*1000))); + assertEquals(cookie, new Cookie(validPath, VALID_NAME, VALID_VALUE, Instant.now().plusMillis(60*1000))); // Value is not checked in equals(). - assertEquals(cookie, new Cookie(validPath, VALID_NAME, "", new Date(CurrentTimeUTC.getInMillis()+60*1000))); - - assertFalse(cookie.equals(new Cookie(new URI(VALID_PATH.toLowerCase()), VALID_NAME, VALID_VALUE, validExpiresDate))); - assertEquals(cookie, new Cookie(validPath, VALID_NAME.toLowerCase(), VALID_VALUE, validExpiresDate)); - + assertEquals(cookie, new Cookie(validPath, VALID_NAME, "", Instant.now().plusMillis(60*1000))); + + assertNotEquals(cookie, new Cookie(new URI(VALID_PATH.toLowerCase()), VALID_NAME, VALID_VALUE, validExpiresTime)); + assertEquals(cookie, new Cookie(validPath, VALID_NAME.toLowerCase(), VALID_VALUE, validExpiresTime)); + + assertNotEquals(cookie, new Object()); + assertNotEquals(cookieForName("first"), cookieForName("second")); + // TODO: Test domain. This is currently done in ReceivedCookieTest } + @Test + public void testHashCodeMethod() { + MatcherAssert.assertThat(cookie.hashCode(), Matchers.any(Integer.class)); + } + @Test public void testGetDomain() { + assertNull(cookie.getDomain()); // TODO: Implement. } @@ -119,17 +324,39 @@ public void testGetName() { public void testGetValue() { assertEquals(VALID_VALUE, cookie.getValue()); } - -// TODO: getExpirationDate() is commented out because it is broken, see ReceivedCookie.java -// public void testGetExpirationDate() { -// assertEquals(validExpiresDate, cookie.getExpirationDate()); -// } + + @Test + public void testGetExpirationDate() { + assertEquals(validExpiresTime, cookie.getExpirationTime()); + } @Test public void testEncodeToHeaderValue() { - System.out.println(cookie.encodeToHeaderValue()); - - // TODO: Implement. + String headerValue = cookie.encodeToHeaderValue(); + + assertNotNull(headerValue); + assertFalse(headerValue.isEmpty()); + + assertTrue(headerValue.contains(String.format("%s=%s;", VALID_NAME.toLowerCase(), VALID_VALUE))); + assertTrue(headerValue.contains("version=1;")); + assertTrue(headerValue.contains(String.format("path=%s;", validPath.getRawPath()))); + + String expireTimestampStr = DateTimeFormatter.RFC_1123_DATE_TIME.format(validExpiresTime.atZone(ZoneOffset.UTC)); + assertTrue(headerValue.contains(String.format("expires=%s;", expireTimestampStr))); + + assertTrue(headerValue.contains("discard=true;")); } + @Test + public void encodeToHeaderValueCanEncodeValueWithSpaces() { + String strWithSpace = "test space"; + Cookie cookie = cookieForValue(strWithSpace); + assertEquals(strWithSpace, cookie.getValue()); + String encoded = cookie.encodeToHeaderValue(); + assertTrue(encoded.contains(String.format("%s=%s;", VALID_NAME.toLowerCase(), strWithSpace))); + } + + protected static void assertThrowsIllegalArgumentException(String message, ThrowingRunnable runnable) { + assertThrows(message, IllegalArgumentException.class, runnable); + } } diff --git a/test/freenet/clients/http/ReceivedCookieTest.java b/test/freenet/clients/http/ReceivedCookieTest.java index 4ba22156a05..c021adaecbf 100644 --- a/test/freenet/clients/http/ReceivedCookieTest.java +++ b/test/freenet/clients/http/ReceivedCookieTest.java @@ -6,34 +6,31 @@ import static org.junit.Assert.*; import java.text.ParseException; -import java.util.ArrayList; -import java.util.Date; +import java.util.Arrays; +import java.util.List; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; -public class ReceivedCookieTest extends CookieTest { - - static final String validEncodedCookie = " SessionID = \"abCd12345\" ;" - + " $Version = 1 ;" - + " $Path = \"/Freetalk\";" - + " $Discard; " - + " $Expires = \"Sun, 25 Oct 2030 15:09:37 GMT\"; " - + " $blah;"; +public class ReceivedCookieTest { + + private static final String validEncodedCookie = " SessionID = \"abCd12345\" ;" + + " $Version = 1 ;" + + " $Path = \"/Freetalk\";" + + " $Discard; " + + " $Expires = \"Fri, 25 Oct 2030 15:09:37 GMT\"; " + + " $blah;"; + + private static final String VALID_NAME = "SessionID"; + private static final String VALID_VALUE = "abCd12345"; + + private ReceivedCookie cookie; @Before - @SuppressWarnings("deprecation") public void setUp() throws Exception { - super.setUp(); - - validExpiresDate = new Date(2030 - 1900, 10 - 1, 25, 15, 9, 37); - - cookie = ReceivedCookie.parseHeader(validEncodedCookie).get(0); - } - - @Override - public void testGetDomain() { - // TODO: Implement. + cookie = parseHeaderAndGetFirst(validEncodedCookie); } @Test @@ -41,37 +38,89 @@ public void testParseHeader() throws ParseException { // The tests for getPath(), getName() etc will be executed using the parsed mCookie and therefore also test parseHeader() for valid values, // we only need to test special cases here. - ArrayList cookies; - Cookie cookie; + List cookies; + ReceivedCookie cookie; // Plain firefox cookie - - cookie = ReceivedCookie.parseHeader("SessionID=abCd12345").get(0); - assertEquals(VALID_NAME.toLowerCase(), cookie.getName()); assertEquals(VALID_VALUE, cookie.getValue()); + + cookie = parseHeaderAndGetFirst("SessionID=abCd12345"); + assertEquals(VALID_NAME, cookie.getName()); + assertEquals(VALID_VALUE, cookie.getValue()); // Two plain firefox cookies cookies = ReceivedCookie.parseHeader("SessionID=abCd12345;key2=valUe2"); - cookie = cookies.get(0); assertEquals(VALID_NAME.toLowerCase(), cookie.getName()); assertEquals(VALID_VALUE, cookie.getValue()); - cookie = cookies.get(1); assertEquals("key2", cookie.getName()); assertEquals("valUe2", cookie.getValue()); - - // Key without value at end: - - cookie = ReceivedCookie.parseHeader(" SessionID = \"abCd12345\" ;" - + " $blah;").get(0); - assertEquals(VALID_NAME.toLowerCase(), cookie.getName()); assertEquals(VALID_VALUE, cookie.getValue()); - - // Key without value and without semicolon at end - cookie = ReceivedCookie.parseHeader(" SessionID = \"abCd12345\" ;" - + " $blah").get(0); - assertEquals(VALID_NAME.toLowerCase(), cookie.getName()); assertEquals(VALID_VALUE, cookie.getValue()); + cookie = cookies.get(0); + assertEquals(VALID_NAME, cookie.getName()); + assertEquals(VALID_VALUE, cookie.getValue()); + + cookie = cookies.get(1); + assertEquals("key2", cookie.getName()); + assertEquals("valUe2", cookie.getValue()); + } + + @Test + public void canParseKeyOnly() throws ParseException { + ReceivedCookie cookie = parseHeaderAndGetFirst("$blah"); + assertEquals("$blah", cookie.getName()); + assertNull(cookie.getValue()); + } + + @Test + public void canParseKeyWithoutValue() throws ParseException { + List cookies = ReceivedCookie.parseHeader(" SessionID = \"abCd12345\" ; $blah;"); + assertEquals(2, cookies.size()); + + ReceivedCookie cookie = cookies.get(0); + assertEquals(VALID_NAME, cookie.getName()); + assertEquals(VALID_VALUE, cookie.getValue()); + + cookie = cookies.get(1); + assertEquals("$blah", cookie.getName()); + assertNull(cookie.getValue()); + } + + @Test + public void canParseKeyWithoutValueAndSemicolonAtTheEnd() throws ParseException { + List cookies = ReceivedCookie.parseHeader(" SessionID = \"abCd12345\" ; $blah"); + assertEquals(2, cookies.size()); + ReceivedCookie cookie = cookies.get(0); + assertEquals(VALID_NAME, cookie.getName()); + assertEquals(VALID_VALUE, cookie.getValue()); + cookie = cookies.get(1); + assertEquals("$blah", cookie.getName()); + assertNull(cookie.getValue()); + } + + @Test + public void canParseEmptyString() throws ParseException { + for (String empty: Arrays.asList( + null, + "", + " ", + "\n", + "\t", + " \t \n" + )) { + List cookies = ReceivedCookie.parseHeader(empty); + assertNotNull(cookies); + assertEquals(0, cookies.size()); + } + } + + @Test + public void testEqualsMethod() throws ParseException { + assertEquals(cookie, cookie); + assertEquals(parseHeaderAndGetFirst(validEncodedCookie), parseHeaderAndGetFirst(validEncodedCookie)); + assertEquals(cookie, parseHeaderAndGetFirst(validEncodedCookie)); + } + + @Test + public void testHashCodeMethod() { + MatcherAssert.assertThat(cookie.hashCode(), Matchers.any(Integer.class)); } - @Override - public void testEncodeToHeaderValue() { - try { - cookie.encodeToHeaderValue(); - fail("ReceivedCookie.encodeToHeaderValue() should throw UnsupportedOperationException!"); - } catch(UnsupportedOperationException e) {} + private static ReceivedCookie parseHeaderAndGetFirst(String cookieValue) throws ParseException { + return ReceivedCookie.parseHeader(cookieValue).get(0); } }