taviso / nntpit

minimalist reddit2nntp gateway
MIT License
245 stars 10 forks source link

nntpit: Add XOVER and LIST OVERVIEW.FMT. #12

Closed daym closed 2 years ago

daym commented 2 years ago

This adds support for XOVER and some initial support for LIST OVERVIEW.FMT. The latter is specified by the RFC as "should implement when implementing XOVER".

This almostly makes nntpit work with the Pan newsreader--the message ids are still not to its liking (missing "@"). I've also got a patch for the latter already. Then it works just fine.

taviso commented 2 years ago

wow, thanks! I tried it out, but slrn sends XOVER without any parameter, and that crashes with a NULL param here:

    int beginning = strtol(param, &endptr, 10);

I'm not sure what the server is supposed to send back, I need to refresh my nntp memory.

daym commented 2 years ago

Thanks :)

https://datatracker.ietf.org/doc/html/rfc2980#section-2.8 says

If no argument is specified, then information from the current article is displayed.

What is the "current" article?

For now, I just fixed the crash by not allowing empty argument.

taviso commented 2 years ago

Ahh - that seems fine to me! It doesn't crash anymore, but slrn doesn't like the field names. It says:

OVERVIEW.FMT not RFC 2980 compliant -- XOVER support disabled.

This seems to work though:

        client_printf(cl, "215 information follows\r\n");
        // TODO: What are the field names usually used here?
        client_printf(cl, "subject\r\n");
        client_printf(cl, "from\r\n");
        client_printf(cl, "date\r\n");
        client_printf(cl, "message-id\r\n");
        client_printf(cl, "references\r\n");
        client_printf(cl, "bytes\n");
        client_printf(cl, "lines\r\n");
        client_printf(cl, ".\r\n");

Can you see if that works with your client too? (You still need to send the article number, just not include it in the overview).

taviso commented 2 years ago

If that works for you too, I see no reason not to merge for now...

I'm not sure if the line and byte count should be accurate, or maybe we will confuse someone... but confusing them seems better than not working at all!

daym commented 2 years ago

Yeah, it works for my reader too without number in the overview.

daym commented 2 years ago

Yeah, I don't see what line and byte counts would be for in the first place (works fine without)... Ideal would be if there's a value to use for "don't know".

That said, XOVER line count and byte count could be done exactly like they are done in comments.c :

        body = json_object_get_string_prop(data, "body");
        // byte_count = strlen(body);
        line_count = str_count_newlines(body);
taviso commented 2 years ago

Looks good to me - thanks!!