bpo-29654 : Support If-Modified-Since HTTP header (browser cache) #298

PierreQuentel · 2017-02-25T21:25:32Z

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.

…as not modified

the-knights-who-say-ni · 2017-02-25T21:25:33Z

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:

If you don't have an account on b.p.o, please create one
Make sure your GitHub username is listed in "Your Details" at b.p.o
If you have not already done so, please sign the PSF contributor agreement. The "bugs.python.org username " requested by the form is the "Login name" field under "Your Details".
If you just signed the CLA, please wait at least one US business day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

serhiy-storchaka

Please sign the PSF contributor agreement and add entries in Misc/NEWS and What's New file.

serhiy-storchaka · 2017-02-26T06:05:46Z

Lib/http/server.py

+                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(


ims_datetime is naive datetime object, but datetime.datetime.utcfromtimestamp() creates aware datetime object.

serhiy-storchaka · 2017-02-26T06:08:14Z

Lib/http/server.py

+                    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:


Why compare string representations rather than datetime objects or just tuples? The order of such strings is different from the order of datetime objects.

serhiy-storchaka · 2017-02-26T06:08:45Z

Lib/http/server.py

@@ -1212,3 +1228,5 @@ def test(HandlerClass=BaseHTTPRequestHandler,
    else:
        handler_class = SimpleHTTPRequestHandler
    test(HandlerClass=handler_class, port=args.port, bind=args.bind)
+


Unrelated changes.

serhiy-storchaka · 2017-02-26T06:10:31Z

Lib/http/server.py

+            # 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)


Use os.fstat().

Maybe combine with the existing call below, too

vadmium · 2017-02-26T04:20:34Z

Lib/http/server.py

@@ -687,6 +688,21 @@ def send_head(self):
            self.send_error(HTTPStatus.NOT_FOUND, "File not found")
              <
10000
span class="blob-code-inner blob-code-marker-context">            return None

            
        try:
+            # Use browser cache if possible
+            if "If-Modified-Since" in self.headers:


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

vadmium · 2017-02-26T04:31:13Z

Lib/http/server.py

+            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"])


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).

vadmium · 2017-02-26T04:38:30Z

Lib/test/test_httpservers.py

+        #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


Spelling: header. But this comment seems redundant; isn’t the code obvious enough?

vadmium · 2017-02-26T04:39:54Z

Lib/test/test_httpservers.py

+        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)


Please repeat the test with an earlier date, verifying that a response body is returned.

vadmium · 2017-02-26T06:22:58Z

Lib/http/server.py

+            # 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)


Maybe combine with the existing call below, too

…Match is present. Add tests.

PierreQuentel · 2017-02-26T09:11:56Z

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 :

the datetime comparison now uses datetime objects in the UTC timezone, not their string representation. The time of file last modification must be rounded to the second because If-Modified-Since is also rounded
if a If-None-Match header is present, If-Modified-Since is ignored - but at the moment, If-None-Match is not handled
tests have been added

serhiy-storchaka · 2017-02-26T09:04:01Z

Lib/http/server.py

        try:
            # Use browser cache if possible
-            if "If-Modified-Since" in self.headers:
+            if "If-Modified-Since" in self.headers \


It is more preferable to use parenthesis rather than line continuation.

serhiy-storchaka · 2017-02-26T09:04:25Z

Lib/http/server.py

        try:
            # 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:


"If-None-Match" not in self.headers

serhiy-storchaka · 2017-02-26T09:11:08Z

Lib/http/server.py

-                        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))


datetime.timezone.utc

But maybe use naive datetime objects?

serhiy-storchaka · 2017-02-26T09:16:25Z

Lib/http/server.py

                # compare If-Modified-Since and date of last file modification
-                fs = os.stat(path)
                ims = email.utils.parsedate(self.headers["If-Modified-Since"])


It looks a problem to me that email.utils.parsedate() silently ignores timezone. Maybe use email.utils.parsedate_to_datetime() instead?

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.

vadmium · 2017-02-26T09:21:31Z

Lib/test/test_httpservers.py

+        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])


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)”.

Correct, I have modified the test.

vadmium · 2017-02-26T09:48:03Z

Lib/http/server.py

+                    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)


“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).

vadmium · 2017-02-26T10:02:00Z

Lib/test/test_httpservers.py

+        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])


Dt[1] is attribute tm_mon, but the comment suggests you want to adjust by one hour.

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()

serhiy-storchaka · 2017-02-26T10:55:49Z

Doc/whatsnew/3.7.rst

@@ -110,6 +110,12 @@ urllib.parse
 adding `~` to the set of characters that is never quoted by default.
 (Contributed by Christian Theune and Ratnadeep Debnath in :issue:`16285`.)

+http.server


Move before unittest.mock. Modules should be lexicographically ordered.

Done in commit 143e1e9

serhiy-storchaka · 2017-02-26T11:02:01Z

Lib/http/server.py

+                        self.send_response(HTTPStatus.NOT_MODIFIED)
+                        self.end_headers()
+                        f.close()
+                        return


return None. If there are return statements with value, all None values should be returned explicitly.

serhiy-storchaka · 2017-02-26T11:05:28Z

Lib/http/server.py

+                # 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


This can be written in one line.

done in c3337f4

serhiy-storchaka · 2017-02-26T11:08:26Z

Lib/http/server.py

@@ -686,10 +687,30 @@ def send_head(self):
        except OSError:
            self.send_error(HTTPStatus.NOT_FOUND, "File not found")
            return None
+
+        fs = os.fstat(f.fileno())


f is leaked if os.fstat() raises an exception.

serhiy-storchaka · 2017-02-26T11:13:05Z

Lib/http/server.py

+            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(


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.

done in c3337f4

serhiy-storchaka · 2017-02-26T11:15:51Z

Lib/http/server.py

        try:
+            # Use browser cache if possible
+            if ("If-Modified-Since" in self.headers


HTTP headers are case-insensitive. Does this work with "if-modified-since"?

I confirm that it's case-insensitive

vadmium · 2017-02-26T11:26:11Z

Lib/http/server.py

+                ims = email.utils.parsedate_to_datetime(
+                    self.headers["If-Modified-Since"])
+                if (ims is not None and
+                        ims.tzinfo is datetime.timezone.utc):


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)

This should not happen often, but it is supported in commit c3337f4

vadmium · 2017-02-26T11:30:02Z

Lib/http/server.py

        try:
+            # Use browser cache if possible
+            if ("If-Modified-Since" in self.headers


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.

serhiy-storchaka · 2017-02-26T14:29:34Z

Lib/http/server.py

+                try:
+                    ims = email.utils.parsedate_to_datetime(
+                        self.headers["If-Modified-Since"])
+                except:


Never use bare except unless you reraise the exception. This hides such exceptions as KeyboardInterrupt or MemoryError.

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.

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 range

The point is the outer exception handler reraises the exception, but your handler swallows it.

Thanks, the exceptions have been specified in commit 1e68be9

serhiy-storchaka · 2017-02-26T14:34:37Z

Lib/http/server.py

+                except:
+                    # ignore ill-formed values
+                    ims = None
+                if ims is not None:


Can email.utils.parsedate_to_datetime() return None? If it can't, just use else.

email.utils.parsedate_to_datetime() either returns a datetime.datetime() or raises an exception, in which case ims is set to None.

serhiy-storchaka · 2017-02-26T14:42:21Z

Lib/test/test_httpservers.py

@@ -444,6 +444,45 @@ def test_head(self):
        self.assertEqual(response.getheader('content-type'),
                         'application/octet-stream')

+    def test_browser_cache(self):


I think it would be better to split this test on few tests.

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.

serhiy-storchaka · 2017-02-26T14:44:34Z

Lib/test/test_httpservers.py

+        self.check_status_and_reason(response, HTTPStatus.OK)        
+
+        # with If-Modified-Since earlier than Last-Modified, must return 200
+        import datetime


Import at module level. Importing inside functions is used in exceptional cases.

done in commit 42edfe3

…several methods ; add a test for Last-Modified header.

serhiy-storchaka · 2017-02-27T08:20:43Z

Doc/whatsnew/3.7.rst

@@ -110,7 +117,6 @@ urllib.parse
 adding `~` to the set of characters that is never quoted by default.
 (Contributed by Christian Theune and Ratnadeep Debnath in :issue:`16285`.)

-


Unrelated change.

Sorry, I don't understand what I should do here

You removed an empty line. Insert it back.

Ok, done in commit 9436e45

serhiy-storchaka · 2017-02-27T08:22:24Z

Lib/http/server.py

+                except (TypeError, IndexError, OverflowError, ValueError):
+                    # ignore ill-formed values
+                    ims = None
+                if ims is not None:


Just else:

except (TypeError, IndexError, OverflowError, ValueError): # ignore ill-formed values pass else:

done in commit fc20596

serhiy-storchaka · 2017-02-27T08:25:39Z

Lib/http/server.py

@@ -1212,3 +1243,4 @@ def test(HandlerClass=BaseHTTPRequestHandler,
    else:
        handler_class = SimpleHTTPRequestHandler
    test(HandlerClass=handler_class, port=args.port, bind=args.bind)
+


Don't add this line.

Done in commit fc20596

PierreQuentel · 2017-03-01T09:38:24Z

Is there something left to do before the pull request can be merged ?

serhiy-storchaka · 2017-03-01T11:06:01Z

Doc/whatsnew/3.7.rst

@@ -96,6 +96,13 @@ New Modules
 Improved Modules
 ================

+http.server
+-----------
+:class:`SimpleHTTPRequestHandler` supports the HTTP If-Modified-Since header.


:class:`~http.server.SimpleHTTPRequestHandler`

done in 6a58894

serhiy-storchaka · 2017-03-01T11:09:21Z

Misc/NEWS

@@ -666,6 +666,9 @@ Library
 - 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


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).

done in 6a58894

serhiy-storchaka · 2017-03-01T11:12:12Z

Lib/http/server.py

@@ -100,6 +100,7 @@
 import socketserver
 import sys
 import time
+import datetime


Try to support alphabetical order of imports. I.e. add import datetime before import email.utils.

Done in commit 708ff03. The order was not supported for argparse and copy...
I also changed to version to 0.7.

serhiy-storchaka · 2017-03-01T11:13:44Z

Lib/test/test_httpservers.py

@@ -17,8 +17,11 @@
 import urllib.parse
 import html
 import http.client
+import email.message


Try to support alphabetical order of imports.

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 support

There 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.

Just place import email.utils the first and import datetime after it. You can also move import base64 before them.

This is just a style nit, you shouldn't follow it if you have other reasons.

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 :-)

PierreQuentel · 2017-03-04T09:24:11Z

Is there still anything to do ?

serhiy-storchaka

LGTM, but maybe @vadmium have other comments.

vadmium

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.

vadmium · 2017-03-06T22:16:13Z

Lib/http/server.py

@@ -80,13 +80,16 @@
 # (Actually, the latter is only true if you know the server configuration
 # at the time the request was made!)

-__version__ = "0.6"
+__version__ = "0.7"


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.

Done in 6dadf6e

vadmium · 2017-03-06T22:23:54Z

Doc/whatsnew/3.7.rst

+-----------
+: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.


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.

Replaced "datetime" by "time" in c389bf6

Update of the http.server documentation proposed in 294e164

serhiy-storchaka · 2017-03-20T07:22:59Z

Do you still have comments @vadmium?

vadmium · 2017-03-23T10:34:45Z

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.

PierreQuentel · 2017-03-24T08:01:57Z

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.

PierreQuentel · 2017-04-01T11:58:06Z

Can you merge the PR if there is nothing else to do ?

serhiy-storchaka · 2017-04-01T17:24:40Z

The Travis CI build failed: trailing whitespaces in http.server.rst.

PierreQuentel · 2017-04-02T07:36:13Z

Fixed in 16f9e11, all checks pass.

PierreQuentel · 2017-04-03T09:37:46Z

@serhiy-storchaka @vadmium
Thank you for your patience, it was my first PR for this project and I missed things that are obvious to you.

The was a race, when extracting libstackman.a.

Kristján pointed me to another race, but now extracting the archive members should be save.

Support If-Modified-Since HTTP headers, return 304 response if file w…

20cc898

…as not modified

the-knights-who-say-ni added the CLA not signed label Feb 25, 2017

serhiy-storchaka requested changes Feb 26, 2017

View reviewed changes

serhiy-storchaka added the type-feature A feature request or enhancement label Feb 26, 2017

vadmium reviewed Feb 26, 2017

View reviewed changes

Fix bug in datetime comparisons. Ignore If-Modified-Since if If-None-…

f1d8a48

…Match is present. Add tests.

serhiy-storchaka requested changes Feb 26, 2017

View reviewed changes

Pierre Quentel added 2 commits February 26, 2017 10:22

Update Misc/NEWS and What's New

c4403ec

Fix typo

3225080

vadmium reviewed Feb 26, 2017

View reviewed changes

Pierre Quentel added 3 commits February 26, 2017 11:47

Use parsedate_to_datetime to extract datetime from If-Modified-Since,…

415e4af

… check that it's a valid HTTP date with timezone set to UTC.

Change computing of dates used to test browser cache

8595735

Remove microseconds from time of last modification with .replace(micr…

2be2f8c

…osecond=0) instead of int()

serhiy-storchaka requested changes Feb 26, 2017

View reviewed changes

vadmium reviewed Feb 26, 2017

View reviewed changes

Pierre Quentel added 2 commits February 26, 2017 15:20

Put os.fstat() call inside the try block ; handle obsolete HTTP date …

c3337f4

…format ; ignore ill-formed values in If-Modified-Since header.

Restore alphabetical order in section Improved Modules

143e1e9

serhiy-storchaka requested changes Feb 26, 2017

View reviewed changes

Pierre Quentel and others added 2 commits February 26, 2017 16:46

Store last modification date in setUp ; split browser cache tests in …

42edfe3

…several methods ; add a test for Last-Modified header.

Specify the exceptions to catch in parsedate_to_datetime

1e68be9

serhiy-storchaka reviewed Feb 27, 2017

View reviewed changes

Use except/else for parsedate_to_datetime exception handling

fc20596

Mariatta removed the CLA not signed label Feb 27, 2017

the-knights-who-say-ni added the CLA signed label Feb 27, 2017

Restore blank line

341A 9436e45

serhiy-storchaka reviewed Mar 1, 2017

View reviewed changes

Put imports in alphabetical order. Change version to 0.7.

708ff03

PierreQuentel added 2 commits March 2, 2017 09:24

Merge branch 'master' into master

204c503

Merge branch 'master' into master

e2fee49

serhiy-storchaka requested a review from vadmium March 5, 2017 16:02

serhiy-storchaka approved these changes Mar 5, 2017

View reviewed changes

vadmium reviewed Mar 6, 2017

View reviewed changes

Pierre Quentel and others added 7 commits March 7, 2017 08:30

Restore version number 0.6

6dadf6e

Replace "datetime" by "time"

c389bf6

Merge branch 'master' of https://github.com/PierreQuentel/cpython

3cc4f6d

Proposal for an update of the http.server module documentation

294e164

Merge branch 'master' into master

10000

6c5ba39

Merge branch 'master' into master

1e0f3e3

Merge branch 'master' into master

2503add

PierreQuentel and others added 3 commits March 23, 2017 22:24

Merge branch 'master' into master

36a8e2a

Add a "Changed in version 3.7" comment

c8b108f

Merge branch 'master' of https://github.com/PierreQuentel/cpython

bd91346

Merge branch 'master' into master

d3238d2

Pierre Quentel added 2 commits April 1, 2017 22:28

Remove trailing whitespaces

16f9e11

Merge branch 'master' of https://github.com/PierreQuentel/cpython

c669909

serhiy-storchaka merged commit 351adda into python:master Apr 2, 2017

akruis pushed a commit to akruis/cpython that referenced this pull request Aug 4, 2021

Stackless issue python#298: Fix a race in the build process

0feca7a

The was a race, when extracting libstackman.a.

akruis pushed a commit to akruis/cpython that referenced this pull request Aug 8, 2021

Stackless issue python#298: Fix a race in the build process

61e405e

Kristján pointed me to another race, but now extracting the archive members should be save.

jaraco pushed a commit that referenced this pull request Dec 2, 2022

Bump sentry-sdk from 0.13.3 to 0.13.4 (#298)

a2337ec

		@@ -110,7 +117,6 @@ urllib.parse
		adding `~` to the set of characters that is never quoted by default.
		(Contributed by Christian Theune and Ratnadeep Debnath in :issue:`16285`.)

Uh oh!

bpo-29654 : Support If-Modified-Since HTTP header (browser cache) #298

bpo-29654 : Support If-Modified-Since HTTP header (browser cache) #298

Uh oh!

Conversation

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!