william-os4y / fapws3

Fast Asynchronous Python Web Server (based on libev)
GNU General Public License v2.0
341 stars 38 forks source link

problem with date format #17

Closed jerzyk closed 13 years ago

jerzyk commented 14 years ago

when you will set locale that is different than en_US or C, there may be an issues related to date formatting.

william-os4y commented 14 years ago

I'm agreed that current date format is not http compliant because it's the unix time format. But could you explain why type of issues did you have ? Anyhow to be consistent your proposed change requires changes in the Staticfile class too (maybe in other places too).

jerzyk commented 14 years ago

Basically, actual method is strictly locale dependent, so if your system has different locale than C or en_US all Date and Last-Modified are messed up, in effect no cache or anything that is using headers will not work.

E.g.

I've found date only in those two files:

william-os4y commented 14 years ago

Currently Fapws is using the unix time format: nbr of seconds since /01/01/1097. I don't understand how "local" will have an impact on that.

Regarding staticfiles ... If you use the Staticfile object, you will see that it return lot of 304 http messages. I've tested it with lot of different browser and they took the files from their cache really fine.

Concerning "varnish" ... Their site sounds down for the moment, I cannot thus check what it is. But if such tool cache files and required a valid date format, you will have a problem with current fapws. Do not forget that, in such a case, you could easily solve this by overwriting the http header before sending it.

jerzyk commented 14 years ago

sorry, maybe I was not clear:

In [1]: import datetime
In [2]: datetime.datetime.now().strftime('%a, %d %b %Y %H:%M:%S GMT')
Out[2]: 'Fri, 10 Sep 2010 13:39:40 GMT'
In [3]: import locale
In [4]: locale.setlocale(locale.LC_ALL, 'pl_PL.UTF-8')
Out[4]: 'pl_PL.UTF-8'
In [5]: datetime.datetime.now().strftime('%a, %d %b %Y %H:%M:%S GMT')
Out[5]: 'ptk, 10 wrz 2010 13:39:50 GMT'

If you have in the code (e.g. django app) locale change, it will mess-up your headers as strftime is locale dependent.

This applies in 3 places (where RFC 1123 formatted date is required).

I've given a varnish as an example, but this is same for other caches, microsoft's or w3cache.

When header is incorrect - is is being ignored.

william-os4y commented 14 years ago

Fapws is using the standard date format recognise by HTTP RFC.

Concerning Staticfile, it's better to use Etag than date. This is what I've just changed. Thanks to check the change of today.

W.

ps: thanks to confirm if I can close the issue

jerzyk commented 14 years ago

Date is still used for cookie..

william-os4y commented 14 years ago

Please can you be much more precise ?

If I read line 136 of base.py, I'm using the standard date format too.

jerzyk commented 14 years ago

so, again - no, format is not standard, at least not all the time. This code will work, but with the assumption that system locale is C or en_US/UK and nobody changes locale during web application execution.

If you are using multiple languages (thus multiple locales) using strftime will lead to non-RFC date format.

This is happens because strftime formatting function are locale bound, so to display date&time they are using values defined by the locale.

E.g. "wed" will become "śro" in pl_PL locale etc. - so date will not be "standard date format".

So this code works with the assumption, that C locale is a default all the time, which it shoudn't.

In the proposed code, I'm using function that is independent of actual locale settings, so it will work despite locale change.

This is common discussion of you will search google.

One of the examples is gunicorn, their approach was to define own function to format date. Look at http://github.com/benoitc/gunicorn/blob/master/gunicorn/util.py

My suggestion was to use standard library, but choice is yours.

Regarding other issue - using ETags, yes it is fine, but if you will put long time expire headers for the static files, browser will not even try to load a file (to check if it is changed) so yes, it will work, but is not optimal - browser will not use cache in 100% - it will try to get a resource and use cache if response was "not modified". But with additional expire header - connection will not be established. For the high-traffic sites, this will reduce significantly, total connections made to the site.

best regards, Janusz

william-os4y commented 14 years ago

Since Lighttpd is also using strftime, does this means that lighhtpd has also a date problem ? Concerning your proposed solution ... it does not include a solution for the C part of the application.

jerzyk commented 14 years ago

yes and no :) yes, because if there will be call to the locale() in the lighttpd process, then date will be affected

no, because there are no changes to locale in main code or modules (at least in built-in modules) and additionally, there is a setting to force the locale to "C" at the beginning of the code (even with nice comment ;))

"src/server.c" line 546: /* for nice %b handling in strfime() */ setlocale(LC_TIME, "C");

With fapws3, there are no guarantee that user code (which is executed as part of the main process) will not change a locale.

For the C part of the application, a solution can be to store current locale setting, set it to "C", then prepare all date headers, then set it back.

Not sure if storing and setting it back will be needed (didn't track an execution flow), but it better to set it back, as some of the application will change locale only on the code load, then assuming it is being set.

regards, Janusz

william-os4y commented 14 years ago

Thus best option would be to create a generic variable (in config.py) where the users set the locale. In python, we can set it by using locale.setlocale() in python; and in C, if not heritate from python, by using setlocale(). A comment will explain that changing it will break the RFC recommendations. But if someone want absolutely change it, then why not.

jerzyk commented 14 years ago

In my opinion, this is not the best option, simply because it will not work out-of-the-box as a replacement of any other comparable server.

Plus, this is not solving an issue, but trying to force users to do something in a very specific way.

If you really, really, really do not want to put small extra code in python (like e.g. gunicorn), so add this: tmp_locale = locale.get_locale() locale.set_locale('C') date_str = datetime.datetime.now().strftime(....) locale.set_locale(tmp_locale)

pretty simple, no additional configuration, no additional lines of manual to read (and you know that nobody reads manuals ;))

I still think, that adding this simple function to format date, and then set headers in python (as it was) will be simplest and most portable. This function can be used to format date in "Date", "Last-modified" and "Expire" headers and cookies. One solution for all of them.

J.

william-os4y commented 14 years ago

OK. In C, we could do something like this.

char *days_names[] ={ "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat" };
char *month_names[] ={ "Jan", "Feb", "Mar", "Apr", "May", "Jun","Jul", "Aug", "Sep", "Oct", "Nov", "Dec" };
strftime(outstr, date_len+1, "---, %d --- %Y %H:%M:%S GMT", tmp);
memcpy(outstr, days_names[tmp->tm_wday], 3);
memcpy(outstr+8, month_names[tmp->tm_mon], 3);

But, now I'm looking a way to send a python datetime object to such C function and convert it to time_t. Should not be that difficult.

jerzyk commented 14 years ago

I'm confused ;) maybe I'm missing something, but in the C code, there is only one place where you are using date formatting - response header, but.. this was few changesets before in the python code - that means there is no need to process date in the C code at all

william-os4y commented 14 years ago

the last commit should solve the date problem. I prefer to have it in C for performance reasons.

You are right ... you can overwrite what fapws is doing within your python code and before sending the response to the browser. Thus indeed, you could overwrite the date in an another format; depends what you want ;-).

Could you test and confirm I can close this issue ?

W.

william-os4y commented 14 years ago

Could you confirm that I can close the issue ?

Thanks

william-os4y commented 13 years ago

issue not reproducable