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 2024/10/20 20:23:25 UTC
[PR] Implement handling of invalid Date header in responses [httpcomponents-core]
arturobernalg opened a new pull request, #500:
URL: https://github.com/apache/httpcomponents-core/pull/500
This pull request optimizes the handling of the Date header in HTTP responses by ensuring that invalid Date headers are correctly replaced with the current time when necessary, following the guidelines in [RFC 9110, Section 6.6.1](https://www.rfc-editor.org/rfc/rfc9110.html#section-6.6.1). This behavior is optional and controlled by the replaceInvalidDate flag; by default, the behavior remains unchanged, preserving the previous logic.
--
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
Re: [PR] Implement handling of invalid Date header in responses [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #500:
URL: https://github.com/apache/httpcomponents-core/pull/500#discussion_r1810132671
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/ResponseDate.java:
##########
@@ -46,27 +46,49 @@
* This interceptor is recommended for the HTTP protocol conformance and
* the correct operation of the server-side message processing pipeline.
* </p>
+ * <p>
+ * If the {@code Date} header is invalid, the interceptor will replace it with the
+ * time at which the response was received.
+ * </p>
*
* @since 4.0
*/
@Contract(threading = ThreadingBehavior.SAFE)
public class ResponseDate implements HttpResponseInterceptor {
+ /**
+ * Indicates whether to replace an invalid Date header.
+ *
+ * @since 5.4
+ */
+ private final boolean replaceInvalidDate;
+
public static final ResponseDate INSTANCE = new ResponseDate();
public ResponseDate() {
+ this(false);
+ }
+
+ /**
+ * Constructs a ResponseDate interceptor.
+ *
+ * @param replaceInvalidDate Whether to replace an invalid {@code Date} header.
+ * If {@code true}, the interceptor will replace any
+ * detected invalid {@code Date} header with a valid value.
+ * @since 5.4
+ */
+ public ResponseDate(final boolean replaceInvalidDate) {
super();
+ this.replaceInvalidDate = replaceInvalidDate;
}
@Override
public void process(final HttpResponse response, final EntityDetails entity, final HttpContext context)
throws HttpException, IOException {
Args.notNull(response, "HTTP response");
- final int status = response.getCode();
- if (status >= HttpStatus.SC_OK &&
- !response.containsHeader(HttpHeaders.DATE)) {
+ final Header dateHeader = response.getFirstHeader(HttpHeaders.DATE);
Review Comment:
@arturobernalg I suppose `replaceInvalidDate` is not the best name now. `alwaysReplace`?
This bit cab be optimized a little. If the `alwaysReplace` flag is true there is no need to do ` response.getFirstHeader(HttpHeaders.DATE)`. Its result is basically ignored. Please re-organize the logical conditions to avoid that.
--
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
Re: [PR] Implement handling of invalid Date header in responses [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #500:
URL: https://github.com/apache/httpcomponents-core/pull/500#issuecomment-2426462442
@arturobernalg Looks unnecessary complex. So what if the `DATE` can be successfully parsed but it represents a point in time when Jesus roamed Galilea? What good that that make? I would either always replace `DATE` with the current date/time without even looking at at the present value.
--
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
Re: [PR] Implement handling of invalid Date header in responses [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #500:
URL: https://github.com/apache/httpcomponents-core/pull/500#issuecomment-2426943351
> @arturobernalg Looks unnecessary complex. So what if the `DATE` can be successfully parsed but it represents a point in time when Jesus roamed Galilea? What good that that make? I would either always replace `DATE` with the current date/time without even looking at at the present value.
@ok2c So, basically, you are suggesting to do this:
```
public void process(final HttpResponse response, final EntityDetails entity, final HttpContext context)
throws HttpException, IOException {
Args.notNull(response, "HTTP response");
final int status = response.getCode();
if (status >= HttpStatus.SC_OK) {
response.setHeader(HttpHeaders.DATE, HttpDateGenerator.INSTANCE.getCurrentDate());
}
}
```
--
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
Re: [PR] Implement handling of invalid Date header in responses [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #500:
URL: https://github.com/apache/httpcomponents-core/pull/500#issuecomment-2427284467
> By the way `status >= HttpStatus.SC_OK` check can be removed, as 1xx intermediate response messages never get passed to the protocol handler along with final responses.
@ok2c I’ve kept the null check for the Date header to ensure that it’s always present in the response
--
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
Re: [PR] Implement handling of invalid Date header in responses [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #500:
URL: https://github.com/apache/httpcomponents-core/pull/500#issuecomment-2427019299
@arturobernalg Yes, pretty much, but with a boolean flag (`alwaysReplace or some such`) like you had before.
By the way `status >= HttpStatus.SC_OK` check can be removed, as 1xx intermediate response messages never get passed to the protocol handler along with final responses.
--
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
Re: [PR] Implement handling of invalid Date header in responses [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #500:
URL: https://github.com/apache/httpcomponents-core/pull/500#discussion_r1810132671
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/ResponseDate.java:
##########
@@ -46,27 +46,49 @@
* This interceptor is recommended for the HTTP protocol conformance and
* the correct operation of the server-side message processing pipeline.
* </p>
+ * <p>
+ * If the {@code Date} header is invalid, the interceptor will replace it with the
+ * time at which the response was received.
+ * </p>
*
* @since 4.0
*/
@Contract(threading = ThreadingBehavior.SAFE)
public class ResponseDate implements HttpResponseInterceptor {
+ /**
+ * Indicates whether to replace an invalid Date header.
+ *
+ * @since 5.4
+ */
+ private final boolean replaceInvalidDate;
+
public static final ResponseDate INSTANCE = new ResponseDate();
public ResponseDate() {
+ this(false);
+ }
+
+ /**
+ * Constructs a ResponseDate interceptor.
+ *
+ * @param replaceInvalidDate Whether to replace an invalid {@code Date} header.
+ * If {@code true}, the interceptor will replace any
+ * detected invalid {@code Date} header with a valid value.
+ * @since 5.4
+ */
+ public ResponseDate(final boolean replaceInvalidDate) {
super();
+ this.replaceInvalidDate = replaceInvalidDate;
}
@Override
public void process(final HttpResponse response, final EntityDetails entity, final HttpContext context)
throws HttpException, IOException {
Args.notNull(response, "HTTP response");
- final int status = response.getCode();
- if (status >= HttpStatus.SC_OK &&
- !response.containsHeader(HttpHeaders.DATE)) {
+ final Header dateHeader = response.getFirstHeader(HttpHeaders.DATE);
Review Comment:
@arturobernalg I suppose `replaceInvalidDate` is not the best name now. `alwaysReplace`?
This bit cab be optimized a bit. If the `alwaysReplace` flat is true there is no need to do ` response.getFirstHeader(HttpHeaders.DATE)`. Its result is basically ignored. Please re-organize the logical conditions to avoid that.
--
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
Re: [PR] Implement handling of invalid Date header in responses [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #500:
URL: https://github.com/apache/httpcomponents-core/pull/500#discussion_r1811122466
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/ResponseDate.java:
##########
@@ -46,27 +46,49 @@
* This interceptor is recommended for the HTTP protocol conformance and
* the correct operation of the server-side message processing pipeline.
* </p>
+ * <p>
+ * If the {@code Date} header is invalid, the interceptor will replace it with the
+ * time at which the response was received.
+ * </p>
*
* @since 4.0
*/
@Contract(threading = ThreadingBehavior.SAFE)
public class ResponseDate implements HttpResponseInterceptor {
+ /**
+ * Indicates whether to replace an invalid Date header.
+ *
+ * @since 5.4
+ */
+ private final boolean replaceInvalidDate;
+
public static final ResponseDate INSTANCE = new ResponseDate();
public ResponseDate() {
+ this(false);
+ }
+
+ /**
+ * Constructs a ResponseDate interceptor.
+ *
+ * @param replaceInvalidDate Whether to replace an invalid {@code Date} header.
+ * If {@code true}, the interceptor will replace any
+ * detected invalid {@code Date} header with a valid value.
+ * @since 5.4
+ */
+ public ResponseDate(final boolean replaceInvalidDate) {
super();
+ this.replaceInvalidDate = replaceInvalidDate;
}
@Override
public void process(final HttpResponse response, final EntityDetails entity, final HttpContext context)
throws HttpException, IOException {
Args.notNull(response, "HTTP response");
- final int status = response.getCode();
- if (status >= HttpStatus.SC_OK &&
- !response.containsHeader(HttpHeaders.DATE)) {
+ final Header dateHeader = response.getFirstHeader(HttpHeaders.DATE);
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
Re: [PR] Implement handling of invalid Date header in responses [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg merged PR #500:
URL: https://github.com/apache/httpcomponents-core/pull/500
--
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