You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "sabberworm (via GitHub)" <gi...@apache.org> on 2026/08/27 10:36:59 UTC
[PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
sabberworm opened a new pull request, #1054:
URL: https://github.com/apache/tomcat/pull/1054
[RFC 9651 (Structured Field Values for HTTP)](https://datatracker.ietf.org/doc/rfc9651/) defines formats for header values of specific types. The format it uses for dates is `@«timestamp»`, which is different from other date headers.
This PR aims to support the date header format from RFC 9651 in `org.apache.catalina.connector.Request#getDateHeader(String)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
Posted by "sabberworm (via GitHub)" <gi...@apache.org>.
sabberworm commented on code in PR #1054:
URL: https://github.com/apache/tomcat/pull/1054#discussion_r3871873897
##########
java/org/apache/tomcat/util/http/FastHttpDateFormat.java:
##########
@@ -146,13 +158,9 @@ public static long parseDate(String value) {
long date = -1;
for (int i = 0; (date == -1) && (i < httpParseFormats.length); i++) {
- try {
- date = httpParseFormats[i].parse(value).getTime();
- updateParseCache(value, Long.valueOf(date));
- } catch (ParseException e) {
- // Ignore
- }
+ date = httpParseFormats[i].tryParseDate(value);
}
+ updateParseCache(value, Long.valueOf(date));
Review Comment:
Now this is also called for values that failed to parse. Not sure if that’s better or worse. IMHO it makes sense to also cache these, since they’re the most expensive to calculate since they have to be rejected by all the parsers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
Posted by "sabberworm (via GitHub)" <gi...@apache.org>.
sabberworm commented on code in PR #1054:
URL: https://github.com/apache/tomcat/pull/1054#discussion_r3871847190
##########
java/org/apache/tomcat/util/http/FastHttpDateFormat.java:
##########
@@ -147,7 +158,7 @@ public static long parseDate(String value) {
long date = -1;
for (int i = 0; (date == -1) && (i < httpParseFormats.length); i++) {
try {
- date = httpParseFormats[i].parse(value).getTime();
+ date = httpParseFormats[i].tryParseDate(value);
updateParseCache(value, Long.valueOf(date));
Review Comment:
Sorry, this is already covered by the `(date == -1)` in the `for` loop.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
Posted by "sabberworm (via GitHub)" <gi...@apache.org>.
sabberworm commented on code in PR #1054:
URL: https://github.com/apache/tomcat/pull/1054#discussion_r3871882503
##########
java/org/apache/tomcat/util/http/FastHttpDateFormat.java:
##########
@@ -185,5 +193,14 @@ private static void updateParseCache(String key, Long value) {
parseCache.put(key, value);
}
-
+ @FunctionalInterface
+ interface TryParseDateToTimestamp {
+ /**
+ * Tries to parse the given string as a date and returns it as a timestamp.
+ *
+ * @param dateString the string representation of the date trying to be parsed
+ * @return the number of *milli*seconds since January 1, 1970, 00:00:00 GMT or -1 if parsing failed
Review Comment:
```suggestion
* @return the number of <em>milli</em>seconds since January 1, 1970, 00:00:00 GMT or -1 if parsing failed
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
Posted by "sabberworm (via GitHub)" <gi...@apache.org>.
sabberworm commented on code in PR #1054:
URL: https://github.com/apache/tomcat/pull/1054#discussion_r3870948522
##########
java/org/apache/tomcat/util/http/FastHttpDateFormat.java:
##########
@@ -147,7 +158,7 @@ public static long parseDate(String value) {
long date = -1;
for (int i = 0; (date == -1) && (i < httpParseFormats.length); i++) {
try {
- date = httpParseFormats[i].parse(value).getTime();
+ date = httpParseFormats[i].tryParseDate(value);
updateParseCache(value, Long.valueOf(date));
Review Comment:
Is this missing a
```suggestion
updateParseCache(value, Long.valueOf(date));
break;
```
?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
Posted by "sabberworm (via GitHub)" <gi...@apache.org>.
sabberworm commented on code in PR #1054:
URL: https://github.com/apache/tomcat/pull/1054#discussion_r3870943636
##########
java/org/apache/tomcat/util/http/FastHttpDateFormat.java:
##########
@@ -60,9 +61,19 @@ private FastHttpDateFormat() {
FORMAT_RFC5322 = new ConcurrentDateFormat(DATE_RFC5322, Locale.US, tz);
FORMAT_OBSOLETE_RFC850 = new ConcurrentDateFormat(DATE_OBSOLETE_RFC850, Locale.US, tz);
FORMAT_OBSOLETE_ASCTIME = new ConcurrentDateFormat(DATE_OBSOLETE_ASCTIME, Locale.US, tz);
+ FORMAT_RFC9651 = dateString -> {
+ if(dateString == null || !dateString.startsWith("@")) {
+ throw new ParseException("Date " + dateString + " not in RFC 9651 format", 0);
+ }
Review Comment:
I don’t like the overhead of using exception handling for control flow here. Maybe use `-1` here too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
Posted by "sabberworm (via GitHub)" <gi...@apache.org>.
sabberworm commented on code in PR #1054:
URL: https://github.com/apache/tomcat/pull/1054#discussion_r3871847190
##########
java/org/apache/tomcat/util/http/FastHttpDateFormat.java:
##########
@@ -147,7 +158,7 @@ public static long parseDate(String value) {
long date = -1;
for (int i = 0; (date == -1) && (i < httpParseFormats.length); i++) {
try {
- date = httpParseFormats[i].parse(value).getTime();
+ date = httpParseFormats[i].tryParseDate(value);
updateParseCache(value, Long.valueOf(date));
Review Comment:
Sorry, this is already covered by the `(date == -1)` condition in the `for` loop.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
Posted by "sabberworm (via GitHub)" <gi...@apache.org>.
sabberworm commented on code in PR #1054:
URL: https://github.com/apache/tomcat/pull/1054#discussion_r3871848925
##########
java/org/apache/tomcat/util/http/FastHttpDateFormat.java:
##########
@@ -60,9 +61,19 @@ private FastHttpDateFormat() {
FORMAT_RFC5322 = new ConcurrentDateFormat(DATE_RFC5322, Locale.US, tz);
FORMAT_OBSOLETE_RFC850 = new ConcurrentDateFormat(DATE_OBSOLETE_RFC850, Locale.US, tz);
FORMAT_OBSOLETE_ASCTIME = new ConcurrentDateFormat(DATE_OBSOLETE_ASCTIME, Locale.US, tz);
+ FORMAT_RFC9651 = dateString -> {
+ if(dateString == null || !dateString.startsWith("@")) {
+ throw new ParseException("Date " + dateString + " not in RFC 9651 format", 0);
+ }
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
Posted by "sabberworm (via GitHub)" <gi...@apache.org>.
sabberworm commented on code in PR #1054:
URL: https://github.com/apache/tomcat/pull/1054#discussion_r3870948522
##########
java/org/apache/tomcat/util/http/FastHttpDateFormat.java:
##########
@@ -147,7 +158,7 @@ public static long parseDate(String value) {
long date = -1;
for (int i = 0; (date == -1) && (i < httpParseFormats.length); i++) {
try {
- date = httpParseFormats[i].parse(value).getTime();
+ date = httpParseFormats[i].tryParseDate(value);
updateParseCache(value, Long.valueOf(date));
Review Comment:
Is this missing a
```suggestion
updateParseCache(value, Long.valueOf(date));
break;
```
?
To me it doesn’t look like any string can ever match more than one format so we’re needlessly trying all of them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
Posted by "sabberworm (via GitHub)" <gi...@apache.org>.
sabberworm commented on code in PR #1054:
URL: https://github.com/apache/tomcat/pull/1054#discussion_r3871873897
##########
java/org/apache/tomcat/util/http/FastHttpDateFormat.java:
##########
@@ -146,13 +158,9 @@ public static long parseDate(String value) {
long date = -1;
for (int i = 0; (date == -1) && (i < httpParseFormats.length); i++) {
- try {
- date = httpParseFormats[i].parse(value).getTime();
- updateParseCache(value, Long.valueOf(date));
- } catch (ParseException e) {
- // Ignore
- }
+ date = httpParseFormats[i].tryParseDate(value);
}
+ updateParseCache(value, Long.valueOf(date));
Review Comment:
Now this is also called for values that failed to parse. Not sure if that’s better or worse. IMHO it makes sense to also cache these, since they’re the most expensive to calculate as they have to be tried (and rejected) by all the parsers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow Request#getDateHeader to parse dates in RFC 9651 Structured Field Values format [tomcat]
Posted by "arturobernalg (via GitHub) via dev" <de...@tomcat.apache.org>.
arturobernalg commented on PR #1054:
URL: https://github.com/apache/tomcat/pull/1054#issuecomment-6719220707
Thanks for working on this. I tried it on a real Tomcat, current main against this branch (merged by hand, because it now conflicts with main), and I think two things need another look.
The shared parser is also used by the DefaultServlet, so conditional requests change. `If-Modified-Since: @4102444800` now gets a 304 and `If-Unmodified-Since: @0` gets a 412, while main ignores both and answers 200. RFC 9110 says a recipient MUST ignore these headers when the value is not a valid HTTP-date. Maybe the `@` form should stay out of `parseDate()` and only be handled where the header is known to be a structured field date.
The value is multiplied by 1000 without a check, so `@9223372036854775807` makes `getDateHeader()` return -1000 instead of throwing. RFC 9651 limits an integer to 15 digits, so longer values (and `@+5`) should be rejected.
The branch also conflicts with #1104 (the rfc850 two-digit year change), so a rebase has to keep that code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org