public inbox for connman@lists.linux.dev
 help / color / mirror / Atom feed
From: Grant Erickson <gerickson@nuovations.com>
To: connman@lists.linux.dev
Subject: [PATCH 00/22] Close Two GWeb Request "Bookend" Failure "Holes"
Date: Mon,  3 Mar 2025 17:10:51 -0800	[thread overview]
Message-ID: <cover.1741048994.git.gerickson@nuovations.com> (raw)

There existed, prior to this set of changes, two "holes" in the
finalization of web request sessions that can, for example in a client
such as WISPr leave one or both of IPv4 and IPv6 "online" HTTP-based
Internet reachability checks "dangling" and non-renewing such that an
expected active, default network service failover does not occur when
it should.

The first of those two failure "holes" involves an end-of-file (EOF)
condition. In the normal case, there are a series of one or more data
receipts for the web request as headers and, if present, the body of
the request response are fulfilled. When there is no further data to
send, the final receive completes with 'g_io_channel_read_chars'
returning 'G_IO_STATUS_EOF' status and zero (0) bytes read. This should
and does lead to a successful EOF closure of the web request.

However, there is a second EOF case that appears to happen with a
random distribution in the nginx server backing the default "online"
HTTP-based Internet reachability check URLs,
'ipv[46].connman.net/online/status.html'. In that case, the nginx
server appears to periodically do an unexpected and spontaneous remote
connection close after the initial connection but before any data has
been received.

For WISPr, presently, all such failures hit the same error-handling
block which effectively maps such a failure to the effective "null"
'GWEB_HTTP_STATUS_CODE_UNKNOWN' status value. Unfortunately, clients
such as WISPr have no way to distinguish this as an actual failure or
success and so it lands on the case:

	case GWEB_HTTP_STATUS_CODE_UNKNOWN:
		wispr_portal_context_ref(wp_context);
		__connman_agent_request_browser(wp_context->service,
				wispr_portal_browser_reply_cb,
				wp_context->status_url, wp_context);

which does not "bookend" the original, initiating WISPr web request
and leaves the original request "dangling" and non-renewed, eventually
leading to the aforementioned "hole" and network service failover
failure.

To handle this failure EOF case, if GWeb asked for a non-zero amount
of data from 'g_io_channel_read_chars' but received none and
accumulated no headers thus far upon receiving the status
'G_IO_STATUS_EOF', then GWeb assumes that the remote peer server
unexpectedly closed the connection, and synthesizes the error
'-ECONNRESET' with the same HTTP status "null" value of
GWEB_HTTP_STATUS_CODE_UNKNOWN. With the addition of the new
'g_web_result_get_err' interface, clients such as WISPr can now
distinguish between a low-level operating system error in which no
HTTP data was received and a success in which HTTP data was received
and the status can be disguished by the HTTP status code.

The second of the two failure "holes" involves the case where
'g_io_channel_read_chars' returns 'G_IO_STATUS_ERROR' status. Prior to
this change, this funneled to the same "hole" as the 'G_IO_STATUS_ERROR'
failure with the same consequence to clients such as WISPr.

To handle this error case, GWeb now passes a glib 'GError' pointer to
'g_io_channel_read_chars'. If it is set, GWeb maps the resulting error
domain/code pairs to negated POSIX domain errors, and default to '-EIO'
if no suitable mapping can be made. As with the failure EOF case, this
allows clients to distinguish and handle such failures and to
successfully "bookend" their initial web request.

With these changes, WISPr now leverages the newly-added GWeb
'g_web_result_get_err' interface to refactor 'wispr_portal_web_result'
into 'wispr_portal_web_result_err' and
'wispr_portal_web_result_no_err' with the former handling
non-successful POSIX error domain cases and the latter handling
successful HTTP status code cases.

With this change, the second, failure EOF case will now return
'-ECONNRESET' from 'g_web_result_get_err' and will be handled as a
failure by 'wispr_portal_web_result_err', "bookending" the original
"online" HTTP-based Internet reachability check.

All of the prior HTTP status code cases are handled by
'wispr_portal_web_result_no_err'.

Grant Erickson (22):
  gweb: Leverage 'GWEB_HTTP_STATUS_CODE_UNKNOWN' mnemonic.
  gweb: Refactor 'received_data' into
    'received_data_{finalize,continue}'.
  gweb: Added 'g_web_result_has_headers'.
  gweb: Add documentation to 'g_web_result_has_headers'.
  gweb: Added 'g_web_result_get_err'.
  gweb: Add documentation to 'g_web_result_get_err'.
  gweb: Add an OS err parameter to 'call_result_func'.
  gweb: Add documentation to 'call_result_func'.
  gweb: Add documentation to 'g_web_result_get_status'.
  gweb: Add documentation to 'received_data'.
  gweb: Close web request session finalization 'hole'.
  gweb: Add documentation to 'map_gerror'.
  gweb: Add documentation to 'received_data_{finalize,continue}'.
  gweb: Document 'struct _GWebResult'.
  wispr: Add and leverage 'portal_manage_failure_status'.
  wispr: Refactor 'wispr_portal_web_result' to leverage
    'g_web_result_get_err'.
  wispr: Document 'portal_manage_failure_status'.
  wispr: Document 'wispr_portal_web_result_err'.
  wispr: Document 'wispr_portal_web_result_no_err'.
  wispr: Document 'wispr_portal_web_result'.
  wispr: Fix documentation typo in 'portal_manage_success_status'.
  service: Capture and propagate '__connman_wispr_start' return status.

 gweb/gweb.c   | 444 ++++++++++++++++++++++++++++++++++++++++++++++----
 gweb/gweb.h   |   3 +
 src/service.c |   9 +-
 src/wispr.c   | 236 +++++++++++++++++++++------
 4 files changed, 602 insertions(+), 90 deletions(-)

-- 
2.45.0


             reply	other threads:[~2025-03-04  1:11 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-04  1:10 Grant Erickson [this message]
2025-03-04  1:10 ` [PATCH 01/22] gweb: Leverage 'GWEB_HTTP_STATUS_CODE_UNKNOWN' mnemonic Grant Erickson
2025-03-04  1:10   ` [PATCH 02/22] gweb: Refactor 'received_data' into 'received_data_{finalize,continue}' Grant Erickson
2025-03-04  1:10   ` [PATCH 03/22] gweb: Added 'g_web_result_has_headers' Grant Erickson
2025-03-04  1:10   ` [PATCH 04/22] gweb: Add documentation to 'g_web_result_has_headers' Grant Erickson
2025-03-04  1:10   ` [PATCH 05/22] gweb: Added 'g_web_result_get_err' Grant Erickson
2025-03-04  1:10   ` [PATCH 06/22] gweb: Add documentation to 'g_web_result_get_err' Grant Erickson
2025-03-04  1:10   ` [PATCH 07/22] gweb: Add an OS err parameter to 'call_result_func' Grant Erickson
2025-03-04  1:10   ` [PATCH 08/22] gweb: Add documentation " Grant Erickson
2025-03-04  1:11   ` [PATCH 09/22] gweb: Add documentation to 'g_web_result_get_status' Grant Erickson
2025-03-04  1:11   ` [PATCH 10/22] gweb: Add documentation to 'received_data' Grant Erickson
2025-03-04  1:11   ` [PATCH 11/22] gweb: Close web request session finalization 'hole' Grant Erickson
2025-03-04  1:11   ` [PATCH 12/22] gweb: Add documentation to 'map_gerror' Grant Erickson
2025-03-04  1:11   ` [PATCH 13/22] gweb: Add documentation to 'received_data_{finalize,continue}' Grant Erickson
2025-03-04  1:11   ` [PATCH 14/22] gweb: Document 'struct _GWebResult' Grant Erickson
2025-03-04  1:11   ` [PATCH 15/22] wispr: Add and leverage 'portal_manage_failure_status' Grant Erickson
2025-03-04  1:11   ` [PATCH 16/22] wispr: Refactor 'wispr_portal_web_result' to leverage 'g_web_result_get_err' Grant Erickson
2025-03-04  1:11   ` [PATCH 17/22] wispr: Document 'portal_manage_failure_status' Grant Erickson
2025-03-04  1:11   ` [PATCH 18/22] wispr: Document 'wispr_portal_web_result_err' Grant Erickson
2025-03-04  1:11   ` [PATCH 19/22] wispr: Document 'wispr_portal_web_result_no_err' Grant Erickson
2025-03-04  1:11   ` [PATCH 20/22] wispr: Document 'wispr_portal_web_result' Grant Erickson
2025-03-04  1:11   ` [PATCH 21/22] wispr: Fix documentation typo in 'portal_manage_success_status' Grant Erickson
2025-03-04  1:11   ` [PATCH 22/22] service: Capture and propagate '__connman_wispr_start' return status Grant Erickson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1741048994.git.gerickson@nuovations.com \
    --to=gerickson@nuovations.com \
    --cc=connman@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox