Send blob image using Ajax/PHP
up vote
2
down vote
favorite
I have an image cropper that crops the uploaded image, Before sending it to the server using PHP and AJAX.
Here is a live fiddle to preview a working cropping example.
Here is the code:
// Create cropped part.
function getRoundedCanvas(sourceCanvas) {
var canvas = document.createElement('canvas');
var context = canvas.getContext('2d');
var width = sourceCanvas.width;
var height = sourceCanvas.height;
canvas.width = width;
canvas.height = height;
context.imageSmoothingEnabled = true;
context.drawImage(sourceCanvas, 0, 0, width, height);
context.globalCompositeOperation = 'destination-in';
context.beginPath();
context.arc(width / 2, height / 2, Math.min(width, height) / 2, 0, 2 * Math.PI, true);
context.fill();
return canvas;
}
// On uploading a file
$("#input").on("change", function(e) {
var _URL = window.URL || window.webkitURL,
file = this.files[0],
image = new Image();
image.src = _URL.createObjectURL(file);
image.onload = function(e) {
var image = document.getElementById('image'),
button = document.getElementById('button');
$('#image').attr('src', _URL.createObjectURL(file));
$('#image').show();
$('#button').show();
var cropper = new Cropper(image, {
aspectRatio: 1,
movable: false,
cropBoxResizable: true,
dragMode: 'move',
ready: function () {
croppable = true;
button.onclick = function () {
var croppedCanvas;
var roundedCanvas;
var roundedImage;
if (!croppable) {
return;
}
// Crop
croppedCanvas = cropper.getCroppedCanvas();
cropper.getCroppedCanvas({
fillColor: '#fff',
imageSmoothingEnabled: true,
imageSmoothingQuality: 'high',
});
// Round
roundedCanvas = getRoundedCanvas(croppedCanvas);
// Show
roundedImage = document.createElement('img');
roundedImage.src = roundedCanvas.toDataURL();
result.innerHTML = '';
result.appendChild(roundedImage);
}
}
});
}
}); #image,
#button{
display:none
}<!-- Cropper CSS -->
<link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.css">
<!-- Cropper JS -->
<script src="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.js"></script>
<script src="http://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.js"></script>
<!-- File input -->
<input type="file" id="input" name="image">
<!-- Selected image will be displayed here -->
<img id="image" src="" alt="Picture">
<!-- Button to scrop the image -->
<button type="button" id="button">Crop</button>
<!-- Preview cropped part -->
<div id="result"></div>Upload an image and click crop, The cropped part would be displayed beneath it.
Then I use the following code to send a blob to PHP using Ajax:
cropper.getCroppedCanvas().toBlob(function (blob) {
var formData = new FormData();
formData.append('avatar', blob);
// Use `jQuery.ajax` method
$.ajax('upload.php', {
method: "POST",
data: formData,
processData: false,
contentType: false,
success: function (response) {
},
error: function () {
}
});
});
In upload.php:
if( isset($_FILES['avatar']) and !$_FILES['avatar']['error'] ){
file_put_contents( "uploads/image.png", file_get_contents($_FILES['avatar']['tmp_name']) );
}
Should I make some checks for security or just the dimensions and size checks?
javascript php jquery image ajax
add a comment |
up vote
2
down vote
favorite
I have an image cropper that crops the uploaded image, Before sending it to the server using PHP and AJAX.
Here is a live fiddle to preview a working cropping example.
Here is the code:
// Create cropped part.
function getRoundedCanvas(sourceCanvas) {
var canvas = document.createElement('canvas');
var context = canvas.getContext('2d');
var width = sourceCanvas.width;
var height = sourceCanvas.height;
canvas.width = width;
canvas.height = height;
context.imageSmoothingEnabled = true;
context.drawImage(sourceCanvas, 0, 0, width, height);
context.globalCompositeOperation = 'destination-in';
context.beginPath();
context.arc(width / 2, height / 2, Math.min(width, height) / 2, 0, 2 * Math.PI, true);
context.fill();
return canvas;
}
// On uploading a file
$("#input").on("change", function(e) {
var _URL = window.URL || window.webkitURL,
file = this.files[0],
image = new Image();
image.src = _URL.createObjectURL(file);
image.onload = function(e) {
var image = document.getElementById('image'),
button = document.getElementById('button');
$('#image').attr('src', _URL.createObjectURL(file));
$('#image').show();
$('#button').show();
var cropper = new Cropper(image, {
aspectRatio: 1,
movable: false,
cropBoxResizable: true,
dragMode: 'move',
ready: function () {
croppable = true;
button.onclick = function () {
var croppedCanvas;
var roundedCanvas;
var roundedImage;
if (!croppable) {
return;
}
// Crop
croppedCanvas = cropper.getCroppedCanvas();
cropper.getCroppedCanvas({
fillColor: '#fff',
imageSmoothingEnabled: true,
imageSmoothingQuality: 'high',
});
// Round
roundedCanvas = getRoundedCanvas(croppedCanvas);
// Show
roundedImage = document.createElement('img');
roundedImage.src = roundedCanvas.toDataURL();
result.innerHTML = '';
result.appendChild(roundedImage);
}
}
});
}
}); #image,
#button{
display:none
}<!-- Cropper CSS -->
<link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.css">
<!-- Cropper JS -->
<script src="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.js"></script>
<script src="http://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.js"></script>
<!-- File input -->
<input type="file" id="input" name="image">
<!-- Selected image will be displayed here -->
<img id="image" src="" alt="Picture">
<!-- Button to scrop the image -->
<button type="button" id="button">Crop</button>
<!-- Preview cropped part -->
<div id="result"></div>Upload an image and click crop, The cropped part would be displayed beneath it.
Then I use the following code to send a blob to PHP using Ajax:
cropper.getCroppedCanvas().toBlob(function (blob) {
var formData = new FormData();
formData.append('avatar', blob);
// Use `jQuery.ajax` method
$.ajax('upload.php', {
method: "POST",
data: formData,
processData: false,
contentType: false,
success: function (response) {
},
error: function () {
}
});
});
In upload.php:
if( isset($_FILES['avatar']) and !$_FILES['avatar']['error'] ){
file_put_contents( "uploads/image.png", file_get_contents($_FILES['avatar']['tmp_name']) );
}
Should I make some checks for security or just the dimensions and size checks?
javascript php jquery image ajax
add a comment |
up vote
2
down vote
favorite
up vote
2
down vote
favorite
I have an image cropper that crops the uploaded image, Before sending it to the server using PHP and AJAX.
Here is a live fiddle to preview a working cropping example.
Here is the code:
// Create cropped part.
function getRoundedCanvas(sourceCanvas) {
var canvas = document.createElement('canvas');
var context = canvas.getContext('2d');
var width = sourceCanvas.width;
var height = sourceCanvas.height;
canvas.width = width;
canvas.height = height;
context.imageSmoothingEnabled = true;
context.drawImage(sourceCanvas, 0, 0, width, height);
context.globalCompositeOperation = 'destination-in';
context.beginPath();
context.arc(width / 2, height / 2, Math.min(width, height) / 2, 0, 2 * Math.PI, true);
context.fill();
return canvas;
}
// On uploading a file
$("#input").on("change", function(e) {
var _URL = window.URL || window.webkitURL,
file = this.files[0],
image = new Image();
image.src = _URL.createObjectURL(file);
image.onload = function(e) {
var image = document.getElementById('image'),
button = document.getElementById('button');
$('#image').attr('src', _URL.createObjectURL(file));
$('#image').show();
$('#button').show();
var cropper = new Cropper(image, {
aspectRatio: 1,
movable: false,
cropBoxResizable: true,
dragMode: 'move',
ready: function () {
croppable = true;
button.onclick = function () {
var croppedCanvas;
var roundedCanvas;
var roundedImage;
if (!croppable) {
return;
}
// Crop
croppedCanvas = cropper.getCroppedCanvas();
cropper.getCroppedCanvas({
fillColor: '#fff',
imageSmoothingEnabled: true,
imageSmoothingQuality: 'high',
});
// Round
roundedCanvas = getRoundedCanvas(croppedCanvas);
// Show
roundedImage = document.createElement('img');
roundedImage.src = roundedCanvas.toDataURL();
result.innerHTML = '';
result.appendChild(roundedImage);
}
}
});
}
}); #image,
#button{
display:none
}<!-- Cropper CSS -->
<link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.css">
<!-- Cropper JS -->
<script src="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.js"></script>
<script src="http://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.js"></script>
<!-- File input -->
<input type="file" id="input" name="image">
<!-- Selected image will be displayed here -->
<img id="image" src="" alt="Picture">
<!-- Button to scrop the image -->
<button type="button" id="button">Crop</button>
<!-- Preview cropped part -->
<div id="result"></div>Upload an image and click crop, The cropped part would be displayed beneath it.
Then I use the following code to send a blob to PHP using Ajax:
cropper.getCroppedCanvas().toBlob(function (blob) {
var formData = new FormData();
formData.append('avatar', blob);
// Use `jQuery.ajax` method
$.ajax('upload.php', {
method: "POST",
data: formData,
processData: false,
contentType: false,
success: function (response) {
},
error: function () {
}
});
});
In upload.php:
if( isset($_FILES['avatar']) and !$_FILES['avatar']['error'] ){
file_put_contents( "uploads/image.png", file_get_contents($_FILES['avatar']['tmp_name']) );
}
Should I make some checks for security or just the dimensions and size checks?
javascript php jquery image ajax
I have an image cropper that crops the uploaded image, Before sending it to the server using PHP and AJAX.
Here is a live fiddle to preview a working cropping example.
Here is the code:
// Create cropped part.
function getRoundedCanvas(sourceCanvas) {
var canvas = document.createElement('canvas');
var context = canvas.getContext('2d');
var width = sourceCanvas.width;
var height = sourceCanvas.height;
canvas.width = width;
canvas.height = height;
context.imageSmoothingEnabled = true;
context.drawImage(sourceCanvas, 0, 0, width, height);
context.globalCompositeOperation = 'destination-in';
context.beginPath();
context.arc(width / 2, height / 2, Math.min(width, height) / 2, 0, 2 * Math.PI, true);
context.fill();
return canvas;
}
// On uploading a file
$("#input").on("change", function(e) {
var _URL = window.URL || window.webkitURL,
file = this.files[0],
image = new Image();
image.src = _URL.createObjectURL(file);
image.onload = function(e) {
var image = document.getElementById('image'),
button = document.getElementById('button');
$('#image').attr('src', _URL.createObjectURL(file));
$('#image').show();
$('#button').show();
var cropper = new Cropper(image, {
aspectRatio: 1,
movable: false,
cropBoxResizable: true,
dragMode: 'move',
ready: function () {
croppable = true;
button.onclick = function () {
var croppedCanvas;
var roundedCanvas;
var roundedImage;
if (!croppable) {
return;
}
// Crop
croppedCanvas = cropper.getCroppedCanvas();
cropper.getCroppedCanvas({
fillColor: '#fff',
imageSmoothingEnabled: true,
imageSmoothingQuality: 'high',
});
// Round
roundedCanvas = getRoundedCanvas(croppedCanvas);
// Show
roundedImage = document.createElement('img');
roundedImage.src = roundedCanvas.toDataURL();
result.innerHTML = '';
result.appendChild(roundedImage);
}
}
});
}
}); #image,
#button{
display:none
}<!-- Cropper CSS -->
<link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.css">
<!-- Cropper JS -->
<script src="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.js"></script>
<script src="http://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.js"></script>
<!-- File input -->
<input type="file" id="input" name="image">
<!-- Selected image will be displayed here -->
<img id="image" src="" alt="Picture">
<!-- Button to scrop the image -->
<button type="button" id="button">Crop</button>
<!-- Preview cropped part -->
<div id="result"></div>Upload an image and click crop, The cropped part would be displayed beneath it.
Then I use the following code to send a blob to PHP using Ajax:
cropper.getCroppedCanvas().toBlob(function (blob) {
var formData = new FormData();
formData.append('avatar', blob);
// Use `jQuery.ajax` method
$.ajax('upload.php', {
method: "POST",
data: formData,
processData: false,
contentType: false,
success: function (response) {
},
error: function () {
}
});
});
In upload.php:
if( isset($_FILES['avatar']) and !$_FILES['avatar']['error'] ){
file_put_contents( "uploads/image.png", file_get_contents($_FILES['avatar']['tmp_name']) );
}
Should I make some checks for security or just the dimensions and size checks?
// Create cropped part.
function getRoundedCanvas(sourceCanvas) {
var canvas = document.createElement('canvas');
var context = canvas.getContext('2d');
var width = sourceCanvas.width;
var height = sourceCanvas.height;
canvas.width = width;
canvas.height = height;
context.imageSmoothingEnabled = true;
context.drawImage(sourceCanvas, 0, 0, width, height);
context.globalCompositeOperation = 'destination-in';
context.beginPath();
context.arc(width / 2, height / 2, Math.min(width, height) / 2, 0, 2 * Math.PI, true);
context.fill();
return canvas;
}
// On uploading a file
$("#input").on("change", function(e) {
var _URL = window.URL || window.webkitURL,
file = this.files[0],
image = new Image();
image.src = _URL.createObjectURL(file);
image.onload = function(e) {
var image = document.getElementById('image'),
button = document.getElementById('button');
$('#image').attr('src', _URL.createObjectURL(file));
$('#image').show();
$('#button').show();
var cropper = new Cropper(image, {
aspectRatio: 1,
movable: false,
cropBoxResizable: true,
dragMode: 'move',
ready: function () {
croppable = true;
button.onclick = function () {
var croppedCanvas;
var roundedCanvas;
var roundedImage;
if (!croppable) {
return;
}
// Crop
croppedCanvas = cropper.getCroppedCanvas();
cropper.getCroppedCanvas({
fillColor: '#fff',
imageSmoothingEnabled: true,
imageSmoothingQuality: 'high',
});
// Round
roundedCanvas = getRoundedCanvas(croppedCanvas);
// Show
roundedImage = document.createElement('img');
roundedImage.src = roundedCanvas.toDataURL();
result.innerHTML = '';
result.appendChild(roundedImage);
}
}
});
}
}); #image,
#button{
display:none
}<!-- Cropper CSS -->
<link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.css">
<!-- Cropper JS -->
<script src="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.js"></script>
<script src="http://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.js"></script>
<!-- File input -->
<input type="file" id="input" name="image">
<!-- Selected image will be displayed here -->
<img id="image" src="" alt="Picture">
<!-- Button to scrop the image -->
<button type="button" id="button">Crop</button>
<!-- Preview cropped part -->
<div id="result"></div>// Create cropped part.
function getRoundedCanvas(sourceCanvas) {
var canvas = document.createElement('canvas');
var context = canvas.getContext('2d');
var width = sourceCanvas.width;
var height = sourceCanvas.height;
canvas.width = width;
canvas.height = height;
context.imageSmoothingEnabled = true;
context.drawImage(sourceCanvas, 0, 0, width, height);
context.globalCompositeOperation = 'destination-in';
context.beginPath();
context.arc(width / 2, height / 2, Math.min(width, height) / 2, 0, 2 * Math.PI, true);
context.fill();
return canvas;
}
// On uploading a file
$("#input").on("change", function(e) {
var _URL = window.URL || window.webkitURL,
file = this.files[0],
image = new Image();
image.src = _URL.createObjectURL(file);
image.onload = function(e) {
var image = document.getElementById('image'),
button = document.getElementById('button');
$('#image').attr('src', _URL.createObjectURL(file));
$('#image').show();
$('#button').show();
var cropper = new Cropper(image, {
aspectRatio: 1,
movable: false,
cropBoxResizable: true,
dragMode: 'move',
ready: function () {
croppable = true;
button.onclick = function () {
var croppedCanvas;
var roundedCanvas;
var roundedImage;
if (!croppable) {
return;
}
// Crop
croppedCanvas = cropper.getCroppedCanvas();
cropper.getCroppedCanvas({
fillColor: '#fff',
imageSmoothingEnabled: true,
imageSmoothingQuality: 'high',
});
// Round
roundedCanvas = getRoundedCanvas(croppedCanvas);
// Show
roundedImage = document.createElement('img');
roundedImage.src = roundedCanvas.toDataURL();
result.innerHTML = '';
result.appendChild(roundedImage);
}
}
});
}
}); #image,
#button{
display:none
}<!-- Cropper CSS -->
<link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.css">
<!-- Cropper JS -->
<script src="https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.4.1/cropper.min.js"></script>
<script src="http://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.js"></script>
<!-- File input -->
<input type="file" id="input" name="image">
<!-- Selected image will be displayed here -->
<img id="image" src="" alt="Picture">
<!-- Button to scrop the image -->
<button type="button" id="button">Crop</button>
<!-- Preview cropped part -->
<div id="result"></div>javascript php jquery image ajax
javascript php jquery image ajax
edited Sep 6 at 17:33
200_success
127k15149412
127k15149412
asked Sep 6 at 16:51
Bon
111
111
add a comment |
add a comment |
1 Answer
1
active
oldest
votes
up vote
0
down vote
Should I make some checks for security or just the dimensions and size checks?
It would be simple to check the type of the file uploaded. For example, you could check if the type is an image. For example, if the user uploads a PNG file, $_FILES['avatar']['type'] should be image/png, so a check for that value starting with image would be a simple conditional (e.g. using strpos() and looking for it to return 0, or preg_match('^image/', $_FILES['avatar']['type]).
The HTML could also be updated to only accept certain file types. One technique for this is to add the accept attribute to the input element with a value like image/* to limit the selectable files to images. While this does not prevent a user from selecting any file (or modifying a file to look like an image), it can act like a filter to the file selection dialog if the OS and browser support it. Read more about this in answers to Limit file format when using ?
<input type="file" id="input" name="image" accept="image/*">One could also add a change handler to check the extension of the file submitted (bear in mind that the user can also change the extension of a file through the OS).
var allowedExtensions = ['jpg', 'gif', 'bmp', 'png', 'tif'];
document.addEventListener('DOMContentLoaded', function() {
var fileInput = document.getElementById('input');
fileInput.addEventListener('change', function(e) {
var parts = this.value.split('.');
if (allowedExtensions.indexOf(parts[parts.length - 1].toLowerCase()) < 0) {
this.value = ''; // clear value
console.log('invalid extension');
}
});
});<input type="file" id="input" name="image" accept="image/*">Combining one or both of those approaches with the server-side check would be a good approach. Bearing in mind that a user could alter the HTML attributes with tools like browser consoles, as well as submit a form manually with their own page or a tool like Postman, the server-side check should be utilized for optimal protection.
Other Feedback
JavaScript
Variable croppable
It apppears that croppable is set to true in the ready function:
ready: function () {
croppable = true;
button.onclick = function () {
While it would be undefined before that line, the onclick function wouldn't be set until after that is set to true, so the following block seems superflous in the onclick function:
if (!croppable) {
return;
}
This is because by the time the click handler is set on the button, that variable will have been set to true so the condition will never evaluate to a truthy value.
I also noticed in that onclick function that three variables (i.e. croppedCanvas, roundedCanvas, and roundedImage) are declared before the conditional return. There isn't really any reason to declare those variables before the condition. Then those variables can be declared whenever assigned.
Cache DOM references
DOM lookups aren't exactly cheap, so it may be better to store references to elements fetched via document.getElementById() as well as using the jQuery function in variables and referencing those variables later.
Use consistent DOM lookups
Much of the code uses jQuery to look up elements, but then there are other places using document.getElementById(). If you are going to have jQuery, it makes sense to use it for all DOM lookups - for the case of getting a single element (e.g. image and button) .get() can be used to access the element.
So these lines:
var image = document.getElementById('image'),
button = document.getElementById('button');
Can be simplified using jQuery and .get():
var image = $('#image').get(0),
button = $('#button').get(0);
Variable name reused
While moving image and button out of the onload callback I noticed that image was used for an Image object and then inside the callback re-assigned to the element with id attribute image. It is wise not to re-use variables like that... Perhaps a better name for the element would be imageElement.
1
Thanks for the answer, But I didn't get exactly what you said aboutcroppable, Bothcroppableandonclickare inside thereadyfunction, Do you mean that it will always be true, So I don't need that condition?
– Bon
Sep 7 at 4:40
Yes- does the updated answer provide a better explanation?
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 5:15
I copied the code from couple examples from the plugin developer, So it's not ordered/cleaned properly.
– Bon
Sep 7 at 18:25
Ah yes... the code looks very similar to the upload-cropped-image-to-server.html. It is a good thing you included your PHP code - otherwise it would be like a review of fengyuanchen's cropperjs code. In the future it might be advisable to disclose the original source of the code
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 18:37
Actually it's a combination from 3 examples, Modal, Upload to server and Round image, So it's a little messy :)
– Bon
Sep 8 at 5:40
|
show 2 more comments
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
0
down vote
Should I make some checks for security or just the dimensions and size checks?
It would be simple to check the type of the file uploaded. For example, you could check if the type is an image. For example, if the user uploads a PNG file, $_FILES['avatar']['type'] should be image/png, so a check for that value starting with image would be a simple conditional (e.g. using strpos() and looking for it to return 0, or preg_match('^image/', $_FILES['avatar']['type]).
The HTML could also be updated to only accept certain file types. One technique for this is to add the accept attribute to the input element with a value like image/* to limit the selectable files to images. While this does not prevent a user from selecting any file (or modifying a file to look like an image), it can act like a filter to the file selection dialog if the OS and browser support it. Read more about this in answers to Limit file format when using ?
<input type="file" id="input" name="image" accept="image/*">One could also add a change handler to check the extension of the file submitted (bear in mind that the user can also change the extension of a file through the OS).
var allowedExtensions = ['jpg', 'gif', 'bmp', 'png', 'tif'];
document.addEventListener('DOMContentLoaded', function() {
var fileInput = document.getElementById('input');
fileInput.addEventListener('change', function(e) {
var parts = this.value.split('.');
if (allowedExtensions.indexOf(parts[parts.length - 1].toLowerCase()) < 0) {
this.value = ''; // clear value
console.log('invalid extension');
}
});
});<input type="file" id="input" name="image" accept="image/*">Combining one or both of those approaches with the server-side check would be a good approach. Bearing in mind that a user could alter the HTML attributes with tools like browser consoles, as well as submit a form manually with their own page or a tool like Postman, the server-side check should be utilized for optimal protection.
Other Feedback
JavaScript
Variable croppable
It apppears that croppable is set to true in the ready function:
ready: function () {
croppable = true;
button.onclick = function () {
While it would be undefined before that line, the onclick function wouldn't be set until after that is set to true, so the following block seems superflous in the onclick function:
if (!croppable) {
return;
}
This is because by the time the click handler is set on the button, that variable will have been set to true so the condition will never evaluate to a truthy value.
I also noticed in that onclick function that three variables (i.e. croppedCanvas, roundedCanvas, and roundedImage) are declared before the conditional return. There isn't really any reason to declare those variables before the condition. Then those variables can be declared whenever assigned.
Cache DOM references
DOM lookups aren't exactly cheap, so it may be better to store references to elements fetched via document.getElementById() as well as using the jQuery function in variables and referencing those variables later.
Use consistent DOM lookups
Much of the code uses jQuery to look up elements, but then there are other places using document.getElementById(). If you are going to have jQuery, it makes sense to use it for all DOM lookups - for the case of getting a single element (e.g. image and button) .get() can be used to access the element.
So these lines:
var image = document.getElementById('image'),
button = document.getElementById('button');
Can be simplified using jQuery and .get():
var image = $('#image').get(0),
button = $('#button').get(0);
Variable name reused
While moving image and button out of the onload callback I noticed that image was used for an Image object and then inside the callback re-assigned to the element with id attribute image. It is wise not to re-use variables like that... Perhaps a better name for the element would be imageElement.
1
Thanks for the answer, But I didn't get exactly what you said aboutcroppable, Bothcroppableandonclickare inside thereadyfunction, Do you mean that it will always be true, So I don't need that condition?
– Bon
Sep 7 at 4:40
Yes- does the updated answer provide a better explanation?
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 5:15
I copied the code from couple examples from the plugin developer, So it's not ordered/cleaned properly.
– Bon
Sep 7 at 18:25
Ah yes... the code looks very similar to the upload-cropped-image-to-server.html. It is a good thing you included your PHP code - otherwise it would be like a review of fengyuanchen's cropperjs code. In the future it might be advisable to disclose the original source of the code
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 18:37
Actually it's a combination from 3 examples, Modal, Upload to server and Round image, So it's a little messy :)
– Bon
Sep 8 at 5:40
|
show 2 more comments
up vote
0
down vote
Should I make some checks for security or just the dimensions and size checks?
It would be simple to check the type of the file uploaded. For example, you could check if the type is an image. For example, if the user uploads a PNG file, $_FILES['avatar']['type'] should be image/png, so a check for that value starting with image would be a simple conditional (e.g. using strpos() and looking for it to return 0, or preg_match('^image/', $_FILES['avatar']['type]).
The HTML could also be updated to only accept certain file types. One technique for this is to add the accept attribute to the input element with a value like image/* to limit the selectable files to images. While this does not prevent a user from selecting any file (or modifying a file to look like an image), it can act like a filter to the file selection dialog if the OS and browser support it. Read more about this in answers to Limit file format when using ?
<input type="file" id="input" name="image" accept="image/*">One could also add a change handler to check the extension of the file submitted (bear in mind that the user can also change the extension of a file through the OS).
var allowedExtensions = ['jpg', 'gif', 'bmp', 'png', 'tif'];
document.addEventListener('DOMContentLoaded', function() {
var fileInput = document.getElementById('input');
fileInput.addEventListener('change', function(e) {
var parts = this.value.split('.');
if (allowedExtensions.indexOf(parts[parts.length - 1].toLowerCase()) < 0) {
this.value = ''; // clear value
console.log('invalid extension');
}
});
});<input type="file" id="input" name="image" accept="image/*">Combining one or both of those approaches with the server-side check would be a good approach. Bearing in mind that a user could alter the HTML attributes with tools like browser consoles, as well as submit a form manually with their own page or a tool like Postman, the server-side check should be utilized for optimal protection.
Other Feedback
JavaScript
Variable croppable
It apppears that croppable is set to true in the ready function:
ready: function () {
croppable = true;
button.onclick = function () {
While it would be undefined before that line, the onclick function wouldn't be set until after that is set to true, so the following block seems superflous in the onclick function:
if (!croppable) {
return;
}
This is because by the time the click handler is set on the button, that variable will have been set to true so the condition will never evaluate to a truthy value.
I also noticed in that onclick function that three variables (i.e. croppedCanvas, roundedCanvas, and roundedImage) are declared before the conditional return. There isn't really any reason to declare those variables before the condition. Then those variables can be declared whenever assigned.
Cache DOM references
DOM lookups aren't exactly cheap, so it may be better to store references to elements fetched via document.getElementById() as well as using the jQuery function in variables and referencing those variables later.
Use consistent DOM lookups
Much of the code uses jQuery to look up elements, but then there are other places using document.getElementById(). If you are going to have jQuery, it makes sense to use it for all DOM lookups - for the case of getting a single element (e.g. image and button) .get() can be used to access the element.
So these lines:
var image = document.getElementById('image'),
button = document.getElementById('button');
Can be simplified using jQuery and .get():
var image = $('#image').get(0),
button = $('#button').get(0);
Variable name reused
While moving image and button out of the onload callback I noticed that image was used for an Image object and then inside the callback re-assigned to the element with id attribute image. It is wise not to re-use variables like that... Perhaps a better name for the element would be imageElement.
1
Thanks for the answer, But I didn't get exactly what you said aboutcroppable, Bothcroppableandonclickare inside thereadyfunction, Do you mean that it will always be true, So I don't need that condition?
– Bon
Sep 7 at 4:40
Yes- does the updated answer provide a better explanation?
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 5:15
I copied the code from couple examples from the plugin developer, So it's not ordered/cleaned properly.
– Bon
Sep 7 at 18:25
Ah yes... the code looks very similar to the upload-cropped-image-to-server.html. It is a good thing you included your PHP code - otherwise it would be like a review of fengyuanchen's cropperjs code. In the future it might be advisable to disclose the original source of the code
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 18:37
Actually it's a combination from 3 examples, Modal, Upload to server and Round image, So it's a little messy :)
– Bon
Sep 8 at 5:40
|
show 2 more comments
up vote
0
down vote
up vote
0
down vote
Should I make some checks for security or just the dimensions and size checks?
It would be simple to check the type of the file uploaded. For example, you could check if the type is an image. For example, if the user uploads a PNG file, $_FILES['avatar']['type'] should be image/png, so a check for that value starting with image would be a simple conditional (e.g. using strpos() and looking for it to return 0, or preg_match('^image/', $_FILES['avatar']['type]).
The HTML could also be updated to only accept certain file types. One technique for this is to add the accept attribute to the input element with a value like image/* to limit the selectable files to images. While this does not prevent a user from selecting any file (or modifying a file to look like an image), it can act like a filter to the file selection dialog if the OS and browser support it. Read more about this in answers to Limit file format when using ?
<input type="file" id="input" name="image" accept="image/*">One could also add a change handler to check the extension of the file submitted (bear in mind that the user can also change the extension of a file through the OS).
var allowedExtensions = ['jpg', 'gif', 'bmp', 'png', 'tif'];
document.addEventListener('DOMContentLoaded', function() {
var fileInput = document.getElementById('input');
fileInput.addEventListener('change', function(e) {
var parts = this.value.split('.');
if (allowedExtensions.indexOf(parts[parts.length - 1].toLowerCase()) < 0) {
this.value = ''; // clear value
console.log('invalid extension');
}
});
});<input type="file" id="input" name="image" accept="image/*">Combining one or both of those approaches with the server-side check would be a good approach. Bearing in mind that a user could alter the HTML attributes with tools like browser consoles, as well as submit a form manually with their own page or a tool like Postman, the server-side check should be utilized for optimal protection.
Other Feedback
JavaScript
Variable croppable
It apppears that croppable is set to true in the ready function:
ready: function () {
croppable = true;
button.onclick = function () {
While it would be undefined before that line, the onclick function wouldn't be set until after that is set to true, so the following block seems superflous in the onclick function:
if (!croppable) {
return;
}
This is because by the time the click handler is set on the button, that variable will have been set to true so the condition will never evaluate to a truthy value.
I also noticed in that onclick function that three variables (i.e. croppedCanvas, roundedCanvas, and roundedImage) are declared before the conditional return. There isn't really any reason to declare those variables before the condition. Then those variables can be declared whenever assigned.
Cache DOM references
DOM lookups aren't exactly cheap, so it may be better to store references to elements fetched via document.getElementById() as well as using the jQuery function in variables and referencing those variables later.
Use consistent DOM lookups
Much of the code uses jQuery to look up elements, but then there are other places using document.getElementById(). If you are going to have jQuery, it makes sense to use it for all DOM lookups - for the case of getting a single element (e.g. image and button) .get() can be used to access the element.
So these lines:
var image = document.getElementById('image'),
button = document.getElementById('button');
Can be simplified using jQuery and .get():
var image = $('#image').get(0),
button = $('#button').get(0);
Variable name reused
While moving image and button out of the onload callback I noticed that image was used for an Image object and then inside the callback re-assigned to the element with id attribute image. It is wise not to re-use variables like that... Perhaps a better name for the element would be imageElement.
Should I make some checks for security or just the dimensions and size checks?
It would be simple to check the type of the file uploaded. For example, you could check if the type is an image. For example, if the user uploads a PNG file, $_FILES['avatar']['type'] should be image/png, so a check for that value starting with image would be a simple conditional (e.g. using strpos() and looking for it to return 0, or preg_match('^image/', $_FILES['avatar']['type]).
The HTML could also be updated to only accept certain file types. One technique for this is to add the accept attribute to the input element with a value like image/* to limit the selectable files to images. While this does not prevent a user from selecting any file (or modifying a file to look like an image), it can act like a filter to the file selection dialog if the OS and browser support it. Read more about this in answers to Limit file format when using ?
<input type="file" id="input" name="image" accept="image/*">One could also add a change handler to check the extension of the file submitted (bear in mind that the user can also change the extension of a file through the OS).
var allowedExtensions = ['jpg', 'gif', 'bmp', 'png', 'tif'];
document.addEventListener('DOMContentLoaded', function() {
var fileInput = document.getElementById('input');
fileInput.addEventListener('change', function(e) {
var parts = this.value.split('.');
if (allowedExtensions.indexOf(parts[parts.length - 1].toLowerCase()) < 0) {
this.value = ''; // clear value
console.log('invalid extension');
}
});
});<input type="file" id="input" name="image" accept="image/*">Combining one or both of those approaches with the server-side check would be a good approach. Bearing in mind that a user could alter the HTML attributes with tools like browser consoles, as well as submit a form manually with their own page or a tool like Postman, the server-side check should be utilized for optimal protection.
Other Feedback
JavaScript
Variable croppable
It apppears that croppable is set to true in the ready function:
ready: function () {
croppable = true;
button.onclick = function () {
While it would be undefined before that line, the onclick function wouldn't be set until after that is set to true, so the following block seems superflous in the onclick function:
if (!croppable) {
return;
}
This is because by the time the click handler is set on the button, that variable will have been set to true so the condition will never evaluate to a truthy value.
I also noticed in that onclick function that three variables (i.e. croppedCanvas, roundedCanvas, and roundedImage) are declared before the conditional return. There isn't really any reason to declare those variables before the condition. Then those variables can be declared whenever assigned.
Cache DOM references
DOM lookups aren't exactly cheap, so it may be better to store references to elements fetched via document.getElementById() as well as using the jQuery function in variables and referencing those variables later.
Use consistent DOM lookups
Much of the code uses jQuery to look up elements, but then there are other places using document.getElementById(). If you are going to have jQuery, it makes sense to use it for all DOM lookups - for the case of getting a single element (e.g. image and button) .get() can be used to access the element.
So these lines:
var image = document.getElementById('image'),
button = document.getElementById('button');
Can be simplified using jQuery and .get():
var image = $('#image').get(0),
button = $('#button').get(0);
Variable name reused
While moving image and button out of the onload callback I noticed that image was used for an Image object and then inside the callback re-assigned to the element with id attribute image. It is wise not to re-use variables like that... Perhaps a better name for the element would be imageElement.
<input type="file" id="input" name="image" accept="image/*"><input type="file" id="input" name="image" accept="image/*">var allowedExtensions = ['jpg', 'gif', 'bmp', 'png', 'tif'];
document.addEventListener('DOMContentLoaded', function() {
var fileInput = document.getElementById('input');
fileInput.addEventListener('change', function(e) {
var parts = this.value.split('.');
if (allowedExtensions.indexOf(parts[parts.length - 1].toLowerCase()) < 0) {
this.value = ''; // clear value
console.log('invalid extension');
}
});
});<input type="file" id="input" name="image" accept="image/*">var allowedExtensions = ['jpg', 'gif', 'bmp', 'png', 'tif'];
document.addEventListener('DOMContentLoaded', function() {
var fileInput = document.getElementById('input');
fileInput.addEventListener('change', function(e) {
var parts = this.value.split('.');
if (allowedExtensions.indexOf(parts[parts.length - 1].toLowerCase()) < 0) {
this.value = ''; // clear value
console.log('invalid extension');
}
});
});<input type="file" id="input" name="image" accept="image/*">edited Dec 2 at 22:56
answered Sep 6 at 19:22
Sᴀᴍ Onᴇᴌᴀ
8,09961751
8,09961751
1
Thanks for the answer, But I didn't get exactly what you said aboutcroppable, Bothcroppableandonclickare inside thereadyfunction, Do you mean that it will always be true, So I don't need that condition?
– Bon
Sep 7 at 4:40
Yes- does the updated answer provide a better explanation?
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 5:15
I copied the code from couple examples from the plugin developer, So it's not ordered/cleaned properly.
– Bon
Sep 7 at 18:25
Ah yes... the code looks very similar to the upload-cropped-image-to-server.html. It is a good thing you included your PHP code - otherwise it would be like a review of fengyuanchen's cropperjs code. In the future it might be advisable to disclose the original source of the code
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 18:37
Actually it's a combination from 3 examples, Modal, Upload to server and Round image, So it's a little messy :)
– Bon
Sep 8 at 5:40
|
show 2 more comments
1
Thanks for the answer, But I didn't get exactly what you said aboutcroppable, Bothcroppableandonclickare inside thereadyfunction, Do you mean that it will always be true, So I don't need that condition?
– Bon
Sep 7 at 4:40
Yes- does the updated answer provide a better explanation?
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 5:15
I copied the code from couple examples from the plugin developer, So it's not ordered/cleaned properly.
– Bon
Sep 7 at 18:25
Ah yes... the code looks very similar to the upload-cropped-image-to-server.html. It is a good thing you included your PHP code - otherwise it would be like a review of fengyuanchen's cropperjs code. In the future it might be advisable to disclose the original source of the code
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 18:37
Actually it's a combination from 3 examples, Modal, Upload to server and Round image, So it's a little messy :)
– Bon
Sep 8 at 5:40
1
1
Thanks for the answer, But I didn't get exactly what you said about
croppable, Bothcroppable and onclick are inside the ready function, Do you mean that it will always be true, So I don't need that condition?– Bon
Sep 7 at 4:40
Thanks for the answer, But I didn't get exactly what you said about
croppable, Bothcroppable and onclick are inside the ready function, Do you mean that it will always be true, So I don't need that condition?– Bon
Sep 7 at 4:40
Yes- does the updated answer provide a better explanation?
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 5:15
Yes- does the updated answer provide a better explanation?
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 5:15
I copied the code from couple examples from the plugin developer, So it's not ordered/cleaned properly.
– Bon
Sep 7 at 18:25
I copied the code from couple examples from the plugin developer, So it's not ordered/cleaned properly.
– Bon
Sep 7 at 18:25
Ah yes... the code looks very similar to the upload-cropped-image-to-server.html. It is a good thing you included your PHP code - otherwise it would be like a review of fengyuanchen's cropperjs code. In the future it might be advisable to disclose the original source of the code
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 18:37
Ah yes... the code looks very similar to the upload-cropped-image-to-server.html. It is a good thing you included your PHP code - otherwise it would be like a review of fengyuanchen's cropperjs code. In the future it might be advisable to disclose the original source of the code
– Sᴀᴍ Onᴇᴌᴀ
Sep 7 at 18:37
Actually it's a combination from 3 examples, Modal, Upload to server and Round image, So it's a little messy :)
– Bon
Sep 8 at 5:40
Actually it's a combination from 3 examples, Modal, Upload to server and Round image, So it's a little messy :)
– Bon
Sep 8 at 5:40
|
show 2 more comments
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f203244%2fsend-blob-image-using-ajax-php%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown