<html><head><meta http-equiv="content-type" content="text/html; charset=us-ascii"></head><body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">Hi,<br><div><br><blockquote type="cite"><div>On 22 Sep 2023, at 19:58, Vladimir Homutov <vl@inspert.ru> wrote:</div><br class="Apple-interchange-newline"><div><meta charset="UTF-8"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">On Fri, Sep 22, 2023 at 07:30:50PM +0400, Roman Arutyunyan wrote:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">Hi Vladimir,<br><br>On Fri, Sep 22, 2023 at 03:44:08PM +0300, Vladimir Homutov via nginx-devel wrote:<br><blockquote type="cite"># HG changeset patch<br># User Vladimir Khomutov <vl@inspert.ru><br># Date 1695386443 -10800<br># Fri Sep 22 15:40:43 2023 +0300<br># Node ID 974ba23e68909ba708616410aa77074213d4d1e5<br># Parent 5741eddf82e826766cd0f5ec7c6fe383145ca581<br>QUIC: handle add_handhshake_data() callback errors in compat.<br><br>The error may be triggered by incorrect transport parameter sent by client.<br>The expected behaviour in this case is to close connection complaining<br>about incorrect parameter. Currently the connection just times out.<br><br>diff --git a/src/event/quic/ngx_event_quic_openssl_compat.c b/src/event/quic/ngx_event_quic_openssl_compat.c<br>--- a/src/event/quic/ngx_event_quic_openssl_compat.c<br>+++ b/src/event/quic/ngx_event_quic_openssl_compat.c<br>@@ -408,7 +408,10 @@ ngx_quic_compat_message_callback(int wri<br> "quic compat tx %s len:%uz ",<br> ngx_quic_level_name(level), len);<br><br>- (void) com->method->add_handshake_data(ssl, level, buf, len);<br>+ if (com->method->add_handshake_data(ssl, level, buf, len) != 1) {<br>+ ngx_post_event(&qc->close, &ngx_posted_events);<br>+ return;<br>+ }<br><br> break;<br></blockquote><br>Thanks for the patch. Indeed, it's a simple way to handle errors in callbacks.<br>I'd also handle the error in send_alert(), even though we don't generate any<br>errors in it now.<br></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">Yes, although I was not sure if we need to close connection if we failed</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">to send alert (but probably if we are sending it, everything is already</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">bad enough). In either case, handling both cases similarly looks</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">as a way to go.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"></div></blockquote><div><br></div><div>I assume, send_alert() would return an error on a fatal condition, not just a local error.</div><br><blockquote type="cite"><div><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><br>--<br>Roman Arutyunyan<br></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"># HG changeset patch<br># User Vladimir Khomutov <<a href="mailto:vl@inspert.ru">vl@inspert.ru</a>><br># Date 1695396237 -14400<br># Fri Sep 22 19:23:57 2023 +0400<br># Node ID 3db945fda515014d220151046d02f3960bcfca0a<br># Parent 32b5aaebcca51854de6e1f8a40798edb13662edb<br>QUIC: handle callback errors in compat.<br><br>The error may be triggered in add_handhshake_data() by incorrect transport<br>parameter sent by client. The expected behaviour in this case is to close<br>connection complaining about incorrect parameter. Currently the connection<br>just times out.<br><br>diff --git a/src/event/quic/ngx_event_quic_openssl_compat.c b/src/event/quic/ngx_event_quic_openssl_compat.c<br>--- a/src/event/quic/ngx_event_quic_openssl_compat.c<br>+++ b/src/event/quic/ngx_event_quic_openssl_compat.c<br>@@ -408,7 +408,9 @@ ngx_quic_compat_message_callback(int wri<br> "quic compat tx %s len:%uz ",<br> ngx_quic_level_name(level), len);<br><br>- (void) com->method->add_handshake_data(ssl, level, buf, len);<br>+ if (com->method->add_handshake_data(ssl, level, buf, len) != 1) {<br>+ goto failed;<br>+ }<br><br> break;<br><br>@@ -420,11 +422,19 @@ ngx_quic_compat_message_callback(int wri<br> "quic compat %s alert:%ui len:%uz ",<br> ngx_quic_level_name(level), alert, len);<br><br>- (void) com->method->send_alert(ssl, level, alert);<br>+ if (com->method->send_alert(ssl, level, alert) != 1) {<br>+ goto failed;<br>+ }<br> }<br><br> break;<br> }<br>+<br>+ return;<br>+<br>+failed:<br>+<br>+ ngx_post_event(&qc->close, &ngx_posted_events);<br>}<br><br></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">Looks good!</span></div></blockquote><br></div><div>Committed, thanks.</div><br><div>
<div dir="auto" style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><div>----</div><div>Roman Arutyunyan</div><div>arut@nginx.com</div><div><br></div></div><br class="Apple-interchange-newline"><br class="Apple-interchange-newline">
</div>
<br></body></html>