-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
bpo-29654 : Support If-Modified-Since HTTP header (browser cache) #298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:
Thanks again to your contribution and we look forward to looking at it! |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please sign the PSF contributor agreement and add entries in Misc/NEWS and What's New file.
Lib/http/server.py
Outdated
| if ims is not None: | ||
| ims_datetime = datetime.datetime(*ims[:7]) | ||
| ims_dtstring = ims_datetime.strftime("%d %b %Y %H:%M:%S") | ||
| last_modif = datetime.datetime.utcfromtimestamp( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ims_datetime is naive datetime object, but datetime.datetime.utcfromtimestamp() creates aware datetime object.
Lib/http/server.py
Outdated
| ims_dtstring = ims_datetime.strftime("%d %b %Y %H:%M:%S") | ||
| last_modif = datetime.datetime.utcfromtimestamp( | ||
| fs.st_mtime).strftime("%d %b %Y %H:%M:%S") | ||
| if last_modif <= ims_dtstring: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why compare string representations rather than datetime objects or just tuples? The order of such strings is different from the order of datetime objects.
Lib/http/server.py
Outdated
| else: | ||
| handler_class = SimpleHTTPRequestHandler | ||
| test(HandlerClass=handler_class, port=args.port, bind=args.bind) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated changes.
Lib/http/server.py
Outdated
| # Use browser cache if possible | ||
| if "If-Modified-Since" in self.headers: | ||
| # compare If-Modified-Since and date of last file modification | ||
| fs = os.stat(path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use os.fstat().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe combine with the existing call below, too
Lib/http/server.py
Outdated
| return None | ||
| try: | ||
| # Use browser cache if possible | ||
| if "If-Modified-Since" in self.headers: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RFC https://tools.ietf.org/html/rfc7232#section-3.3 says you have to ignore this field if If-None-Match is also specified
Lib/http/server.py
Outdated
| if "If-Modified-Since" in self.headers: | ||
| # compare If-Modified-Since and date of last file modification | ||
| fs = os.stat(path) | ||
| ims = email.utils.parsedate(self.headers["If-Modified-Since"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This email function accepts a timezone (e.g. from the documentation: -0500), and seems to ignore it. But it may make more sense for a HTTP server to either adjust the date according to the timezone (e.g. add 5 hours), or treat it as an error (required by RFC 7232).
Lib/test/test_httpservers.py
Outdated
| #constructs the path relative to the root directory of the HTTPServer | ||
| response = self.request(self.base_url + '/test') | ||
| self.check_status_and_reason(response, HTTPStatus.OK, data=self.data) | ||
| # get Last-Modified response heaer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling: header. But this comment seems redundant; isn’t the code obvious enough?
| headers = Message() | ||
| headers['If-Modified-Since'] = last_modif | ||
| response = self.request(self.base_url + '/test', headers=headers) | ||
| self.check_status_and_reason(response, HTTPStatus.NOT_MODIFIED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please repeat the test with an earlier date, verifying that a response body is returned.
Lib/http/server.py
Outdated
| # Use browser cache if possible | ||
| if "If-Modified-Since" in self.headers: | ||
| # compare If-Modified-Since and date of last file modification | ||
| fs = os.stat(path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe combine with the existing call below, too
…Match is present. Add tests.
|
Thanks for all the feedback. I have signed the CLA, it will probably be approved tomorrow. I will add the entries in Misc/NEWS and What's New. In the second commit :
|
Lib/http/server.py
Outdated
| try: | ||
| # Use browser cache if possible | ||
| if "If-Modified-Since" in self.headers: | ||
| if "If-Modified-Since" in self.headers \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is more preferable to use parenthesis rather than line continuation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Lib/http/server.py
Outdated
| # Use browser cache if possible | ||
| if "If-Modified-Since" in self.headers: | ||
| if "If-Modified-Since" in self.headers \ | ||
| and not "If-None-Match" in self.headers: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"If-None-Match" not in self.headers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done too
Lib/http/server.py
Outdated
| fs.st_mtime).strftime("%d %b %Y %H:%M:%S") | ||
| if last_modif <= ims_dtstring: | ||
| # If-Modified-Since is UTC, rounded to the second | ||
| tzinfo = datetime.timezone(datetime.timedelta(hours=0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
datetime.timezone.utc
But maybe use naive datetime objects?
Lib/http/server.py
Outdated
| and not "If-None-Match" in self.headers: | ||
| # compare If-Modified-Since and date of last file modification | ||
| fs = os.stat(path) | ||
| ims = email.utils.parsedate(self.headers["If-Modified-Since"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks a problem to me that email.utils.parsedate() silently ignores timezone. Maybe use email.utils.parsedate_to_datetime() instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, the latest commit uses parsedate_to_datetime() and checks that it has a timezone set to UTC, which should be the case for the header (cf RFC 7231). To compare with last modification time, both must be timezone-aware datetimes.
Lib/test/test_httpservers.py
Outdated
| import email.utils | ||
| dt = email.utils.parsedate(last_modif) | ||
| # build datetime object : one year before last modification | ||
| old_dt = [dt[0] - 1] + list(dt[1:7]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming the date reflects when the test suite is run, this will fail on a leap day (e.g. 2020-02-29). Perhaps try something like “datetime(*old_dt) - timedelta(days=365)”.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, I have modified the test.
Lib/http/server.py
Outdated
| ims_datetime = datetime.datetime(*ims[:7], tzinfo=tzinfo) | ||
| # compare to UTC datetime of last modification, also | ||
| # rounded to the second | ||
| mtime = int(fs.st_mtime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“Int” rounds towards 1970. The actual rounding that “date_time_string” uses seems to be a combination of rounding half to even microseconds (thanks to datetime), and then rounding down (always backwards in time) to seconds.
It might be more sensible to call datetime.replace(microseconds=0).
Lib/test/test_httpservers.py
Outdated
| self.check_status_and_reason(response, HTTPStatus.OK) | ||
|
|
||
| # build datetime object : one hours after last modification | ||
| new_dt = [dt[0]] + [dt[1] + 1] + list(dt[2:7]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dt[1] is attribute tm_mon, but the comment suggests you want to adjust by one hour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the implementation with timedelta(hours=1) instead
… check that it's a valid HTTP date with timezone set to UTC.
…osecond=0) instead of int()
Doc/whatsnew/3.7.rst
Outdated
| adding `~` to the set of characters that is never quoted by default. | ||
| (Contributed by Christian Theune and Ratnadeep Debnath in :issue:`16285`.) | ||
|
|
||
| http.server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move before unittest.mock. Modules should be lexicographically ordered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit 143e1e9
Lib/http/server.py
Outdated
| self.send_response(HTTPStatus.NOT_MODIFIED) | ||
| self.end_headers() | ||
| f.close() | ||
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return None. If there are return statements with value, all None values should be returned explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Lib/http/server.py
Outdated
| # compare If-Modified-Since and time of last file modification | ||
| ims = email.utils.parsedate_to_datetime( | ||
| self.headers["If-Modified-Since"]) | ||
| if (ims is not None and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be written in one line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in c3337f4
Lib/http/server.py
Outdated
| self.send_error(HTTPStatus.NOT_FOUND, "File not found") | ||
| return None | ||
|
|
||
| fs = os.fstat(f.fileno()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f is leaked if os.fstat() raises an exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Lib/http/server.py
Outdated
| if ("If-Modified-Since" in self.headers | ||
| and "If-None-Match" not in self.headers): | ||
| # compare If-Modified-Since and time of last file modification | ||
| ims = email.utils.parsedate_to_datetime( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
email.utils.parsedate_to_datetime() can raise an exception. It might be better just to ignore the cache rather than return an error.
Needed a cache for ill-formed "If-Modified-Since" value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in c3337f4
|
|
||
| try: | ||
| # Use browser cache if possible | ||
| if ("If-Modified-Since" in self.headers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP headers are case-insensitive. Does this work with "if-modified-since"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self.headers should be (similar to) an email.message.Message object, so I think it should work fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirm that it's case-insensitive
Lib/http/server.py
Outdated
| ims = email.utils.parsedate_to_datetime( | ||
| self.headers["If-Modified-Since"]) | ||
| if (ims is not None and | ||
| ims.tzinfo is datetime.timezone.utc): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you should also allow tzinfo to be None, to support the obsolete “asctime” format from https://tools.ietf.org/html/rfc7231#section-7.1.1.1 without a timezone (e.g. Sun Nov 6 08:49:37 1994)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not happen often, but it is supported in commit c3337f4
|
|
||
| try: | ||
| # Use browser cache if possible | ||
| if ("If-Modified-Since" in self.headers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self.headers should be (similar to) an email.message.Message object, so I think it should work fine
…format ; ignore ill-formed values in If-Modified-Since header.
Lib/http/server.py
Outdated
| try: | ||
| ims = email.utils.parsedate_to_datetime( | ||
| self.headers["If-Modified-Since"]) | ||
| except: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never use bare except unless you reraise the exception. This hides such exceptions as KeyboardInterrupt or MemoryError.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the try block because you said parsedate_to_datetime() could raise exceptions ; do you know which ones ? By looking at the code I see that it might raise TypeError because _parsedate_tz may return None and the line
*dtuple, tz = _parsedate_tz(data)would raise a TypeError in this case ; but is it the only one ?
Anyway, the code is itself inside a try...except block where the except block is just except:, so this is not making things much worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That TypeError looks like a bug in the implementation. Other exceptions:
>>> parsedate_to_datetime("Sun, 99 Feb 2017 20:49:31")
ValueError: day is out of range for month
>>> parsedate_to_datetime("Sun, 26 Feb 2017 20:49:31 +12345678901234")
OverflowError: Python int too large to convert to C long
>>> parsedate_to_datetime("Mon, 20 Nov , 19:12:08 GMT")
IndexError: string index out of rangeThe point is the outer exception handler reraises the exception, but your handler swallows it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, the exceptions have been specified in commit 1e68be9
Lib/http/server.py
Outdated
| except: | ||
| # ignore ill-formed values | ||
| ims = None | ||
| if ims is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can email.utils.parsedate_to_datetime() return None? If it can't, just use else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
email.utils.parsedate_to_datetime() either returns a datetime.datetime() or raises an exception, in which case ims is set to None.
| self.assertEqual(response.getheader('content-type'), | ||
| 'application/octet-stream') | ||
|
|
||
| def test_browser_cache(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to split this test on few tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit 42edfe3.
The datetime of last modification is stored in method setUp and used in the browser cache tests. I also added a test for the Last-Modified header.
Lib/test/test_httpservers.py
Outdated
| self.check_status_and_reason(response, HTTPStatus.OK) | ||
|
|
||
| # with If-Modified-Since earlier than Last-Modified, must return 200 | ||
| import datetime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import at module level. Importing inside functions is used in exceptional cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in commit 42edfe3
…several methods ; add a test for Last-Modified header.
| adding `~` to the set of characters that is never quoted by default. | ||
| (Contributed by Christian Theune and Ratnadeep Debnath in :issue:`16285`.) | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't understand what I should do here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed an empty line. Insert it back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, done in commit 9436e45
Lib/http/server.py
Outdated
| except (TypeError, IndexError, OverflowError, ValueError): | ||
| # ignore ill-formed values | ||
| ims = None | ||
| if ims is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just else:
except (TypeError, IndexError, OverflowError, ValueError):
# ignore ill-formed values
pass
else:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in commit fc20596
Lib/http/server.py
Outdated
| else: | ||
| handler_class = SimpleHTTPRequestHandler | ||
| test(HandlerClass=handler_class, port=args.port, bind=args.bind) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't add this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit fc20596
|
Is there something left to do before the pull request can be merged ? |
Doc/whatsnew/3.7.rst
Outdated
|
|
||
| http.server | ||
| ----------- | ||
| :class:`SimpleHTTPRequestHandler` supports the HTTP If-Modified-Since header. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:class:`~http.server.SimpleHTTPRequestHandler`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in 6a58894
Misc/NEWS
Outdated
| - Issue #24142: Reading a corrupt config file left configparser in an | ||
| invalid state. Original patch by Florian Höch. | ||
|
|
||
| - bpo-29654 : Support If-Modified-Since HTTP header (browser cache). Patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the space before colon. Use two spaces after sentence ending period. Move the entry to the start of the "Library" section (new entries are added at the start).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in 6a58894
Lib/http/server.py
Outdated
| import socketserver | ||
| import sys | ||
| import time | ||
| import datetime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to support alphabetical order of imports. I.e. add import datetime before import email.utils.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit 708ff03. The order was not supported for argparse and copy...
I also changed to version to 0.7.
| import urllib.parse | ||
| import html | ||
| import http.client | ||
| import email.message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to support alphabetical order of imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand. In the current version, the imports are not in alphabetic order :
from http.server import BaseHTTPRequestHandler, HTTPServer, \
SimpleHTTPRequestHandler, CGIHTTPRequestHandler
from http import server, HTTPStatus
import os
import sys
import re
import base64
import ntpath
import shutil
import urllib.parse
import html
import http.client
import tempfile
import time
from io import BytesIO
import unittest
from test import supportThere doesn't seem to be any logical order either - http.client is not grouped with http.server for instance.
Would you mind explaining more what you want and why ? It will be helpful for next Pull Requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just place import email.utils the first and import datetime after it. You can also move import base64 before them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a style nit, you shouldn't follow it if you have other reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the order in commit db46671. I tried several orders but couldn't find something obvious. I prefer to leave Internet-related modules together, and time and datetime together.
... and thanks for the expression "style nit", I didn't know it :-)
|
Is there still anything to do ? |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but maybe @vadmium have other comments.
vadmium
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes look okay in general, and I support the idea of the change. I haven’t actually tested this out though (and am unlikely to do so for a while). Just left a couple minor thoughts.
Lib/http/server.py
Outdated
| # at the time the request was made!) | ||
|
|
||
| __version__ = "0.6" | ||
| __version__ = "0.7" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t understand the __version__ change. This is roughly documented as affecting SimpleHTTPRequestHandler.server_version, and the Server response header field. But the current 0.6 value has existed since 2001: revision 077153e. It seems safer to leave it alone unless we know what the numbers mean etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 6dadf6e
Doc/whatsnew/3.7.rst
Outdated
| ----------- | ||
| :class:`~http.server.SimpleHTTPRequestHandler` supports the HTTP | ||
| If-Modified-Since header. The server returns the 304 response status if the | ||
| target file was not modified after the datetime specified in the header. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to just say “after the time specified in the header”? Datetime is just a Python implementation detail, not an English word or part of HTTP.
Also, I wonder if the details should go in the main documentation. Already https://docs.python.org/dev/library/http.server.html#http.server.SimpleHTTPRequestHandler.do_GET has quite a few details of the server behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced "datetime" by "time" in c389bf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update of the http.server documentation proposed in 294e164
|
Do you still have comments @vadmium? |
|
Only other thing that I wonder about is if the documentation should say the Not-Modified handling was added in 3.7 (usually with “.. versionchanged::” markup). But if other people don’t think this needs to be added, I guess it doesn’t matter that much. |
|
I think it's worth mentioning, for instance if a test suite expects a status 200 and gets 304 in version 3.7. It's not a very likely use case, but it may happen. I have added the "Changed in version 3.7" comment in c8b108f. |
|
Can you merge the PR if there is nothing else to do ? |
|
The Travis CI build failed: trailing whitespaces in |
|
Fixed in 16f9e11, all checks pass. |
|
@serhiy-storchaka @vadmium |
The PR adds support of the If-Modified-Since HTTP request header.
If the user agent sends this header and the url matches a file that was not modified after the value specified in the header, the server returns status code 304 (Not Modified).
A test for this feature (test_browser_cache) is added in test_httpservers.py.