Self explained in the title. I’ve seen a lot of improvements in WebPositive recently, which in turn made the browser useful for a lot of real life daily use cases (congratulations to the devs!), and even encouraged me to send a patch that got merged (the ALT -or CTRL- + TAB tab cycle shortcut I personally use a lot). Following item in the list is that website sessions are not remembered between Webpositive sessions, and so if I close the browser I have to log in again in a lot (if not all) of websites.
Why is that? Are we missing something in Web+ or HaikuWebkit to be able to support this? Using the cookie manager I’ve found that most of the cookies are deleted on exit using the PurgeForExit() method from the system’s cookie jar. Is this intented behaviour and sessions should be remembered in a different way or are we just deleting things that we shouldn’t on exit?
I went ahead and commented out the call to PurgeForExit() on WebPositive source code, recompiled and… it’ worked. Logins were kept. Obviously you don’t want session cookies being kept, but for testing purposes it was more than enough. So I went to see what BNetworkCookieJar’s PurgeForExit() does, and basically it checks every cookie: if it’s a session cookie or the expiration date and time has passed, it gets deleted. I then opened the Cookie manager and to my surprise, for some reason most (if not all) of the cookies are stored with an expiration datetime of 1970/01/01 00:00:00 GMT, or 0 in epoch time, which means they all get deleted on exit. Using this recompiled version of Webpositive I could see all of the cookies transformed in session cookies one of the times I opened the browser, so something funny is happening to the timestamps.
Not knowing anything about cookies, for a moment I thought maybe 0 was reserved for “not expiring cookies”, if that makes sense, so I checked the same website on Chromium on Windows and the cookies have a logical expiration date and time, on the next hours/days/months.
Probably something in the HTTP library for parsing the expiration dates for cookies is broken. Maybe it is related to timezones.
I have already fixed bugs a couple times in that area, and wrote a few unit tests, but apparently it is too complex for me because I can never get it to work right.
I also had quite a few surprises because there is no RFC describing what websites and webservers really do with cookies. The RFCs you will find are:
The original one defining cookie, saying “this is how it should work”, but the original web browsers did not exactly implement that (because of bugs) and the webservers went with “ok then let’s workaroudn the bugs in the browsers”
Then 2 or 3 RFCs that say “the way this works now is so broken, here is how it should really work”. One pretending to be backwards compatible, and a later one saying “ok, no one wants to fix cookie, so let’s have cookie2” which no ones implements.
So I spent a few hours looking at what websites were sending me, and tweaked the date parsing until I got most things working. But I must have missed a few things or maybe some webserver update changed something, or maybe the code I wrote was broken by some other change on our side.
Thanks for the input. I’m currently compiling Haikuwebkit for the first time on a not-so-new machine (i5-8gb from 2016 I reckon) but cmake + ninja should take care of this after this time and I should be able to test small changes in a reasonable amount of time. Took me a while to figure out where the code for (sorry about the expression) converting a Webkit cookie jar into a BNetworkCookieJar was, but I found it and it has a convenient TRACE define already in place that I only had to enable. I couldn’t find anything wrong in BNetworkCookieJar’s code, and WebPositive doesn’t even mess with cookies at all (it just opens the cookie jar and little more) so my hopes for a quick test went down the toilet and here we are, waiting for the compilation to finish but stay tuned
And is explained even better in a comment in HttpTime.cpp, which you obviously wrote (thanks for that!) Indeed you were right on the money. Inside the Parse() method in that class, strptime is not matching. Correct me if I’m wrong but strptime is locale dependant, and cookies, thanks to the bright minds you mention, store the day of the week written in english. Why on earth someone decided that was a good idea instead of just using numbers is beyond my understanding, but with that given if the locale is not english strptime is never going to match. I changed my locale to english and yet the problem seemed to persist, so there might be another reason for this (according to git there were recent changes in that function), but unless I’m missing something we shouldn’t be using strptime anyway.
When I wrote the code, strptime wasn’t locale dependant, or at least, it was dependant on the LC_* variables which were nornally not set. Maybe it is now. That would explain the problem indeed.
unless I’m missing something we shouldn’t be using strptime anyway.
So what should we use instead? That’s exactly the thing strptime is designed to do.
We should use it but use setlocale (now that it is implemented) to set the correct locale. Or maybe we can add some functions in the Locale Kit to do this in a less complicated way.
If you try to write code using C and posix standard functions you find out that there are only wrong solutions. You can complain about it on the posix bugtracker. Or suggest a correct thing to do here, because otherwise this isn’t really going to help with fixing the issue.
But that conversation might have to wait. I stripped down HttpTime.cpp to a bare minimum to make a really simple test program. Tried changing the locale to en_US (even rebooting, just in case) and strptime is never matching. Could it be broken?
#include <cstdio>
#include <cstring>
#include <ctime>
static const char* kDateFormats = "%a, %d %b %Y %H:%M:%S";
static const char* example = "Mon, 5 Sep 2022 14:14:14";
// Main function
int main(void)
{
struct tm expireTime;
memset(&expireTime, 0, sizeof(struct tm));
int fDateFormat = -1;
const char* result = strptime(example, kDateFormats,
&expireTime);
printf("Parsing %s\n", example);
printf("Year %d, hour %d\n", expireTime.tm_year, expireTime.tm_hour);
if (result != NULL) {
printf("Format found\n");
fDateFormat = 1;
}
if (fDateFormat == -1)
printf("Format not found\n");
}
Possibly, you can never trust anything in a beta-quality software like Haiku and there are always bugs in unexpected places deeper than you expected.
I don’t know how the POSIX locale functions are linked to the Locale Kit and if they change locale together or if you need to set the environment variables for the POSIX functions to work.
According to the man page I found online %d and %e are interchangeable and have the same behaviour.
I was forgetting to change the formats on the Locale preflet (the second tab) so I had the opportunity to have a look at this a few minutes ago. The bash environment variables get changed as soon as I change the settings (good!). I changed everything to english and even rebooted to be sure I was on an english locale, and nothing: strptime never matches. In any case, if the POSIX functions weren’t tied to the LocaleKit they should be in english since I’ve never changed anything in another place.
I then booted up a Xubuntu Live CD in English, took the exact same test code snippet, compiled it without a problem, ran it and… it matches perfectly fine. So that means we have a broken strptime, I’ll file a bug report.
In any case, seems that we can’t use it for our purposes here. I’ll have a look at strptime_l.
netservices does not use CURL, so how would it do that? It may make sense to fix the issues @PulkoMandy identified with using the CURL backend in HaikuWebKit (missing features, etc.) so that we can take advantage of CURL’s greater stability and performance, and not have to worry about bugs like these, to be sure.
Importing the curl function. This code looks like it’s specific to cookie expiration date, why shouldn’t we rely on something battle tested for this usage? Fixing bugs is also possible obviously, if we find someone to do it.
but stay tuned