From 5e6aa6648b695ca57483ae008c5bf5d4debeb6f3 Mon Sep 17 00:00:00 2001
From: Charlton Austin <charlton.austin@gmail.com>
Date: Tue, 20 Jun 2017 15:50:46 -0400
Subject: [PATCH] fix(endpoints): added in proper error handling

before we would return a 400 without a message because the errors were not being caught

Issue: https://www.pivotaltracker.com/story/show/145459707

- [ ] It works!
- [ ] Comments provide sufficient explanations for the next contributor
- [ ] Tests cover changes and corner cases
- [ ] Follows Quay syntax patterns and format
---
 data/model/label.py       |  2 +-
 endpoints/api/manifest.py | 15 ++++++++++++---
 test/test_api_usage.py    | 14 ++++++++++++++
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/data/model/label.py b/data/model/label.py
index 467eca86f..aef0a9449 100644
--- a/data/model/label.py
+++ b/data/model/label.py
@@ -62,7 +62,7 @@ def create_manifest_label(tag_manifest, key, value, source_type_name, media_type
 
   media_type_id = _get_media_type_id(media_type_name)
   if media_type_id is None:
-    raise InvalidMediaTypeException
+    raise InvalidMediaTypeException()
 
   source_type_id = _get_label_source_type_id(source_type_name)
 
diff --git a/endpoints/api/manifest.py b/endpoints/api/manifest.py
index e96283f7f..66716da3f 100644
--- a/endpoints/api/manifest.py
+++ b/endpoints/api/manifest.py
@@ -10,6 +10,7 @@ from endpoints.exception import NotFound
 from data import model
 
 from digest import digest_tools
+from util.validation import VALID_LABEL_KEY_REGEX
 
 BASE_MANIFEST_ROUTE = '/v1/repository/<apirepopath:repository>/manifest/<regex("{0}"):manifestref>'
 MANIFEST_DIGEST_ROUTE = BASE_MANIFEST_ROUTE.format(digest_tools.DIGEST_PATTERN)
@@ -92,9 +93,17 @@ class RepositoryManifestLabels(RepositoryParamResource):
     if label_validator.has_reserved_prefix(label_data['key']):
       abort(400, message='Label has a reserved prefix')
 
-    label = model.label.create_manifest_label(tag_manifest, label_data['key'],
-                                              label_data['value'], 'api',
-                                              media_type_name=label_data['media_type'])
+    label = None
+    try:
+      label = model.label.create_manifest_label(tag_manifest, label_data['key'],
+                                                label_data['value'], 'api',
+                                                media_type_name=label_data['media_type'])
+    except model.InvalidLabelKeyException:
+      abort(400, message='Label is of an invalid format or missing please use %s format for labels'.format(
+            VALID_LABEL_KEY_REGEX))
+    except model.InvalidMediaTypeException:
+      abort(400, message='Media type is invalid please use a valid media type of text/plain or application/json')
+
     metadata = {
       'id': label.uuid,
       'key': label_data['key'],
diff --git a/test/test_api_usage.py b/test/test_api_usage.py
index 14316d41e..75fed8888 100644
--- a/test/test_api_usage.py
+++ b/test/test_api_usage.py
@@ -4834,6 +4834,20 @@ class TestRepositoryManifestLabels(ApiTestCase):
 
     self.assertEquals(0, len(json['labels']))
 
+    self.postJsonResponse(RepositoryManifestLabels,
+                          params=dict(repository=repository,
+                                      manifestref=tag_manifest.digest),
+                          data=dict(key='bad_label', value='world',
+                                    media_type='text/plain'),
+                          expected_code=400)
+
+    self.postJsonResponse(RepositoryManifestLabels,
+                          params=dict(repository=repository,
+                                      manifestref=tag_manifest.digest),
+                          data=dict(key='hello', value='world',
+                                    media_type='bad_media_type'),
+                          expected_code=400)
+
     # Add some labels to the manifest.
     with assert_action_logged('manifest_label_add'):
       label1 = self.postJsonResponse(RepositoryManifestLabels,