You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/11/05 18:34:43 UTC

[GitHub] [qpid-dispatch] jiridanek opened a new pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

jiridanek opened a new pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425


   Unit tests and related changes for the log message amount to much of the bulk of this commit.
   
   * Old g++ (RHEL 7) does not properly implement regexes
   * On Ubuntu and rarely Fedora, and all other modern Linuxes, the qd_lws_listener_free is sometimes not called in my test. This is resolved with the .finalize, available only in LibWebSockets 3.1 (RHEL 7 has version 3.0.1)


-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] jiridanek commented on pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#issuecomment-974704700


   Tsan failure in GHA is known issue https://issues.apache.org/jira/browse/PROTON-2436. Next, on Travis the known issue https://issues.apache.org/jira/browse/DISPATCH-1689 is manifesting in VhostPolicyConfigHashPattern test. Also, there are failing `pip install` commands on Travis, probably due to 3rd party dependencies not installing correctly. Finally, the AMQP listener port 0 test is failing on macOS.
   
   I'm going to merge this PR despite all 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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#discussion_r753696907



##########
File path: src/http-libwebsockets.c
##########
@@ -374,25 +380,38 @@ static void listener_start(qd_lws_listener_t *hl, qd_http_server_t *hs) {
 
         info.options |=
             LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT |
-#if LWS_LIBRARY_VERSION_MAJOR > 3 || (LWS_LIBRARY_VERSION_MAJOR == 3 && LWS_LIBRARY_VERSION_MINOR >= 2)
+#ifdef QD_HAVE_MODERN_LIBWEBSOCKETS
             (config->ssl_required ? 0 : LWS_SERVER_OPTION_ALLOW_NON_SSL_ON_SSL_PORT | LWS_SERVER_OPTION_ALLOW_HTTP_ON_HTTPS_LISTENER) |
 #else
             (config->ssl_required ? 0 : LWS_SERVER_OPTION_ALLOW_NON_SSL_ON_SSL_PORT) |
 #endif
             ((config->requireAuthentication && info.ssl_ca_filepath) ? LWS_SERVER_OPTION_REQUIRE_VALID_OPENSSL_CLIENT_CERT : 0);
     }
     info.vhost_name = hl->listener->config.host_port;
+#ifdef QD_HAVE_MODERN_LIBWEBSOCKETS
+    info.finalize = finalize_http;
+    info.finalize_arg = hl;
+#endif
     hl->vhost = lws_create_vhost(hs->context, &info);
-    if (hl->vhost) {
-        /* Store hl pointer in vhost */
-        void *vp = lws_protocol_vh_priv_zalloc(hl->vhost, &protocols[0], sizeof(hl));
-        memcpy(vp, &hl, sizeof(hl));
-        qd_log(hs->log, QD_LOG_NOTICE, "Listening for HTTP on %s", config->host_port);
-        return;
-    } else {
+    if (!hl->vhost) {
         qd_log(hs->log, QD_LOG_NOTICE, "Error listening for HTTP on %s", config->host_port);
         goto error;
     }
+
+    /* Store hl pointer in vhost */
+    void *vp = lws_protocol_vh_priv_zalloc(hl->vhost, &protocols[0], sizeof(hl));
+    memcpy(vp, &hl, sizeof(hl));
+
+    if (port == 0) {
+        // If a 0 (zero) is specified for a port, get the actual listening port from the listener.
+        const int resolved_port = lws_get_vhost_port(hl->vhost);

Review comment:
       The vhost is started successfully at this point, so there had to be an actual port number assigned to it at some point. I'm adding the assert.




-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#discussion_r753696907



##########
File path: src/http-libwebsockets.c
##########
@@ -374,25 +380,38 @@ static void listener_start(qd_lws_listener_t *hl, qd_http_server_t *hs) {
 
         info.options |=
             LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT |
-#if LWS_LIBRARY_VERSION_MAJOR > 3 || (LWS_LIBRARY_VERSION_MAJOR == 3 && LWS_LIBRARY_VERSION_MINOR >= 2)
+#ifdef QD_HAVE_MODERN_LIBWEBSOCKETS
             (config->ssl_required ? 0 : LWS_SERVER_OPTION_ALLOW_NON_SSL_ON_SSL_PORT | LWS_SERVER_OPTION_ALLOW_HTTP_ON_HTTPS_LISTENER) |
 #else
             (config->ssl_required ? 0 : LWS_SERVER_OPTION_ALLOW_NON_SSL_ON_SSL_PORT) |
 #endif
             ((config->requireAuthentication && info.ssl_ca_filepath) ? LWS_SERVER_OPTION_REQUIRE_VALID_OPENSSL_CLIENT_CERT : 0);
     }
     info.vhost_name = hl->listener->config.host_port;
+#ifdef QD_HAVE_MODERN_LIBWEBSOCKETS
+    info.finalize = finalize_http;
+    info.finalize_arg = hl;
+#endif
     hl->vhost = lws_create_vhost(hs->context, &info);
-    if (hl->vhost) {
-        /* Store hl pointer in vhost */
-        void *vp = lws_protocol_vh_priv_zalloc(hl->vhost, &protocols[0], sizeof(hl));
-        memcpy(vp, &hl, sizeof(hl));
-        qd_log(hs->log, QD_LOG_NOTICE, "Listening for HTTP on %s", config->host_port);
-        return;
-    } else {
+    if (!hl->vhost) {
         qd_log(hs->log, QD_LOG_NOTICE, "Error listening for HTTP on %s", config->host_port);
         goto error;
     }
+
+    /* Store hl pointer in vhost */
+    void *vp = lws_protocol_vh_priv_zalloc(hl->vhost, &protocols[0], sizeof(hl));
+    memcpy(vp, &hl, sizeof(hl));
+
+    if (port == 0) {
+        // If a 0 (zero) is specified for a port, get the actual listening port from the listener.
+        const int resolved_port = lws_get_vhost_port(hl->vhost);

Review comment:
       The vhost is started successfully at this point, so there had to be an actual port number assigned to it at some point. I'm adding the assert.




-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] jiridanek commented on pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#issuecomment-974704700


   Tsan failure in GHA is known issue https://issues.apache.org/jira/browse/PROTON-2436. Next, on Travis the known issue https://issues.apache.org/jira/browse/DISPATCH-1689 is manifesting in VhostPolicyConfigHashPattern test. Also, there are failing `pip install` commands on Travis, probably due to 3rd party dependencies not installing correctly. Finally, the AMQP listener port 0 test is failing on macOS.
   
   I'm going to merge this PR despite all 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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] jiridanek merged pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
jiridanek merged pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425


   


-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] codecov-commenter edited a comment on pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#issuecomment-962281395


   # [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1425](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (611a8c0) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/ef3e34cfa9b4a8ffbbd513a22bd5f70cac2fab89?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ef3e34c) will **decrease** coverage by `0.03%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/graphs/tree.svg?width=650&height=150&src=pr&token=rk2Cgd27pP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##             main    #1425      +/-   ##
   ==========================================
   - Coverage   84.77%   84.73%   -0.04%     
   ==========================================
     Files         116      116              
     Lines       28619    28618       -1     
   ==========================================
   - Hits        24262    24250      -12     
   - Misses       4357     4368      +11     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [src/server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3NlcnZlci5j) | `87.38% <100.00%> (+1.15%)` | :arrow_up: |
   | [...router\_core/modules/edge\_router/link\_route\_proxy.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvbGlua19yb3V0ZV9wcm94eS5j) | `78.69% <0.00%> (-4.15%)` | :arrow_down: |
   | [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `88.96% <0.00%> (-1.25%)` | :arrow_down: |
   | [src/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvZWRnZV9tZ210LmM=) | `84.15% <0.00%> (-1.00%)` | :arrow_down: |
   | [src/adaptors/tcp\_adaptor.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL3RjcF9hZGFwdG9yLmM=) | `77.08% <0.00%> (-0.32%)` | :arrow_down: |
   | [src/adaptors/http1/http1\_server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL2h0dHAxL2h0dHAxX3NlcnZlci5j) | `85.73% <0.00%> (-0.28%)` | :arrow_down: |
   | [src/message.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21lc3NhZ2UuYw==) | `87.49% <0.00%> (ø)` | |
   | [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=) | `93.55% <0.00%> (+0.09%)` | :arrow_up: |
   | [src/parse.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3BhcnNlLmM=) | `88.15% <0.00%> (+0.19%)` | :arrow_up: |
   | [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `94.38% <0.00%> (+0.21%)` | :arrow_up: |
   | ... and [2 more](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ef3e34c...611a8c0](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] codecov-commenter edited a comment on pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#issuecomment-962281395


   # [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1425](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (63c60ec) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/ef3e34cfa9b4a8ffbbd513a22bd5f70cac2fab89?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ef3e34c) will **decrease** coverage by `0.03%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/graphs/tree.svg?width=650&height=150&src=pr&token=rk2Cgd27pP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##             main    #1425      +/-   ##
   ==========================================
   - Coverage   84.77%   84.74%   -0.04%     
   ==========================================
     Files         116      116              
     Lines       28619    28619              
   ==========================================
   - Hits        24262    24252      -10     
   - Misses       4357     4367      +10     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [src/server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3NlcnZlci5j) | `87.38% <100.00%> (+1.15%)` | :arrow_up: |
   | [...router\_core/modules/edge\_router/link\_route\_proxy.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvbGlua19yb3V0ZV9wcm94eS5j) | `78.69% <0.00%> (-4.15%)` | :arrow_down: |
   | [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `89.15% <0.00%> (-1.06%)` | :arrow_down: |
   | [src/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvZWRnZV9tZ210LmM=) | `84.15% <0.00%> (-1.00%)` | :arrow_down: |
   | [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `93.73% <0.00%> (-0.44%)` | :arrow_down: |
   | [src/adaptors/http1/http1\_server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL2h0dHAxL2h0dHAxX3NlcnZlci5j) | `85.73% <0.00%> (-0.28%)` | :arrow_down: |
   | [src/message.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21lc3NhZ2UuYw==) | `87.35% <0.00%> (-0.14%)` | :arrow_down: |
   | [src/router\_core/delivery.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2RlbGl2ZXJ5LmM=) | `94.08% <0.00%> (+0.18%)` | :arrow_up: |
   | [src/parse.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3BhcnNlLmM=) | `88.18% <0.00%> (+0.21%)` | :arrow_up: |
   | ... and [2 more](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ef3e34c...63c60ec](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#discussion_r744308899



##########
File path: tests/c_unittests/test_listener_startup.cpp
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "./qdr_doctest.hpp"
+#include "./helpers.hpp"  // must come after ./qdr_doctest.hpp
+
+#include <proton/listener.h>
+
+#include <regex>
+#include <thread>
+
+extern "C" {
+qd_listener_t *qd_dispatch_configure_listener(qd_dispatch_t *qd, qd_entity_t *entity);
+void qd_connection_manager_delete_listener(qd_dispatch_t *qd, void *impl);
+}
+
+
+/// GCC 4.8 made a questionable choice to implement std::regex_search to always
+/// return false. Meaning that tests cannot use regex on RHEL 7
+static bool regex_is_broken() {
+    return !std::regex_search("", std::regex(""));
+}
+
+void check_amqp_listener_startup_log_message(qd_server_config_t config, std::string listen, std::string stop)
+{
+    QDR qdr{};
+    CaptureCStream css(&stderr);
+    qdr.initialize("./minimal_trace.conf");
+
+    qd_listener_t *li = qd_server_listener(qdr.qd->server);
+    li->config = config;
+
+    CHECK(qd_listener_listen(li));
+    pn_listener_close(li->pn_listener);
+    {
+        /* AMQP socket is opened (and closed) only when proactor loop runs; meaning router has to be started */
+        auto timer = qdr.schedule_stop(0);
+        qdr.run();
+    }
+
+    qd_listener_decref(li);
+    qdr.deinitialize();
+
+    std::string logging = css.str();
+    CHECK_MESSAGE(std::regex_search(logging, std::regex(listen)),
+                  listen, " not found in ", logging);
+    CHECK_MESSAGE(std::regex_search(logging, std::regex(stop)),
+                  stop, " not found in ", logging);
+}
+
+void check_http_listener_startup_log_message(qd_server_config_t config, std::string listen, std::string stop, std::string failed)
+{
+    QDR qdr{};
+    CaptureCStream css(&stderr);
+    qdr.initialize("./minimal_trace.conf");
+
+    qd_listener_t *li = qd_server_listener(qdr.qd->server);
+    li->config = config;
+
+    const bool http_supported = qd_server_http(qdr.qd->server) != nullptr;
+
+    CHECK(qd_listener_listen(li) == http_supported);
+    qdr.wait();
+    qd_lws_listener_close(li->http);
+    qd_listener_decref(li);
+    {
+        auto timer = qdr.schedule_stop(0);
+        qdr.run();
+    }
+
+    qdr.deinitialize();
+
+    std::string logging = css.str();
+    CHECK_MESSAGE((logging.find("SERVER (warning) HTTP support is not available") == std::string::npos) == http_supported,
+                  listen, " (not) found in ", logging);
+
+    CHECK_MESSAGE(std::regex_search(logging, std::regex(listen)) == http_supported,
+                  listen, " (not) found in ", logging);
+    CHECK_MESSAGE(std::regex_search(logging, std::regex(stop)) == http_supported,
+                  stop, " (not) found in ", logging);
+
+    CHECK_MESSAGE(std::regex_search(logging, std::regex(failed)) != http_supported,
+                  stop, " (not) found in ", logging);
+
+}
+
+TEST_CASE("Start AMQP listener with zero port" * doctest::skip(regex_is_broken()))
+{
+    std::thread([] {
+        qd_server_config_t config{};
+        config.port      = strdup("0");
+        config.host      = strdup("localhost");
+        config.host_port = strdup("localhost:0");
+
+        check_amqp_listener_startup_log_message(
+            config,
+            R"EOS(SERVER \(notice\) Listening on (127.0.0.1)|(::1):(\d\d+))EOS",
+            R"EOS(SERVER \(trace\) Listener closed on localhost:0)EOS"
+        );
+    }).join();
+}
+
+TEST_CASE("Start AMQP listener with zero port and a name" * doctest::skip(regex_is_broken()))
+{
+    std::thread([] {
+        qd_server_config_t config{};
+        config.name      = strdup("pepa");
+        config.port      = strdup("0");
+        config.host      = strdup("localhost");
+        config.host_port = strdup("localhost:0");
+
+        check_amqp_listener_startup_log_message(
+            config,
+            R"EOS(SERVER \(notice\) Listening on (127.0.0.1)|(::1):(\d\d+) \(pepa\))EOS",
+            R"EOS(SERVER \(trace\) Listener closed on localhost:0)EOS"
+        );
+    }).join();
+}
+
+TEST_CASE("Start HTTP listener with zero port" * doctest::skip(regex_is_broken()))
+{
+    std::thread([] {
+        qd_server_config_t config{};
+        config.port      = strdup("0");
+        config.host      = strdup("localhost");
+        config.host_port = strdup("localhost:0");
+        config.http      = true;
+
+        check_http_listener_startup_log_message(
+            config,
+            R"EOS(SERVER \(notice\) Listening for HTTP on localhost:(\d\d+))EOS",
+            R"EOS(SERVER \(notice\) Stopped listening for HTTP on localhost:0)EOS",
+
+            R"EOS(SERVER \(error\) No HTTP support to listen on localhost:0)EOS"
+        );
+    }).join();
+}
+
+TEST_CASE("Start HTTP listener with zero port and a name" * doctest::skip(regex_is_broken()))
+{
+    std::thread([] {
+        qd_server_config_t config{};
+        config.name      = strdup("pepa");
+        config.port      = strdup("0");
+        config.host      = strdup("localhost");
+        config.host_port = strdup("localhost:0");
+        config.http      = true;
+
+        check_http_listener_startup_log_message(
+            config,
+            R"EOS(SERVER \(notice\) Listening for HTTP on localhost:(\d\d+))EOS",

Review comment:
       ```suggestion
               R"EOS(SERVER \(notice\) Listening for HTTP on localhost:(\d\d+) \(pepa\))EOS",
   ```




-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] kgiusti commented on a change in pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#discussion_r753373851



##########
File path: src/http-libwebsockets.c
##########
@@ -374,25 +380,38 @@ static void listener_start(qd_lws_listener_t *hl, qd_http_server_t *hs) {
 
         info.options |=
             LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT |
-#if LWS_LIBRARY_VERSION_MAJOR > 3 || (LWS_LIBRARY_VERSION_MAJOR == 3 && LWS_LIBRARY_VERSION_MINOR >= 2)
+#ifdef QD_HAVE_MODERN_LIBWEBSOCKETS
             (config->ssl_required ? 0 : LWS_SERVER_OPTION_ALLOW_NON_SSL_ON_SSL_PORT | LWS_SERVER_OPTION_ALLOW_HTTP_ON_HTTPS_LISTENER) |
 #else
             (config->ssl_required ? 0 : LWS_SERVER_OPTION_ALLOW_NON_SSL_ON_SSL_PORT) |
 #endif
             ((config->requireAuthentication && info.ssl_ca_filepath) ? LWS_SERVER_OPTION_REQUIRE_VALID_OPENSSL_CLIENT_CERT : 0);
     }
     info.vhost_name = hl->listener->config.host_port;
+#ifdef QD_HAVE_MODERN_LIBWEBSOCKETS
+    info.finalize = finalize_http;
+    info.finalize_arg = hl;
+#endif
     hl->vhost = lws_create_vhost(hs->context, &info);
-    if (hl->vhost) {
-        /* Store hl pointer in vhost */
-        void *vp = lws_protocol_vh_priv_zalloc(hl->vhost, &protocols[0], sizeof(hl));
-        memcpy(vp, &hl, sizeof(hl));
-        qd_log(hs->log, QD_LOG_NOTICE, "Listening for HTTP on %s", config->host_port);
-        return;
-    } else {
+    if (!hl->vhost) {
         qd_log(hs->log, QD_LOG_NOTICE, "Error listening for HTTP on %s", config->host_port);
         goto error;
     }
+
+    /* Store hl pointer in vhost */
+    void *vp = lws_protocol_vh_priv_zalloc(hl->vhost, &protocols[0], sizeof(hl));
+    memcpy(vp, &hl, sizeof(hl));
+
+    if (port == 0) {
+        // If a 0 (zero) is specified for a port, get the actual listening port from the listener.
+        const int resolved_port = lws_get_vhost_port(hl->vhost);

Review comment:
       Any chance this can fail?  Docs say lws_get_vhost_port() can return -1 on error.  If we don't expect it to return -1 add an assert(resolved_port != -1) in case we break the code at some point.




-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] codecov-commenter edited a comment on pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#issuecomment-962281395






-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] jiridanek merged pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
jiridanek merged pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425


   


-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] codecov-commenter edited a comment on pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#issuecomment-962281395


   # [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1425](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3bc4080) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/f14a4b8d14f9c1280461ee43c306b0866f09535d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f14a4b8) will **decrease** coverage by `0.05%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/graphs/tree.svg?width=650&height=150&src=pr&token=rk2Cgd27pP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##             main    #1425      +/-   ##
   ==========================================
   - Coverage   84.78%   84.73%   -0.06%     
   ==========================================
     Files         116      116              
     Lines       28613    28613              
   ==========================================
   - Hits        24259    24244      -15     
   - Misses       4354     4369      +15     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [src/server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3NlcnZlci5j) | `87.38% <100.00%> (+1.15%)` | :arrow_up: |
   | [src/adaptors/tcp\_adaptor.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL3RjcF9hZGFwdG9yLmM=) | `77.32% <0.00%> (-1.80%)` | :arrow_down: |
   | [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `88.96% <0.00%> (-0.87%)` | :arrow_down: |
   | [src/router\_core/modules/mobile\_sync/mobile.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvbW9iaWxlX3N5bmMvbW9iaWxlLmM=) | `92.36% <0.00%> (-0.24%)` | :arrow_down: |
   | [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=) | `93.45% <0.00%> (-0.20%)` | :arrow_down: |
   | [src/message.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21lc3NhZ2UuYw==) | `87.35% <0.00%> (-0.14%)` | :arrow_down: |
   | [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `94.38% <0.00%> (+0.86%)` | :arrow_up: |
   | [src/http-none.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2h0dHAtbm9uZS5j) | `62.50% <0.00%> (+12.50%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f14a4b8...3bc4080](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] codecov-commenter commented on pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#issuecomment-962281395


   # [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1425](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6c0a590) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/f14a4b8d14f9c1280461ee43c306b0866f09535d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f14a4b8) will **decrease** coverage by `0.02%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/graphs/tree.svg?width=650&height=150&src=pr&token=rk2Cgd27pP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##             main    #1425      +/-   ##
   ==========================================
   - Coverage   84.78%   84.75%   -0.03%     
   ==========================================
     Files         116      116              
     Lines       28613    28613              
   ==========================================
   - Hits        24259    24252       -7     
   - Misses       4354     4361       +7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [src/server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3NlcnZlci5j) | `87.38% <100.00%> (+1.15%)` | :arrow_up: |
   | [src/adaptors/tcp\_adaptor.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL3RjcF9hZGFwdG9yLmM=) | `77.00% <0.00%> (-2.11%)` | :arrow_down: |
   | [src/router\_core/router\_core.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlcl9jb3JlLmM=) | `86.09% <0.00%> (-0.95%)` | :arrow_down: |
   | [src/router\_core/delivery.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2RlbGl2ZXJ5LmM=) | `93.34% <0.00%> (-0.56%)` | :arrow_down: |
   | [src/router\_core/modules/mobile\_sync/mobile.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvbW9iaWxlX3N5bmMvbW9iaWxlLmM=) | `92.36% <0.00%> (-0.24%)` | :arrow_down: |
   | [src/message.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21lc3NhZ2UuYw==) | `87.35% <0.00%> (-0.14%)` | :arrow_down: |
   | [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=) | `93.65% <0.00%> (ø)` | |
   | [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `89.92% <0.00%> (+0.09%)` | :arrow_up: |
   | [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `94.38% <0.00%> (+0.86%)` | :arrow_up: |
   | [src/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvZWRnZV9tZ210LmM=) | `85.14% <0.00%> (+0.99%)` | :arrow_up: |
   | ... and [3 more](https://codecov.io/gh/apache/qpid-dispatch/pull/1425/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f14a4b8...6c0a590](https://codecov.io/gh/apache/qpid-dispatch/pull/1425?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-dispatch] jiridanek commented on pull request #1425: DISPATCH-2103 Log actual HTTP listener port number when 0 is configured

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #1425:
URL: https://github.com/apache/qpid-dispatch/pull/1425#issuecomment-962428886


   I'll try to explain the steps that got me to put in
   
   ```c
   #ifdef QD_HAVE_MODERN_LIBWEBSOCKETS
       info.finalize = finalize_http;
       info.finalize_arg = hl;
   #endif
   ```
   
   # Without touching this at all, qd_listener_t is sometimes not decrefed in http listener test
   
   # I came to believe this is because `qd_lws_listener_free` in `http_handler` is sometimes not called (This only happens on Ubuntu)
   
   ```c
       switch (reason) {
       case LWS_CALLBACK_PROTOCOL_DESTROY:
           qd_lws_listener_free(wsi_listener(wsi));
           break;
         default:
           break;
       }
   ```
   
   https://github.com/jiridanek/qpid-dispatch/runs/4116448428?check_suite_focus=true#step:9:41
   
   ```
   78: refcount is 2
   78: refcount is 1
   78: refcount is 0; freed
   78: refcount is 2
   78: refcount is 1
   78: refcount is 0; freed
   78: refcount is 2
   78: ===============================================================================
   78: ../tests/c_unittests/test_listener_startup.cpp:126:
   78: TEST CASE:  Start HTTP listener with zero port
   78: 
   78: ../tests/c_unittests/test_listener_startup.cpp:126: FATAL ERROR: test case CRASHED: SIGABRT - Abort (abnormal termination) signal
   ```
   
   
   # If I instead change http_handler ro respond to LWS_CALLBACK_WSI_DESTROY, I  get use after free when logging vhost name
   
   ```
       switch (reason) {
       case LWS_CALLBACK_WSI_DESTROY:
           qd_lws_listener_free(wsi_listener(wsi));
           break;
       default:
           break;
       }
   ```
   
   https://github.com/jiridanek/qpid-dispatch/runs/4117408625?check_suite_focus=true#step:9:42
   
   ```
   8: refcount is 2
   78: refcount is 1
   78: refcount is 0; freed
   78: refcount is 2
   78: refcount is 1
   78: refcount is 0; freed
   78: =================================================================
   78: ==2802==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000019210 at pc 0x7f89ee3cbdbb bp 0x7f89e81fc510 sp 0x7f89e81fbc88
   78: READ of size 2 at 0x602000019210 thread T5
   78:     #0 0x7f89ee3cbdba  (/lib/x86_64-linux-gnu/libasan.so.5+0x9cdba)
   78:     #1 0x7f89ee3cf255 in __vsnprintf_chk (/lib/x86_64-linux-gnu/libasan.so.5+0xa0255)
   78:     #2 0x7f89eda41eb3 in _lws_logv (/lib/x86_64-linux-gnu/libwebsockets.so.15+0x10eb3)
   78:     #3 0x7f89eda41fa7 in _lws_log (/lib/x86_64-linux-gnu/libwebsockets.so.15+0x10fa7)
   78:     #4 0x7f89eda4843d  (/lib/x86_64-linux-gnu/libwebsockets.so.15+0x1743d)
   78:     #5 0x7f89eda436c8  (/lib/x86_64-linux-gnu/libwebsockets.so.15+0x126c8)
   78:     #6 0x7f89eda3fcaa in lws_context_destroy (/lib/x86_64-linux-gnu/libwebsockets.so.15+0xecaa)
   78:     #7 0x559501500e2b in qd_http_server_free ../src/http-libwebsockets.c:982
   78:     #8 0x5595012dbd81 in qd_dispatch_free ../src/dispatch.c:369
   78:     #9 0x559501621c35 in QDR::deinitialize(bool) const ../tests/c_unittests/./helpers.hpp:265
   78:     #10 0x559501611bd9 in check_http_listener_startup_log_message(qd_server_config_t, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) ../tests/c_unittests/test_listener_startup.cpp:82
   78:     #11 0x55950161402b in operator() ../tests/c_unittests/test_listener_startup.cpp:133
   78:     #12 0x55950161adae in __invoke_impl<void, _DOCTEST_ANON_FUNC_24()::<lambda()> > /usr/include/c++/9/bits/invoke.h:60
   78:     #13 0x55950161a869 in __invoke<_DOCTEST_ANON_FUNC_24()::<lambda()> > /usr/include/c++/9/bits/invoke.h:95
   78:     #14 0x55950161a446 in _M_invoke<0> /usr/include/c++/9/thread:244
   78:     #15 0x55950161a0dc in operator() /usr/include/c++/9/thread:251
   78:     #16 0x559501619a8e in _M_run /usr/include/c++/9/thread:195
   78:     #17 0x7f89ed8fcde3  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xd6de3)
   78:     #18 0x7f89ee315608 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x9608)
   78:     #19 0x7f89ecc7f292 in __clone (/lib/x86_64-linux-gnu/libc.so.6+0x122292)
   ```


-- 
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@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org