Add guppy:// support

[dimkr]

Protocol is described in gemini://gemini.dimakrasner.com/guppy-v0.4.gmi (or guppy://gemini.dimakrasner.com/guppy-v0.4.gmi). EDIT: moved to gemini://guppy.000090000.xyz, guppy://guppy.000090000.xyz and https://github.com/dimkr/guppy-protocol

I was thinking, if Lagrange supports Nex, maybe it would be nice to support Guppy too, especially if this doesn't complicate the implementation of other protocols and doesn't introduce stability or security risks for those who don't use this protocol at all. v0.4 of the protocol includes additions and changes (like status code 1) that increase code reuse if support for this protocol is added to an existing Gemini client. If this protocol takes off thanks to high quality implementations of clients and servers, great! If not, this is not the end of the world and I understand that a new protocol needs special treatment in the crowded space of gmrequest.c (especially if it's based on UDP and not TCP like the others) and every line of code can be a burden.

This PR implements Guppy v0.4 support with:

• 16 cached data chunks, appended to the rendered document in the correct order as early as possible
  • Enough to receive and render a 8K sized document sent in 512b chunks, without having to wait for some chunks to be re-transmitted (should be very generous considering average and median page size stats from Gemini crawlers)
  • With logic to free a cache slot if the first response chunk arrives after all slots are occupied by other chunks
• Retries (not very smart, every 100ms)
  • Retry of the request every 1s until the first response chunk is received
  • Retry of the last acknowledgement packet every 500ms, until more data is received
• Support for input prompts and redirects
• 🐟 for guppy:// links

This implementation is naive in some ways, I can't rule out the possibility of memory leaks or corner cases I haven't fixed, and I can see places where the implementation can be optimized to reduce memory copying at the cost of extra code complexity.

Tested against guppy://hd.206267.xyz, server code is available at https://github.com/dimkr/tootik/compare/guppy and the protocol spec contains Python examples that might be easier to understand and scrutinize for corner cases that need to be handled in a reliable and well-behaving client.

For this to work, iSocket constructor needs to receive an additional socket type parameter:

diff --git a/include/the_Foundation/socket.h b/include/the_Foundation/socket.h
index 33e3821..3c6878b 100644
--- a/include/the_Foundation/socket.h
+++ b/include/the_Foundation/socket.h
@@ -73,7 +73,7 @@ enum iSocketStatus {
     disconnected_SocketStatus,
 };
 
-iDeclareObjectConstructionArgs(Socket, const char *hostName, uint16_t port)
+iDeclareObjectConstructionArgs(Socket, const char *hostName, uint16_t port, enum iSocketType socketType)
 
 iSocket *   newAddress_Socket   (const iAddress *address);
 iSocket *   newExisting_Socket  (int fd, const void *sockAddr, size_t sockAddrSize);
diff --git a/src/platform/posix/socket.c b/src/platform/posix/socket.c
index 047ce16..c9f1d9c 100644
--- a/src/platform/posix/socket.c
+++ b/src/platform/posix/socket.c
@@ -55,6 +55,7 @@ struct Impl_Socket {
     iBuffer *output;
     iBuffer *input;
     enum iSocketStatus status;
+    enum iSocketType type;
     iAddress *address;
     int fd;
     iPipe *stopConnect;
@@ -238,8 +239,8 @@ iLocalDef void start_SocketThread(iSocketThread *d) { start_Thread(&d->thread);
 /*-------------------------------------------------------------------------------------*/
 
 iDefineObjectConstructionArgs(Socket,
-                              (const char *hostName, uint16_t port),
-                              hostName, port)
+                              (const char *hostName, uint16_t port, enum iSocketType socketType),
+                              hostName, port, socketType)
 
 static iBool setStatus_Socket_(iSocket *d, enum iSocketStatus status) {
     if (d->status != status) {
@@ -258,6 +259,7 @@ static void init_Socket_(iSocket *d) {
     openEmpty_Buffer(d->output);
     openEmpty_Buffer(d->input);
     d->fd = -1;
+    d->type = tcp_SocketType;
     d->address = NULL;
     d->stopConnect = new_Pipe(); /* used for aborting select() on user action */
     d->connecting = NULL;
@@ -511,12 +513,12 @@ iSocket *newExisting_Socket(int fd, const void *sockAddr, size_t sockAddrSize) {
     return d;
 }
 
-void init_Socket(iSocket *d, const char *hostName, uint16_t port) {
+void init_Socket(iSocket *d, const char *hostName, uint16_t port, enum iSocketType socketType) {
     init_Socket_(d);
     d->address = new_Address();
     setStatus_Socket_(d, addressLookup_SocketStatus);
     iConnect(Address, d->address, lookupFinished, d, addressLookedUp_Socket_);
-    lookupTcpCStr_Address(d->address, hostName, port);
+    lookupCStr_Address(d->address, hostName, port, socketType);
 }
 
 iBool open_Socket(iSocket *d) {
diff --git a/src/platform/win32/socket.c b/src/platform/win32/socket.c
index 2ebf680..e879329 100644
--- a/src/platform/win32/socket.c
+++ b/src/platform/win32/socket.c
@@ -237,8 +237,8 @@ iLocalDef void start_SocketThread(iSocketThread *d) { start_Thread(&d->thread);
 /*-------------------------------------------------------------------------------------*/
 
 iDefineObjectConstructionArgs(Socket,
-                              (const char *hostName, uint16_t port),
-                              hostName, port)
+                              (const char *hostName, uint16_t port, enum iSocketType socketType)),
+                              hostName, port, socketType)
 
 static iBool setStatus_Socket_(iSocket *d, enum iSocketStatus status) {
     if (d->status != status) {
@@ -498,12 +498,12 @@ iSocket *newExisting_Socket(int fd, const void *sockAddr, size_t sockAddrSize) {
     return d;
 }
 
-void init_Socket(iSocket *d, const char *hostName, uint16_t port) {
+void init_Socket(iSocket *d, const char *hostName, uint16_t port, enum iSocketType socketType) {
     init_Socket_(d);
     d->address = new_Address();
     setStatus_Socket_(d, addressLookup_SocketStatus);
     iConnect(Address, d->address, lookupFinished, d, addressLookedUp_Socket_);
-    lookupTcpCStr_Address(d->address, hostName, port);
+    lookupCStr_Address(d->address, hostName, port, socketType);
 }
 
 iBool open_Socket(iSocket *d) {
diff --git a/src/tlsrequest.c b/src/tlsrequest.c
index 1e03493..31cc55e 100644
--- a/src/tlsrequest.c
+++ b/src/tlsrequest.c
@@ -1117,7 +1117,7 @@ void submit_TlsRequest(iTlsRequest *d) {
     if (d->sessionCacheEnabled) {
         d->cert = maybeReuseSession_Context_(context_, d->ssl, d->hostName, d->port, d->clientCert);
     }
-    d->socket = new_Socket(cstr_String(d->hostName), d->port);
+    d->socket = new_Socket(cstr_String(d->hostName), d->port, tcp_SocketType);
     iConnect(Socket, d->socket, connected, d, connected_TlsRequest_);
     iConnect(Socket, d->socket, disconnected, d, disconnected_TlsRequest_);
     iConnect(Socket, d->socket, readyRead, d, gotIncoming_TlsRequest_);

(plus something similar for win32)

[skyjake]

Unfortunately, the Socket class is meant to be for TCP only. Perhaps you missed the Datagram class in the_Foundation? That implements an API appropriate for UDP where there is no concept of two-way connections, just writing out and receiving messages. Please take a look at it and see if it can be used instead.

For async usage, Datagram has the message audience where one can add a callback for when a UDP message is received.

[dimkr]

Perhaps you missed the Datagram class in the_Foundation?

The first draft used this API, but I had to duplicate some parts of socket.c inside gmrequest.c (the async DNS resolver stuff) and wait with the request until DNS resolution succeeds. Using the Socket API with UDP made things much simpler because it takes care of DNS and connect(), maximizing code reuse and abstracting away TCP vs. UDP differences.

I'll see if I can re-implement things with Datagram, I'm much more familiar with the the_Foundation API now.

[skyjake]

The async address resolving is done by the Address class. The Socket constructor you modified is basically just for convenience. You can always do the resolving manually with Address first. Is this the part of Socket you were duplicating?

In any case, I am unwilling to change the Socket class as you were proposing initially.

[dimkr]

Is this the part of Socket you were duplicating?

Yes, I had to keep an iAddress inside iGmRequest and pretty much duplicate addressLookedUp_Socket_(), open_Socket_(), etc'.

[skyjake]

I see. Another change I would request that you do is move everything related to Guppy out of gmrequest.c into a guppy.c, and have a top-level object called Guppy that GmRequest can then construct and use to perform the request. In other words, please make a Guppy request helper class. I consider your changes to gmrequest.c so wide-ranging that they start to muddy the implementation of the class.

Guppy is different enough from Gemini and the other protocols supported by GmRequest so it's better to keep the implementation separate. I understand you're trying to reuse code, but for an experimental protocol that is not even TCP-based, the reuse should be occurring at a lower level.

[dimkr]

Two unhandled corner cases:

• Request should fail on ICMP port unreachable messages
• Timeouts - request to closed port can be dropped silently and request should fail after some time

EDIT: first problem can be fixed by a small patch to run_SocketThread_():

             if (readSize == -1) {
+                int err = errno;
                 if (status_Socket(d->socket) == connected_SocketStatus) {
                     iWarning("[Socket] error when receiving: %s\n", strerror(errno));
+                    if (err == ECONNREFUSED) {
+                        setError_Socket_(d->socket, ECONNREFUSED, "Connection refused");
+                    }
                     shutdown_Socket_(d->socket);
                     return errno;
                 }

connect() returns 0 (succeeds immediately) and errors are reported by the recv() that follows.

EDIT 2: connectionTimeoutSeconds_Socket_ applies only to connect(), which always succeed immediately and doesn't do much with UDP, so some concept of per-session timeout needs to be implemented at least for Guppy. done

[dimkr]

@skyjake Please take a second look, I moved almost everything minus the parts that change iGmRequest.state and iGmResponse.statusCode out of gmrequest.c, and the coding style is closer to that of gopher.c.

[skyjake]

Status update: Don't worry, I haven't forgotten about this. I've just been too busy to look into this more closely. The plan is to get this into the v1.18 release of Lagrange.

[dimkr]

Thanks for the update @skyjake!

[skyjake]

I reviewed the latest changes here in the PR and they look fine apart from the UDP Socket stuff, as discussed before (i.e., should use Datagram instead of Socket).

Could you provide a complete patch set including changes in the_Foundation?

[dimkr]

Could you provide a complete patch set including changes in the_Foundation?

Yes. Should I just fork https://git.skyjake.fi/skyjake/the_Foundation and supply a branch name?

[skyjake]

Yes, a branch in a forked repo should do.

[dimkr]

dimkr/the_Foundation@main...udp

This is what I had when I opened this PR, a short patch that allows Socket to use UDP, so GmRequest can work pretty much the same way across Gemini and Guppy.

EDIT: haven't tested the Windows part at all

EDIT 2: oh yes, I thought it would be cleaner to make Socket support UDP, because it handles corner cases like ECONNREFUSED and takes care of DNS resolution, maximizing shared code