Jelajahi Sumber

web: Distinguish between missing files and read errors.

Missing files now return a 404, whereas read errors return a 500.

We now do not need to log 404s and 403s, since they will typically be
logged by the WSGI container and the root causes are fairly unambiguous.
Dave Borowitz 14 tahun lalu
induk
melakukan
2d7904e381
3 mengubah file dengan 19 tambahan dan 3 penghapusan
  1. 7 0
      NEWS
  2. 2 1
      dulwich/tests/test_web.py
  3. 10 2
      dulwich/web.py

+ 7 - 0
NEWS

@@ -17,6 +17,13 @@
 
 
   * New public function dulwich.pack.write_pack_header. (Dave Borowitz)
   * New public function dulwich.pack.write_pack_header. (Dave Borowitz)
 
 
+  * Distinguish between missing files and read errors in HTTP server.
+    (Dave Borowitz)
+
+ TESTS
+
+  * Use GitFile when modifying packed-refs in tests. (Dave Borowitz)
+
  API CHANGES
  API CHANGES
 
 
   * ObjectStore.iter_tree_contents now walks contents in depth-first, sorted
   * ObjectStore.iter_tree_contents now walks contents in depth-first, sorted

+ 2 - 1
dulwich/tests/test_web.py

@@ -31,6 +31,7 @@ from dulwich.web import (
     HTTP_OK,
     HTTP_OK,
     HTTP_NOT_FOUND,
     HTTP_NOT_FOUND,
     HTTP_FORBIDDEN,
     HTTP_FORBIDDEN,
+    HTTP_ERROR,
     send_file,
     send_file,
     get_info_refs,
     get_info_refs,
     handle_service_request,
     handle_service_request,
@@ -98,7 +99,7 @@ class DumbHandlersTestCase(WebTestCase):
 
 
         f = TestFile()
         f = TestFile()
         list(send_file(self._req, f, 'text/plain'))
         list(send_file(self._req, f, 'text/plain'))
-        self.assertEquals(HTTP_NOT_FOUND, self._status)
+        self.assertEquals(HTTP_ERROR, self._status)
         self.assertTrue(f.closed)
         self.assertTrue(f.closed)
 
 
     def test_get_info_refs(self):
     def test_get_info_refs(self):

+ 10 - 2
dulwich/web.py

@@ -48,6 +48,7 @@ logger = log_utils.getLogger(__name__)
 HTTP_OK = '200 OK'
 HTTP_OK = '200 OK'
 HTTP_NOT_FOUND = '404 Not Found'
 HTTP_NOT_FOUND = '404 Not Found'
 HTTP_FORBIDDEN = '403 Forbidden'
 HTTP_FORBIDDEN = '403 Forbidden'
+HTTP_ERROR = '500 Internal Server Error'
 
 
 
 
 def date_time_string(timestamp=None):
 def date_time_string(timestamp=None):
@@ -100,7 +101,7 @@ def send_file(req, f, content_type):
         f.close()
         f.close()
     except IOError:
     except IOError:
         f.close()
         f.close()
-        yield req.not_found('Error reading file')
+        yield req.error('Error reading file')
     except:
     except:
         f.close()
         f.close()
         raise
         raise
@@ -128,7 +129,7 @@ def get_loose_object(req, backend, mat):
     try:
     try:
         data = object_store[sha].as_legacy_object()
         data = object_store[sha].as_legacy_object()
     except IOError:
     except IOError:
-        yield req.not_found('Error reading object')
+        yield req.error('Error reading object')
     req.cache_forever()
     req.cache_forever()
     req.respond(HTTP_OK, 'application/x-git-loose-object')
     req.respond(HTTP_OK, 'application/x-git-loose-object')
     yield data
     yield data
@@ -285,6 +286,13 @@ class HTTPGitRequest(object):
         self.respond(HTTP_FORBIDDEN, 'text/plain')
         self.respond(HTTP_FORBIDDEN, 'text/plain')
         return message
         return message
 
 
+    def error(self, message):
+        """Begin a HTTP 500 response and return the text of a message."""
+        self._cache_headers = []
+        logger.error('Error: %s', message)
+        self.respond(HTTP_ERROR, 'text/plain')
+        return message
+
     def nocache(self):
     def nocache(self):
         """Set the response to never be cached by the client."""
         """Set the response to never be cached by the client."""
         self._cache_headers = [
         self._cache_headers = [