Skip to content

Add Views to blog#10

Open
MegamindAme wants to merge 4 commits into
archey347:masterfrom
MegamindAme:develop
Open

Add Views to blog#10
MegamindAme wants to merge 4 commits into
archey347:masterfrom
MegamindAme:develop

Conversation

@MegamindAme
Copy link
Copy Markdown
Contributor

Add Views to blog and blog post


public function uploadImage(Request $request, Response $response, $args)
{
$params = $request->getParsedBody();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, this allows anybody to upload files, to a folder that is publicly accessible via the web server. This is a major security vulnerability. I'd recommend reading https://owasp.org/www-community/vulnerabilities/Unrestricted_File_Upload for why this is a bad idea.

A way around this could be to add something similar to the blog editing method.

return $this->ci->view->render($response, 'raw/upload_message.json.twig', [
'image_url' => $image_url,
]);
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using twig to generate JSON, Slim has a way of being able to automatically generate the JSON for you:

return $response->withJson([
    'image_url' => $image_url,
]);

Copy link
Copy Markdown
Owner

@archey347 archey347 Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although having said that, in newer versions of Slim this has disappeared, so something like this would be better.

$payload = json_encode([
    'image_url' => $image_url,
], JSON_THROW_ON_ERROR);
$response->getBody()->write($payload);
return $response->withHeader('Content-Type', 'application/json');

@archey347
Copy link
Copy Markdown
Owner

Apologies, I reviewed this a while ago but forgot to press the submit button 🙃

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than re-creating a new base template, it'd be worth extending of UserFrosting's main one, so that it has the same style:

https://github.com/userfrosting/UserFrosting/blob/4.6/app/sprinkles/core/templates/pages/abstract/default.html.twig

@archey347
Copy link
Copy Markdown
Owner

Thanks for submitting this PR. It does have quite a few new features in it; in future, could you do an individual pull-request per feature, it just makes reviewing and making sense of the changes a lot easier (and therefore take less time for it to be merged).

public function up()
{
$this->schema->table('blog_posts', function (Blueprint $table) {
$table->string('post_style_slug');
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a new column here, but it appears to not be used anywhere?

public function up()
{
$this->schema->table('blogs', function (Blueprint $table) {
$table->string('blog_style_slug', 500)->unique()->comment('Style twig template file');
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a new column here, but it appears to not be used anywhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants