Add form post signature #58
Conversation
|
Looking good! This is more or less what I developed previously in a test. One thing left to test (at least for me) is to check whether you can use Custom Domains to submit Form Posts, or if you would need to use OVH’s URL format for that. |
|
One thing, I am thinking that the Also, one thing that should change is that the defaults should maybe come from config. The config options are really become too large for something simple, but I suppose it would be nice to have the possibility. The server resources will be used when uploading using the custom endpoint. Maybe it's better to use the regular upload when doing that? It should probably work though. Would you be okay to test it? |
|
I think it’s okay to enforce expiration parameter for the generation function. User should be able to truly know what they’re doing. I’ll be on my way to keep testing against custom endpoints, hopefully they’ll work. Otherwise, traditional url should be used. The main reason to use Form Post option is to relax the stress of using a server as a middleman for file uploading. |
|
I've encountered a weird situation regarding CORS... If you upload via Javascript (axios) you can hit an error regarding EDIT: Javascript upload works fine without setting up the corresponding CORS headers, but will throw an annoying error on browser's console. From CLI, the command would be: I'm not an expert with CORS, but allowing every domain doesn't seem harmful while using Signed petitions. |
|
FormPost using Custom Endpoints works flawlessly! Tested against Cloudflare DNS using Flexible SSL. Below is the code I used to test the From Upload. web.php fileRoute::get('/', function () {
return view('welcome');
});
Route::get('signature', function () {
$path = '';
$redirect = '';
$expires = now()->addMinutes(5)->getTimestamp();
$max_file_size = 1 * 1024 * 1024;
$max_file_count = 5;
$options = compact('path', 'redirect', 'expires', 'max_file_count', 'max_file_size');
$adapter = Storage::cloud()->getAdapter();
$options['signature'] = $adapter->getFormPostSignature('', $options);
$options['url'] = $adapter->getUrl($path);
return response()->json($options);
});welcome.blade.php file<!DOCTYPE html>
<html lang="{{ str_replace('_', '-', app()->getLocale()) }}">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>
OVH Playground
</title>
</head>
<body>
<form
method="POST"
enctype="multipart/form-data"
>
<input id="files" type="file" multiple />
</form>
<script src="https://unpkg.com/axios@0.19.2/dist/axios.min.js"></script>
<script>
function tryUpload() {
axios.get('/signature')
.then(response => {
console.log('Got signature, building FormData')
let data = new FormData()
data.append('max_file_size', response.data['max_file_size'])
data.append('max_file_count', response.data['max_file_count'])
data.append('expires', response.data['expires'])
data.append('signature', response.data['signature'])
const files = document.getElementById('files').files
for (let i = 0; i < files.length; ++i) {
data.append('file[]', files[i])
}
console.log('Data Ready, uploading...')
axios.post(response.data['url'], data)
.then(response => {
console.log('File Uploaded', response)
})
.catch(error => {
console.log(error, error.message)
})
})
.catch(error => {
console.log(error, error.message)
})
}
</script>
</body>
</html> |
I suppose the right thing to do would be to have a command option in the package to set these headers. I'll add separate PR for the command, but the release can be together with the command in place. Thanks for testing 👍 |
|
Talking about configuration, I think we should not provide a configuration proxy for this, as it should be intended for the developer to place stricter rules on the data submission by end users. Config defaults may let some developers relax the limits about what their users upload, this way, we encourage them to keep an eye on default values each time they implement a FormPost |
|
Once this is merged, I would like to send another PR before tagging the release, looking into full param list and a way to abstract signature logic with TempUrlKey (If possible, if not, just params) |
Agree! |
First iteration on implementation as discussed in #48