You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "arturobernalg (via GitHub)" <gi...@apache.org> on 2023/06/15 20:01:33 UTC
[GitHub] [httpcomponents-client] arturobernalg opened a new pull request, #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
arturobernalg opened a new pull request, #456:
URL: https://github.com/apache/httpcomponents-client/pull/456
This commit enhances the processing and calculation of the "Age" response header field to comply fully with Sections 5.1
and 4.2.3 of RFC9111.
Changes include:
- Parsing of the Age field value as a non-negative integer, representing time in seconds.
- Handling of messages with list-based Age field values, using the first member and discarding subsequent ones.
- Ignoring of invalid field values (e.g., containing something other than a non-negative integer).
- The presence of an Age header field implies that the response was not generated or validated by the origin server for this request.
- Improved/Fixing the age calculation algorithm considering the apparent age, response delay, corrected age value, and current age to align it with the standard.
-
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] arturobernalg commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1233374549
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java:
##########
@@ -206,28 +206,51 @@ protected TimeValue getApparentAge(final HttpCacheEntry entry) {
return TimeValue.ofSeconds(diff.getSeconds());
}
+ /**
+ * Extracts and processes the Age value from an HttpCacheEntry. The method goes through each digit of the Age value,
+ * performing direct arithmetic operations to calculate the total age in delta-seconds. If the Age value cannot be parsed
+ * into a number or if it exceeds Integer.MAX_VALUE, the method returns the value of MAX_AGE in seconds.
+ *
+ * @param entry The HttpCacheEntry from which to extract the Age value.
+ * @return The Age value in delta-seconds, or the MAX_AGE in seconds if the Age value is invalid or exceeds Integer.MAX_VALUE.
+ */
protected long getAgeValue(final HttpCacheEntry entry) {
- // This is a header value, we leave as-is
- long ageValue = 0;
- for (final Header hdr : entry.getHeaders(HttpHeaders.AGE)) {
- long hdrAge;
+ final Header age = entry.getFirstHeader(HttpHeaders.AGE);
+ if (age != null) {
try {
- hdrAge = Long.parseLong(hdr.getValue());
- if (hdrAge < 0) {
- hdrAge = MAX_AGE.toSeconds();
+ final String ageValue = age.getValue();
+ long result = 0;
+ for (int i = 0; i < ageValue.length(); i++) {
+ final int digit = Character.digit(ageValue.charAt(i), 10);
+ if (digit < 0) {
+ if (LOG.isWarnEnabled()) {
+ LOG.warn("Invalid Age header: {}", ageValue);
+ }
+ return 0;
+ }
+ result = result * 10 + digit;
+ if (result > Integer.MAX_VALUE) {
+ if (LOG.isWarnEnabled()) {
Review Comment:
changed.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1233376104
##########
httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java:
##########
@@ -2123,7 +2123,13 @@ public void testAgeHeaderPopulatedFromCacheEntryCurrentAge() throws Exception {
final ClassicHttpResponse result = execute(request);
Assertions.assertEquals(200, result.getCode());
- Assertions.assertEquals("11", result.getFirstHeader("Age").getValue());
+ // We calculate the age of the cache entry as per RFC 9111:
+ // We first find the "corrected_initial_age" which is the maximum of "apparentAge" and "correctedReceivedAge".
+ // In this case, max(1, 2) = 2 seconds.
+ // We then add the "residentTime" which is "now - responseTime",
+ // which is the current time minus the time the cache entry was created. In this case, that is 8 seconds.
+ // So, the total age is "corrected_initial_age" + "residentTime" = 2 + 8 = 10 seconds.
+ Assertions.assertEquals("10", result.getFirstHeader("Age").getValue());
Review Comment:
@arturobernalg Ah, i fixed the matcher in my branch but not in 5.3.x. Please pull the latest changes.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1234472414
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java:
##########
@@ -206,28 +207,62 @@ protected TimeValue getApparentAge(final HttpCacheEntry entry) {
return TimeValue.ofSeconds(diff.getSeconds());
}
+ /**
+ * Extracts and processes the Age value from an HttpCacheEntry by tokenizing the Age header value.
+ * The Age header value is interpreted as a sequence of tokens, and the first token is parsed into a number
+ * representing the age in delta-seconds. If the first token cannot be parsed into a number or if it exceeds
+ * Integer.MAX_VALUE, the Age value is set to MAX_AGE (in seconds).
+ * This method uses CacheSupport.parseTokens to robustly handle the Age header value.
+ * <p>
+ * Note: If the HttpCacheEntry contains multiple Age headers, only the first one is considered.
+ *
+ * @param entry The HttpCacheEntry from which to extract the Age value.
+ * @return The Age value in delta-seconds, or MAX_AGE in seconds if the Age value is invalid or exceeds Integer.MAX_VALUE.
+ * @throws NumberFormatException if the first token in the Age header value cannot be parsed into a number.
+ */
protected long getAgeValue(final HttpCacheEntry entry) {
- // This is a header value, we leave as-is
- long ageValue = 0;
- for (final Header hdr : entry.getHeaders(HttpHeaders.AGE)) {
- long hdrAge;
+ final Header age = entry.getFirstHeader(HttpHeaders.AGE);
+ if (age != null) {
try {
- hdrAge = Long.parseLong(hdr.getValue());
- if (hdrAge < 0) {
- hdrAge = MAX_AGE.toSeconds();
+ final AtomicLong ageValue = new AtomicLong(0);
Review Comment:
@arturobernalg Please use this code to extract the first token. It is massively simpler and is likely more efficient.
```
final AtomicReference<String> firstToken = new AtomicReference<>();
CacheSupport.parseTokens(age, token -> {
firstToken.compareAndSet(null, token);
});
final String s = firstToken.get();
```
If you want to handle delta-seconds `"1" + Long.MAX_VALUE` please do it consistently for all headers, not just `Age`. For now I think accepting only valid long values that can be parsed with `Long#parseLong` should be good enough.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1233386929
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java:
##########
@@ -206,28 +207,60 @@ protected TimeValue getApparentAge(final HttpCacheEntry entry) {
return TimeValue.ofSeconds(diff.getSeconds());
}
+ /**
+ * Extracts and processes the Age value from an HttpCacheEntry by tokenizing the Age header.
+ * Each token is checked, and direct arithmetic operations are performed to calculate the total age in delta-seconds.
+ * If a token cannot be parsed into a number or exceeds Integer.MAX_VALUE, the method sets the Age value to MAX_AGE (in seconds).
+ * This method makes use of CacheSupport.parseTokens to ensure robust handling of the Age header.
+ *
+ * @param entry The HttpCacheEntry from which to extract the Age value.
+ * @return The Age value in delta-seconds, or the MAX_AGE (in seconds) if the Age value is invalid or exceeds Integer.MAX_VALUE.
+ * @throws NumberFormatException if a token in the Age header cannot be parsed into a number.
+ */
protected long getAgeValue(final HttpCacheEntry entry) {
- // This is a header value, we leave as-is
- long ageValue = 0;
- for (final Header hdr : entry.getHeaders(HttpHeaders.AGE)) {
- long hdrAge;
- try {
- hdrAge = Long.parseLong(hdr.getValue());
- if (hdrAge < 0) {
- hdrAge = MAX_AGE.toSeconds();
+ final Header[] ages = entry.getHeaders(HttpHeaders.AGE);
Review Comment:
RFC 9111, 5.1
```
Although it is defined as a singleton header field, a cache encountering a message with a list-based
Age field value SHOULD use the first member of the field value, discarding subsequent ones.
If the field value (after discarding additional members, as per above) is invalid (e.g., it contains
something other than a non-negative integer), a cache SHOULD ignore the field.
```
@arturobernalg This looks wrong. We are supposed to take the first value, not the last one, aren't we? Get the first header and get the first token out of it. That is it.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1233603220
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java:
##########
@@ -206,28 +207,62 @@ protected TimeValue getApparentAge(final HttpCacheEntry entry) {
return TimeValue.ofSeconds(diff.getSeconds());
}
+ /**
+ * Extracts and processes the Age value from an HttpCacheEntry by tokenizing the Age header value.
+ * The Age header value is interpreted as a sequence of tokens, and the first token is parsed into a number
+ * representing the age in delta-seconds. If the first token cannot be parsed into a number or if it exceeds
+ * Integer.MAX_VALUE, the Age value is set to MAX_AGE (in seconds).
+ * This method uses CacheSupport.parseTokens to robustly handle the Age header value.
+ * <p>
+ * Note: If the HttpCacheEntry contains multiple Age headers, only the first one is considered.
+ *
+ * @param entry The HttpCacheEntry from which to extract the Age value.
+ * @return The Age value in delta-seconds, or MAX_AGE in seconds if the Age value is invalid or exceeds Integer.MAX_VALUE.
+ * @throws NumberFormatException if the first token in the Age header value cannot be parsed into a number.
+ */
protected long getAgeValue(final HttpCacheEntry entry) {
- // This is a header value, we leave as-is
- long ageValue = 0;
- for (final Header hdr : entry.getHeaders(HttpHeaders.AGE)) {
- long hdrAge;
+ final Header age = entry.getFirstHeader(HttpHeaders.AGE);
+ if (age != null) {
try {
- hdrAge = Long.parseLong(hdr.getValue());
- if (hdrAge < 0) {
- hdrAge = MAX_AGE.toSeconds();
+ final AtomicLong ageValue = new AtomicLong(0);
Review Comment:
@arturobernalg You are still parsing all tokens instead of one (which is wasteful) and taking the last value from the sequence instead of the first. Please add test cases for various malformed values and make sure they all pass.
It is perfectly fine to use Long#parseLong. It is not going to be any less efficient.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] arturobernalg commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1234426573
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java:
##########
@@ -206,28 +207,62 @@ protected TimeValue getApparentAge(final HttpCacheEntry entry) {
return TimeValue.ofSeconds(diff.getSeconds());
}
+ /**
+ * Extracts and processes the Age value from an HttpCacheEntry by tokenizing the Age header value.
+ * The Age header value is interpreted as a sequence of tokens, and the first token is parsed into a number
+ * representing the age in delta-seconds. If the first token cannot be parsed into a number or if it exceeds
+ * Integer.MAX_VALUE, the Age value is set to MAX_AGE (in seconds).
+ * This method uses CacheSupport.parseTokens to robustly handle the Age header value.
+ * <p>
+ * Note: If the HttpCacheEntry contains multiple Age headers, only the first one is considered.
+ *
+ * @param entry The HttpCacheEntry from which to extract the Age value.
+ * @return The Age value in delta-seconds, or MAX_AGE in seconds if the Age value is invalid or exceeds Integer.MAX_VALUE.
+ * @throws NumberFormatException if the first token in the Age header value cannot be parsed into a number.
+ */
protected long getAgeValue(final HttpCacheEntry entry) {
- // This is a header value, we leave as-is
- long ageValue = 0;
- for (final Header hdr : entry.getHeaders(HttpHeaders.AGE)) {
- long hdrAge;
+ final Header age = entry.getFirstHeader(HttpHeaders.AGE);
+ if (age != null) {
try {
- hdrAge = Long.parseLong(hdr.getValue());
- if (hdrAge < 0) {
- hdrAge = MAX_AGE.toSeconds();
+ final AtomicLong ageValue = new AtomicLong(0);
Review Comment:
@ok2c
Should be fine now.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c merged pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c merged PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] arturobernalg commented on pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#issuecomment-1596269768
> @arturobernalg What happens when you get `Age: 123crap` header?
>
> It is great that you are trying to make your code efficient but in this case it is getting a bit too complex. Please use the support methods from `CacheSupport` I just added, parser the header into a sequence of tokens, take the first token and try to convert into an int.
> @arturobernalg What happens when you get `Age: 123crap` header?
>
> It is great that you are trying to make your code efficient but in this case it is getting a bit too complex. Please use the support methods from `CacheSupport` I just added, parser the header into a sequence of tokens, take the first token and try to convert into an int.
wait. I dint see this comment./.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1233310473
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java:
##########
@@ -206,28 +206,51 @@ protected TimeValue getApparentAge(final HttpCacheEntry entry) {
return TimeValue.ofSeconds(diff.getSeconds());
}
+ /**
+ * Extracts and processes the Age value from an HttpCacheEntry. The method goes through each digit of the Age value,
+ * performing direct arithmetic operations to calculate the total age in delta-seconds. If the Age value cannot be parsed
+ * into a number or if it exceeds Integer.MAX_VALUE, the method returns the value of MAX_AGE in seconds.
+ *
+ * @param entry The HttpCacheEntry from which to extract the Age value.
+ * @return The Age value in delta-seconds, or the MAX_AGE in seconds if the Age value is invalid or exceeds Integer.MAX_VALUE.
+ */
protected long getAgeValue(final HttpCacheEntry entry) {
- // This is a header value, we leave as-is
- long ageValue = 0;
- for (final Header hdr : entry.getHeaders(HttpHeaders.AGE)) {
- long hdrAge;
+ final Header age = entry.getFirstHeader(HttpHeaders.AGE);
+ if (age != null) {
try {
- hdrAge = Long.parseLong(hdr.getValue());
- if (hdrAge < 0) {
- hdrAge = MAX_AGE.toSeconds();
+ final String ageValue = age.getValue();
+ long result = 0;
+ for (int i = 0; i < ageValue.length(); i++) {
+ final int digit = Character.digit(ageValue.charAt(i), 10);
+ if (digit < 0) {
+ if (LOG.isWarnEnabled()) {
+ LOG.warn("Invalid Age header: {}", ageValue);
+ }
+ return 0;
+ }
+ result = result * 10 + digit;
+ if (result > Integer.MAX_VALUE) {
+ if (LOG.isWarnEnabled()) {
Review Comment:
@arturobernalg Please reduce priority of those logs to DEBUG. There is nothing more annoying than a noisy low level transport library in a huge application.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] arturobernalg commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1233380617
##########
httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java:
##########
@@ -2123,7 +2123,13 @@ public void testAgeHeaderPopulatedFromCacheEntryCurrentAge() throws Exception {
final ClassicHttpResponse result = execute(request);
Assertions.assertEquals(200, result.getCode());
- Assertions.assertEquals("11", result.getFirstHeader("Age").getValue());
+ // We calculate the age of the cache entry as per RFC 9111:
+ // We first find the "corrected_initial_age" which is the maximum of "apparentAge" and "correctedReceivedAge".
+ // In this case, max(1, 2) = 2 seconds.
+ // We then add the "residentTime" which is "now - responseTime",
+ // which is the current time minus the time the cache entry was created. In this case, that is 8 seconds.
+ // So, the total age is "corrected_initial_age" + "residentTime" = 2 + 8 = 10 seconds.
+ Assertions.assertEquals("10", result.getFirstHeader("Age").getValue());
Review Comment:
@ok2c Change applied.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1235592336
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java:
##########
@@ -206,28 +207,53 @@ protected TimeValue getApparentAge(final HttpCacheEntry entry) {
return TimeValue.ofSeconds(diff.getSeconds());
}
+ /**
+ * Extracts and processes the Age value from an HttpCacheEntry by tokenizing the Age header value.
+ * The Age header value is interpreted as a sequence of tokens, and the first token is parsed into a number
+ * representing the age in delta-seconds. If the first token cannot be parsed into a number, the Age value is
+ * considered as invalid and this method returns 0. If the first token represents a negative number or a number
+ * that exceeds Integer.MAX_VALUE, the Age value is set to MAX_AGE (in seconds).
+ * This method uses CacheSupport.parseTokens to robustly handle the Age header value.
+ * <p>
+ * Note: If the HttpCacheEntry contains multiple Age headers, only the first one is considered.
+ *
+ * @param entry The HttpCacheEntry from which to extract the Age value.
+ * @return The Age value in delta-seconds, or MAX_AGE in seconds if the Age value exceeds Integer.MAX_VALUE or
+ * is negative. If the Age value is invalid (cannot be parsed into a number or contains non-numeric characters),
+ * this method returns 0.
+ */
protected long getAgeValue(final HttpCacheEntry entry) {
- // This is a header value, we leave as-is
- long ageValue = 0;
- for (final Header hdr : entry.getHeaders(HttpHeaders.AGE)) {
- long hdrAge;
+ final Header age = entry.getFirstHeader(HttpHeaders.AGE);
+ if (age != null) {
try {
- hdrAge = Long.parseLong(hdr.getValue());
- if (hdrAge < 0) {
- hdrAge = MAX_AGE.toSeconds();
+ final AtomicReference<String> firstToken = new AtomicReference<>();
+ CacheSupport.parseTokens(age, token -> firstToken.compareAndSet(null, token));
+ final String s = firstToken.get();
+ if (s != null) {
+ long ageValue = Long.parseLong(s);
+ if (ageValue < 0) {
+ ageValue = 0; // Handle negative age values as invalid
+ } else if (ageValue > Integer.MAX_VALUE) {
+ ageValue = MAX_AGE.toSeconds();
+ }
+ return ageValue;
+ }
+ } catch (final NumberFormatException ex) {
+ if (LOG.isWarnEnabled()) {
Review Comment:
@arturobernalg Looks very good now. Please do reduce this one to DEBUG priority.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] arturobernalg commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1233374525
##########
httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java:
##########
@@ -2123,7 +2123,13 @@ public void testAgeHeaderPopulatedFromCacheEntryCurrentAge() throws Exception {
final ClassicHttpResponse result = execute(request);
Assertions.assertEquals(200, result.getCode());
- Assertions.assertEquals("11", result.getFirstHeader("Age").getValue());
+ // We calculate the age of the cache entry as per RFC 9111:
+ // We first find the "corrected_initial_age" which is the maximum of "apparentAge" and "correctedReceivedAge".
+ // In this case, max(1, 2) = 2 seconds.
+ // We then add the "residentTime" which is "now - responseTime",
+ // which is the current time minus the time the cache entry was created. In this case, that is 8 seconds.
+ // So, the total age is "corrected_initial_age" + "residentTime" = 2 + 8 = 10 seconds.
+ Assertions.assertEquals("10", result.getFirstHeader("Age").getValue());
Review Comment:
@ok2c
The current implementation of ContainsHeaderMatcher does not directly support the object types we're working with here, such as ClassicHttpResponse. Adapting it to these types may require significant changes to the existing code. Maybe I'm missing something.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#issuecomment-1596158581
@arturobernalg The way I read `RFC 9111 5.1` the client would be able to handle `Age` values like `123, 123, 0` or `123, 123, crap` and take the first value from the token list as long as it is valid.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] arturobernalg commented on a diff in pull request #456: HTTPCLIENT-2277: Improve Age Header Handling and Calculation in Accordance with RFC9111.
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #456:
URL: https://github.com/apache/httpcomponents-client/pull/456#discussion_r1234994878
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java:
##########
@@ -206,28 +207,62 @@ protected TimeValue getApparentAge(final HttpCacheEntry entry) {
return TimeValue.ofSeconds(diff.getSeconds());
}
+ /**
+ * Extracts and processes the Age value from an HttpCacheEntry by tokenizing the Age header value.
+ * The Age header value is interpreted as a sequence of tokens, and the first token is parsed into a number
+ * representing the age in delta-seconds. If the first token cannot be parsed into a number or if it exceeds
+ * Integer.MAX_VALUE, the Age value is set to MAX_AGE (in seconds).
+ * This method uses CacheSupport.parseTokens to robustly handle the Age header value.
+ * <p>
+ * Note: If the HttpCacheEntry contains multiple Age headers, only the first one is considered.
+ *
+ * @param entry The HttpCacheEntry from which to extract the Age value.
+ * @return The Age value in delta-seconds, or MAX_AGE in seconds if the Age value is invalid or exceeds Integer.MAX_VALUE.
+ * @throws NumberFormatException if the first token in the Age header value cannot be parsed into a number.
+ */
protected long getAgeValue(final HttpCacheEntry entry) {
- // This is a header value, we leave as-is
- long ageValue = 0;
- for (final Header hdr : entry.getHeaders(HttpHeaders.AGE)) {
- long hdrAge;
+ final Header age = entry.getFirstHeader(HttpHeaders.AGE);
+ if (age != null) {
try {
- hdrAge = Long.parseLong(hdr.getValue());
- if (hdrAge < 0) {
- hdrAge = MAX_AGE.toSeconds();
+ final AtomicLong ageValue = new AtomicLong(0);
Review Comment:
@ok2c
Changed
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org