diff --git a/tests/test_uploads.py b/tests/test_uploads.py index 9522ebc1e..9b3696fa8 100644 --- a/tests/test_uploads.py +++ b/tests/test_uploads.py @@ -79,15 +79,20 @@ def testGet(self): rv = self.test_client.get('/upload/') self.assertEqual(rv.status_code, 404) - def testBadFileUploads(self): + def testRegularUploads(self): # Loop over some files and the status codes that we are expecting + # Basically it should never be possible to upload a "regular" file. for filename, status_code in ( - ('evil.py', 415), - ('evil', 415), - ('evil.png', 415), - ('green_square.webp', 415)): - - # The reason why we are definisng it in here and not outside + ('evil.py', 501), + ('evil', 501), + ('evil.png', 501), + ('green_square.webp', 501), + ('green_square.png', 501), + ('green_square.jpg', 501), + ('green_square.gif', 501), + ('green_square.bmp', 501)): + + # The reason why we are defining it in here and not outside # this method is that we are setting the filename of the # TestingFileStorage to be the one in the for loop. This way # we can ensure that the filename that we are "uploading" @@ -110,32 +115,6 @@ def files(self): rv = test_client.post('/upload/', data=dict()) self.assertEqual(rv.status_code, status_code) - def testGoodFileUploads(self): - # Loop over some files and the URLs that we are expecting back - for filename, status_code in ( - ('green_square.png', 201), - ('green_square.jpg', 201), - ('green_square.gif', 201), - ('green_square.bmp', 201)): - - class TestingRequest(Request): - """A testing request to use that will return a - TestingFileStorage to test the uploading.""" - @property - def files(self): - d = MultiDict() - f = open(os.path.join('tests', 'fixtures', filename), 'r') - d['image'] = TestingFileStorage(stream=StringIO(f.read()), - filename=filename) - f.close() - return d - - self.app.request_class = TestingRequest - test_client = self.app.test_client() - - rv = test_client.post('/upload/', data=dict()) - self.assertEqual(rv.status_code, status_code) - def testBase64ScreenshotUploads(self): BASE64_PNG = u'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVQYV2P4DwABAQEAWk1v8QAAAABJRU5ErkJggg==' # nopep8 BASE64_PNG_GARBAGE = u'data:image/png;base64,garbage!' @@ -155,7 +134,7 @@ class TestingRequest(Request): @property def form(self): d = MultiDict() - d['screenshot'] = filedata + d['image'] = filedata return d self.app.request_class = TestingRequest diff --git a/webcompat/__init__.py b/webcompat/__init__.py index beb74a5c5..107974e16 100644 --- a/webcompat/__init__.py +++ b/webcompat/__init__.py @@ -15,8 +15,10 @@ app = Flask(__name__, static_url_path='') app.config.from_object('config') -# set limit of 4MB for file uploads -app.config['MAX_CONTENT_LENGTH'] = 4 * 1024 * 1024 +# set limit of 5.5MB for file uploads +# in practice, this is ~4MB (5.5 / 1.37) +# after the data URI is saved to disk +app.config['MAX_CONTENT_LENGTH'] = 5.5 * 1024 * 1024 github = GitHub(app) limiter = Limiter(app) diff --git a/webcompat/api/uploads.py b/webcompat/api/uploads.py index e8d240996..2a28c734a 100644 --- a/webcompat/api/uploads.py +++ b/webcompat/api/uploads.py @@ -17,7 +17,6 @@ from flask import request from io import BytesIO from PIL import Image -from werkzeug.datastructures import FileStorage from werkzeug.exceptions import RequestEntityTooLarge from uuid import uuid4 @@ -44,10 +43,7 @@ def __init__(self, imagedata): def to_image_object(self, imagedata): '''Method to return a Pillow Image object from the raw imagedata.''' try: - # Is this a file uploaded via ? - if isinstance(imagedata, FileStorage): - return Image.open(imagedata) - # Is this base64 encoded screenshot? + # Make sure we're being sent a base64 encoded image if (isinstance(imagedata, unicode) and imagedata.startswith('data:image/')): # Chop off 'data:image/.+;base64,' before decoding @@ -59,7 +55,12 @@ def to_image_object(self, imagedata): abort(415) def get_file_ext(self): - '''Method to return the file extension, as determined by Pillow.''' + '''Method to return the file extension, as determined by Pillow. + + (But, we return jpg for png images, since we convert them always.) + ''' + if self.image_object.format.lower() == 'png': + return 'jpg' return self.image_object.format.lower() def get_filename(self): @@ -105,10 +106,8 @@ def upload(): Returns a JSON string that contains the filename and url. ''' - if 'image' in request.files and request.files['image'].filename: - imagedata = request.files['image'] - elif 'screenshot' in request.form: - imagedata = request.form['screenshot'] + if 'image' in request.form: + imagedata = request.form['image'] else: # We don't know what you're trying to do, but it ain't gonna work. abort(501) diff --git a/webcompat/static/css/development/components/loader.css b/webcompat/static/css/development/components/loader.css index 15a479f80..aeeb85dee 100644 --- a/webcompat/static/css/development/components/loader.css +++ b/webcompat/static/css/development/components/loader.css @@ -15,3 +15,15 @@ display:block; background-color:rgba(255,255,255,.8); } + +.wc-Upload-Loader { + display:none; + position:absolute; + right:4%;top: 27%; + width: 24px; + height: 24px; +} + +.wc-Upload-Loader.is-active { + display: block; +} \ No newline at end of file diff --git a/webcompat/static/img/upload-loader.svg b/webcompat/static/img/upload-loader.svg new file mode 100644 index 000000000..196091fc8 --- /dev/null +++ b/webcompat/static/img/upload-loader.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/webcompat/static/js/lib/bugform.js b/webcompat/static/js/lib/bugform.js index 5e6929835..2e1d1e29c 100644 --- a/webcompat/static/js/lib/bugform.js +++ b/webcompat/static/js/lib/bugform.js @@ -12,6 +12,7 @@ function BugForm() { this.loadingIndicator = $('.js-Loader'); this.reportButton = $('#js-ReportBug'); this.loaderImage = $('.js-Loader'); + this.uploadLoader = $('.js-Upload-Loader'); this.screenshotData = ''; // by default, submission type is anonymous this.submitType = 'github-proxy-report'; @@ -65,13 +66,9 @@ function BugForm() { // Set up listener for message events from screenshot-enabled add-ons window.addEventListener('message', _.bind(function(event) { - var img = document.createElement('img'); - var canvas = document.createElement('canvas'); - var ctx = canvas.getContext('2d'); // Make sure the data is coming from ~*inside the house*~! // (i.e., our add-on sent it) if (location.origin === event.origin) { - this.form.on('submit', _.bind(this.submitFormWithScreenshot, this)); this.screenshotData = event.data; // The final size of Base64-encoded binary data is ~equal to @@ -79,62 +76,44 @@ function BugForm() { // so, bytes = (encoded_string.length - 814) / 1.37) // see https://en.wikipedia.org/wiki/Base64#MIME if ((String(this.screenshotData).length - 814 / 1.37) > this.UPLOAD_LIMIT) { - img.onload = _.bind(function() { - // scale the tmp canvas to 50% - canvas.width = Math.floor(img.width / 2); - canvas.height = Math.floor(img.height / 2); - ctx.scale(0.5, 0.5); - // draw back in the screenshot (at 50% scale) - // and re-serialize to data URI - ctx.drawImage(img, 0, 0); - this.screenshotData = canvas.toDataURL(); - img = null, canvas = null; - - this.addPreviewBackground(this.screenshotData); - }, this); - - img.src = this.screenshotData; + this.downsampleImageAndUpload(this.screenshotData); } else { - this.addPreviewBackground(this.screenshotData); + this.addPreviewBackgroundAndUpload(this.screenshotData); } } }, this), false); }; - // If we've gotten this far, form validation has happened and - // we're reacting to a submit event. - this.submitFormWithScreenshot = function(event) { - // Stop regular form submission - event.preventDefault(); - // construct a FormData object and append the base64 image to it. - var formdata = new FormData(this.form.get(0)); - // we call this 'screenshot', rather than 'image' because it needs to be - // handled differently on the backend. - formdata.append('screenshot', this.screenshotData); - // add in the submit-type, which won't be included in JS form submission by default - formdata.append('submit-type', this.submitType); - - this.loaderImage.show(); - $.ajax({ - contentType: false, - processData: false, - data: formdata, - method: 'POST', - url: '/issues/new', - success: _.bind(function(response) { - // navigate to thanks page. - this.loaderImage.hide(); - window.location.href = '/thanks/' + response.number; - }, this), - error: _.bind(function(response) { - var msg = 'There was an error trying to file the bug, try again?.'; - if (response && response.status === 413) { - msg = 'The image is too big! Please choose something smaller than 4MB.'; - } - wcEvents.trigger('flash:error', {message: msg, timeout: 5000}); - this.loaderImage.hide(); - }, this) - }); + this.downsampleImageAndUpload = function(dataURI) { + var img = document.createElement('img'); + var canvas = document.createElement('canvas'); + var ctx = canvas.getContext('2d'); + + img.onload = _.bind(function() { + // scale the tmp canvas to 50% + canvas.width = Math.floor(img.width / 2); + canvas.height = Math.floor(img.height / 2); + ctx.scale(0.5, 0.5); + // draw back in the screenshot (at 50% scale) + // and re-serialize to data URI + ctx.drawImage(img, 0, 0); + // Note: this will convert GIFs to JPEG, which breaks + // animated GIFs. However, this only will happen if they + // were above the upload limit size. So... sorry? + this.screenshotData = canvas.toDataURL('image/jpeg', 0.8); + + // The limit is 4MB (which is crazy big!), so let the user know if their + // file is unreasonably large at this point (after 1 round of downsampling) + if (this.screenshotData > this.UPLOAD_LIMIT) { + this.makeInvalid('img_too_big'); + return; + } + + this.addPreviewBackgroundAndUpload(this.screenshotData); + img = null, canvas = null; + }, this); + + img.src = dataURI; }; this.checkParams = function() { @@ -298,14 +277,6 @@ function BugForm() { // We can just grab the 0th one, because we only allow uploading // a single image at a time (for now) var img = event.target.files[0]; - // The limit is 4MB (which is crazy big!), so let the user know if their - // file is unreasonably large. - if (img.size > this.UPLOAD_LIMIT) { - this.makeInvalid('img_too_big'); - return; - } else if (img.size < this.UPLOAD_LIMIT) { - this.makeValid('img_too_big'); - } // One last image type validation check. if (!img.type.match('image.*')) { @@ -315,12 +286,17 @@ function BugForm() { var reader = new FileReader(); reader.onload = _.bind(function(event) { - this.addPreviewBackground(event.target.result); + var dataURI = event.target.result; + if ((String(dataURI).length - 814 / 1.37) > this.UPLOAD_LIMIT) { + this.downsampleImageAndUpload(dataURI); + } else { + this.addPreviewBackgroundAndUpload(dataURI); + } }, this); reader.readAsDataURL(img); }; - this.addPreviewBackground = function(dataURI) { + this.addPreviewBackgroundAndUpload = function(dataURI) { if (!_.startsWith(dataURI, 'data:image/')) { return; } @@ -332,6 +308,7 @@ function BugForm() { }); this.showRemoveUpload(label); + this.getUploadURL(dataURI); }; /* Allow users to remove an image from the form upload. @@ -347,16 +324,69 @@ function BugForm() { removeBanner.removeClass('is-hidden'); uploadWrapper.addClass('is-hidden'); removeBanner.on('click', _.bind(function() { - // clear out the input value or screenshot data - this.uploadField.val(this.uploadField.get(0).defaultValue); - this.screenshotData = ''; // remove the preview and hide the banner label.css('background', 'none'); removeBanner.addClass('is-hidden'); uploadWrapper.removeClass('is-hidden'); removeBanner.off('click'); + + // remove the last embedded image URL + // Note: this could fail in weird ways depending on how + // the user has edited the descField. + this.descField.val(function(idx, value) { + return value.replace(/!\[.+\.jpe*g\)$/, ''); + }); }, this)); }; + /* + Upload the image before form submission so we can + put an image link in the bug description. + */ + this.getUploadURL = function(dataURI) { + this.disableSubmits(); + this.uploadLoader.addClass('is-active'); + var formdata = new FormData(); + formdata.append('image', dataURI); + + $.ajax({ + contentType: false, + processData: false, + data: formdata, + method: 'POST', + url: '/upload/', + success: _.bind(function(response) { + this.addImageURL(response.url); + this.uploadLoader.removeClass('is-active'); + this.enableSubmits(); + }, this), + error: _.bind(function(response) { + var msg; + if (response && response.status === 415) { + wcEvents.trigger('flash:error', + {message: this.inputMap.image.helpText, timeout: 5000}); + } + + if (response && response.status === 413) { + msg = 'The image is too big! Please choose something smaller than 4MB.'; + wcEvents.trigger('flash:error', {message: msg, timeout: 5000}); + } + this.loaderImage.hide(); + }, this) + }); + + // clear out the input[type=file], because we don't need it anymore. + this.uploadField.val(this.uploadField.get(0).defaultValue); + }; + /* + copy over the URL of a newly uploaded image asset to the bug + description textarea. + */ + this.addImageURL = function(url) { + var imageURL = ['![Screenshot Description](', url, ')'].join(''); + this.descField.val(function(idx, value) { + return value + '\n\n' + imageURL; + }); + }; /* copy URL from urlField into the first line of the description field. early return if the user has deleted @@ -371,7 +401,6 @@ function BugForm() { } return value.replace(firstLine, prefix + this.urlField.val() + '\n'); }, this)); - }; return this.init(); diff --git a/webcompat/templates/home-page/form.html b/webcompat/templates/home-page/form.html index 5f68948a3..d6d49d6b9 100644 --- a/webcompat/templates/home-page/form.html +++ b/webcompat/templates/home-page/form.html @@ -72,7 +72,11 @@ {% endif %} {{ form.image(class_='wc-ButtonUpload', accept='.jpe,.jpg,.jpeg,.gif,.png,.bmp') }} - + diff --git a/webcompat/views.py b/webcompat/views.py index 6d8e88d89..0e62f8680 100644 --- a/webcompat/views.py +++ b/webcompat/views.py @@ -160,10 +160,6 @@ def create_issue(): # copy the form so we can add the full UA string to it. form = request.form.copy() form['ua_header'] = request.headers.get('User-Agent') - # Do we have an image or screenshot ready to be uploaded? - if ((request.files['image'] and request.files['image'].filename) or - request.form.get('screenshot')): - form['image_upload'] = json.loads(upload()[0]) if form.get('submit-type') == AUTH_REPORT: if g.user: # If you're already authed, submit the bug. response = report_issue(form)