Conversation
|
@Harshal96 thanks for the contribution, the code is missing the support of HTTP/2 that works differently than with a HTTP header, I recommend you read the ALT-SVC RFC to learn more about this. For that matter you should look at |
|
@vietj done, can you please take a look now? |
|
@vietj is this good for another review? |
| * Options for configuring {@link AltSvcHandler}. | ||
| */ | ||
| @DataObject | ||
| @JsonGen(publicConverter = false) |
There was a problem hiding this comment.
we actually don't need JsonGen annotation here, there is no interested in having a converter for this
| public class AltSvcHandlerImpl implements AltSvcHandler { | ||
|
|
||
| private final Map<String, String> origins; | ||
| private final Map<HttpConnection, Set<String>> announcedOrigins = new WeakHashMap<>(); |
There was a problem hiding this comment.
I think we cannot reasonnably maintain such a map because it will never be cleaned from stale connections, so we need to find a way to do that properly.
There was a problem hiding this comment.
updated to remove stale connections
| origins = new HashSet<>(); | ||
| announcedOrigins.put(connection, origins); | ||
| // Drop the entry when the connection closes, so the map does not retain stale connections. | ||
| connection.closeHandler(v -> { |
There was a problem hiding this comment.
I think for this we would rather need to develop an HTTP connection local storage instead of relying on the close handler, or actually have a close future for the connection
| /** | ||
| * Storage scoped to the lifetime of an {@link HttpConnection}, without retaining connections after they become unreachable. | ||
| */ | ||
| public class HttpConnectionLocal<T> { |
There was a problem hiding this comment.
actually I meant I would add a map like structure on vertx internal HTTP connection object in vertx-core which would solve the issue more directly
There was a problem hiding this comment.
oh, do you want me to support this in https://github.com/eclipse-vertx/vert.x/tree/master/vertx-core before this can be merged?
|
yes in HttpConnectionInternal interface contract
…On Wed, Jul 8, 2026 at 6:22 PM Harshal Parekh ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In vertx-web/src/main/java/io/vertx/ext/web/impl/HttpConnectionLocal.java
<#2910 (comment)>:
> + *
+ * You may elect to redistribute this code under either of these licenses.
+ */
+
+package io.vertx.ext.web.impl;
+
+import io.vertx.core.http.HttpConnection;
+
+import java.util.Map;
+import java.util.WeakHashMap;
+import java.util.function.Supplier;
+
+/**
+ * Storage scoped to the lifetime of an ***@***.*** HttpConnection}, without retaining connections after they become unreachable.
+ */
+public class HttpConnectionLocal<T> {
oh, do you want me to support this in
https://github.com/eclipse-vertx/vert.x/tree/master/vertx-core before
this can be merged?
—
Reply to this email directly, view it on GitHub
<#2910?email_source=notifications&email_token=AABXDCROSZJFHMBWUP3WN3T5DZYKVA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRVGYYDIMBSGE22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#discussion_r3545605797>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABXDCSMJK5CB66IRMKRCI35DZYKVAVCNFSNUABEKJSXA33TNF2G64TZHMZDMNRSHA4TKNB3JFZXG5LFHM2DKNZUGMZDMNRZGWQXMAQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Motivation:
Adds a new
AltSvcHandlerandAltSvcOptionsAPI for advertising HTTP Alternative Services from Vert.x Web routes.This enables applications to declaratively send
Alt-Svcresponse headers for configured origins, which is useful for HTTP/3 discovery. For example:Fixes #2847